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

Provide support for context menus #1

Merged
merged 8 commits into from
Sep 2, 2021

Conversation

sjchmiela
Copy link

@sjchmiela sjchmiela commented Sep 2, 2021

Instagram#1430 +:

  • merged latest master in
    • the only conflict was in CHANGELOG
  • added return to collectionView:contextMenuConfigurationForItemAt:point: as per this comment
  • fixed indentation in existing changes to the codebase
    • thanks for letting me know of Reindent!
  • removed context menu methods from selection delegate as per this comment
  • added support for preview methods too (we need it to customize UITargetedPreview)
    • I left/added support for implementing context menu in section controllers even though we are not using it right now. I think author of the original pull request intended to let developers implement context menu methods in either collectionViewDelegate of the IGListAdapter or section controllers (see this comment).
    • Since we do receive indexPath when asked about context menu configuration it's easy to grab the appropriate section controller.
    • It is not so easy when we're asked for the targeted preview though where we only receive the configuration.
      • In ChatViewController I used configuration.identifier = indexPath as NSCopying to be able to figure out for which message the preview is. Here we can't really do that — it's the delegate who should be able to set their own identifier, we can't override it in library.
      • To match configuration and section controller I added an NSMapTable with weak keys and weak values — as long as configuration and section controller are used/remembered/retained by other objects we will remember them in the table. As long as they are to be forgotten – they are since the references we hold are weak.

@sjchmiela sjchmiela force-pushed the feature/support-for-context-menu branch from b498546 to 223ee4b Compare September 2, 2021 09:13
@sjchmiela sjchmiela requested a review from danqing September 2, 2021 09:29
@sjchmiela sjchmiela merged commit 753d660 into master Sep 2, 2021
@sjchmiela sjchmiela deleted the feature/support-for-context-menu branch September 2, 2021 19:16
@amalenduk
Copy link

Hi @sjchmiela, The example you have added doesn't work at all, I see you are returning nil (UIContextMenuConfiguration *)collectionView:(UICollectionView *)collectionView contextMenuConfigurationForItemAtIndexPath:(NSIndexPath *)indexPath point:(CGPoint)point in this function you should have returned configuration otherwise it doesn't work at all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants