Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ios] Recover the deleted bookmark using the Recover button on the place page #8352

Merged

Conversation

kirylkaveryn
Copy link
Contributor

@kirylkaveryn kirylkaveryn commented May 31, 2024

This PR allows recovering accidentally deleted bookmarks from the Place Page while the Place Page isn't closed yet.

When a user closes the Place Page or taps on another POI a recently deleted bookmark will be removed from the memory and restoring it will not be possible.

Fixes #1173
Fixes #4547

Todo:

  • implement add the Recover title to Android. For now it works correctly, but without the proper word (Save is shown where the Recover should be). @Jean-BaptisteC can you please help with that?

Simulator Screen Recording - iPhone 15 Pro - 2024-06-01 at 00 02 10
Simulator Screen Recording - iPhone 15 Pro - 2024-06-01 at 00 03 45

Screen_recording_20240601_003543.mp4

@kirylkaveryn kirylkaveryn added iOS iOS development Bookmarks and Tracks Bookmarks, imported tracks, and KML, KMZ, KMB, GPX, GPZ import or export UI User interface issues labels May 31, 2024
@biodranik biodranik added this to the Ready for alpha testing milestone May 31, 2024
@kirylkaveryn kirylkaveryn marked this pull request as ready for review May 31, 2024 20:42
@kirylkaveryn kirylkaveryn force-pushed the ios/recover-bookmark-using-recover-button-on-place-page branch from 1427d65 to 805707b Compare June 1, 2024 12:14
@rtsisyk
Copy link
Contributor

rtsisyk commented Jun 2, 2024

to be reviewed after finishing with the current release

Copy link
Member

@biodranik biodranik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Does it work well with iCloud sync?

map/bookmark_manager.cpp Outdated Show resolved Hide resolved
map/bookmark_manager.cpp Show resolved Hide resolved
map/bookmark_manager.cpp Outdated Show resolved Hide resolved
map/bookmark_manager.cpp Outdated Show resolved Hide resolved
map/bookmark_manager.cpp Outdated Show resolved Hide resolved
map/bookmark_manager.cpp Outdated Show resolved Hide resolved
map/bookmark_manager.hpp Outdated Show resolved Hide resolved
map/bookmark_manager.hpp Outdated Show resolved Hide resolved
@kirylkaveryn
Copy link
Contributor Author

Thanks! Does it work well with iCloud sync?

Yes! This case was tested.
When a new update comes from the cloud, the category is reloaded the opened PP will be closed if bm has an invalid ID.
If the user deletes the bm the cloud will be updated as usual.

We do not support the PP screen state updation when the BM was updated from the cloud because the problem with the bm identification wasn't solved (this is why we reload the entire collection).

@kirylkaveryn kirylkaveryn force-pushed the ios/recover-bookmark-using-recover-button-on-place-page branch from c66ca0e to 838c149 Compare June 6, 2024 09:11
@kirylkaveryn
Copy link
Contributor Author

@biodranik I've updated the discussed issues.
But I didn't remove the RemoveRecentlyDeletedBookmark(); because its needed for deletion from the Framework when the PP is disappearing or when a user changes the poi so it should be public.

@kirylkaveryn kirylkaveryn force-pushed the ios/recover-bookmark-using-recover-button-on-place-page branch from 838c149 to 01f4f9d Compare June 6, 2024 14:04
@biodranik
Copy link
Member

@kirylkaveryn can you please take a look at 0630ac3 ? There is a way to avoid creating bookmark and copying bmData, by keeping and reusing the original bookmark. WDYT?

@biodranik
Copy link
Member

@kirylkaveryn I've updated the commits after our discussion call. Also added Android Restore button.

Can you please test it, and then squash commits into a better history, something like:

  • C++ support
  • iOS
  • Android
  • strings
  • strings regen?

@kirylkaveryn kirylkaveryn force-pushed the ios/recover-bookmark-using-recover-button-on-place-page branch from e0aa258 to 16fd52b Compare June 7, 2024 08:31
kirylkaveryn and others added 5 commits June 7, 2024 13:27
Signed-off-by: Kiryl Kaveryn <kirylkaveryn@gmail.com>
Signed-off-by: Kiryl Kaveryn <kirylkaveryn@gmail.com>
Signed-off-by: Kiryl Kaveryn <kirylkaveryn@gmail.com>
Signed-off-by: Alexander Borsuk <me@alex.bio>
Signed-off-by: Kiryl Kaveryn <kirylkaveryn@gmail.com>
@kirylkaveryn kirylkaveryn force-pushed the ios/recover-bookmark-using-recover-button-on-place-page branch from 16fd52b to fcb2c5e Compare June 7, 2024 09:35
Copy link
Member

@biodranik biodranik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Дзякуй!

@biodranik biodranik merged commit 391e910 into master Jun 7, 2024
16 checks passed
@biodranik biodranik deleted the ios/recover-bookmark-using-recover-button-on-place-page branch June 7, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bookmarks and Tracks Bookmarks, imported tracks, and KML, KMZ, KMB, GPX, GPZ import or export iOS iOS development UI User interface issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[android][ios] Deleting a bookmark needs an undo Too simple to remove a bookmark
3 participants