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] [bookmarks] Remove observer pattern during the category files exporting #7933

Merged
merged 3 commits into from
Apr 16, 2024

Conversation

kirylkaveryn
Copy link
Contributor

@kirylkaveryn kirylkaveryn commented Apr 16, 2024

For now to handle the file sharing OM uses the observing pattern tightened to the MWMBookmarksManagers, that is too complex for this straightforward task:

  1. subscribe for the observarion
  2. register observer
  3. send sharing message
  4. wait for sharing result
  5. handle sharing result
  6. notify observers
  7. unsubscribe from observing

This is too complex because the c++ method that we interact with uses the completion handlers to return sharing results and there is no sense in wrapping the completion handle into observation:

void BookmarkManager::PrepareFileForSharing(kml::GroupIdCollection && categoriesIds, SharingHandler && handler)

This PR converts c++ completion handler into the objc and returns to the caller so we can share files easily with the MWMBookmarksManagers and handle result without additional overhead:

- (void)shareCategory:(MWMMarkGroupID)groupId completion:(SharingResultCompletionHandler)completion;
- (void)shareAllCategoriesWithCompletion:(SharingResultCompletionHandler)completion;
trim.BEEC13F2-FDDE-44CD-99A6-A051EA627A6E.MOV

@kirylkaveryn kirylkaveryn added iOS iOS development Bookmarks and Tracks Bookmarks, imported tracks, and KML, KMZ, KMB, GPX import or export labels Apr 16, 2024
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, the functionality did not change, right? All cases (including errors) are properly handled now in the Bookmarks and Tracks dialog, right?

iphone/CoreApi/CoreApi/Bookmarks/MWMBookmarksManager.h Outdated Show resolved Hide resolved
iphone/CoreApi/CoreApi/Bookmarks/MWMBookmarksManager.mm Outdated Show resolved Hide resolved
@kirylkaveryn
Copy link
Contributor Author

Thanks, the functionality did not change, right? All cases (including errors) are properly handled now in the Bookmarks and Tracks dialog, right?

Yes!
Tested on iPhone 11Pro (17.2) and iPhone 6 (12.5).

@kirylkaveryn kirylkaveryn force-pushed the ios/share-bookmarks-with-completion-handler branch from 421ecf7 to 4dfa883 Compare April 16, 2024 17:25
…ger to not use observers

Signed-off-by: Kiryl Kaveryn <kirylkaveryn@gmail.com>
…all files sharing

Signed-off-by: Kiryl Kaveryn <kirylkaveryn@gmail.com>
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!

…hods

Signed-off-by: Kiryl Kaveryn <kirylkaveryn@gmail.com>
@kirylkaveryn kirylkaveryn force-pushed the ios/share-bookmarks-with-completion-handler branch from 4dfa883 to d34360d Compare April 16, 2024 18:10
@kirylkaveryn
Copy link
Contributor Author

Updated with documentation!

@biodranik biodranik merged commit 4566643 into master Apr 16, 2024
5 checks passed
@biodranik biodranik deleted the ios/share-bookmarks-with-completion-handler branch April 16, 2024 18:41
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 import or export iOS iOS development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants