Added copy support for message bubble#422
Conversation
| let selectedIndexPath = selectedIndexPathForMenu { | ||
| NotificationCenter.default.removeObserver(self, name: NSNotification.Name.UIMenuControllerWillShowMenu, object: nil) | ||
| defer { | ||
| NotificationCenter.default.addObserver(self, selector: #selector(MessagesViewController.menuControllerWillShow(aNotification:)), name: NSNotification.Name.UIMenuControllerWillShowMenu, object: nil) |
There was a problem hiding this comment.
Just a question, why do we keep adding ab observer here? It looks like we never remove it
There was a problem hiding this comment.
@nathantannar4 , emm, we need remove here first, because we call currentMenuController.setMenuVisible(true, animated: true) later, it would leads to endless loop if we not remove in advance.
|
@zhongwuzw i mean why add the observer Sent with GitHawk |
|
@nathantannar4 , we need customize the target rect to bubble, default would target rect to cell. |
|
Thanks for doing this @zhongwuzw! It will take me some time to be able to review it |
| } | ||
|
|
||
| /// Show menuController and set target rect to selected bubble | ||
| @objc func menuControllerWillShow(aNotification:Notification) { |
There was a problem hiding this comment.
@zhongwuzw Can we just name this parameter notification? And add a space after the colon
| - Added `configureAvatarView(_ avatarView: AvatarView, for message: MessageType, at indexPath: IndexPath, in messagesCollectionView: MessagesCollectionView)` method in `MessagesDisplayDelegate` `protocol` to configure `avatarView`. | ||
| [#416](https://github.com/MessageKit/MessageKit/pull/416) by [@zhongwuzw](https://github.com/zhongwuzw). | ||
|
|
||
| - Added copy support for message bubble. |
There was a problem hiding this comment.
"Added copy support for image, text, and emoji messages".
This would sound better. Sorry!
| extension MessagesViewController { | ||
| /// Add observer for `UIMenuControllerWillShowMenu` notification | ||
| fileprivate func addMenuControllerObservers() { | ||
| NotificationCenter.default.addObserver(self, selector: #selector(MessagesViewController.menuControllerWillShow(aNotification:)), name: NSNotification.Name.UIMenuControllerWillShowMenu, object: nil) |
There was a problem hiding this comment.
Can we use the shorthand version of the notification names please .UIMenuControllerWillShowMenu?
| /// Handle long press gesture, return true when gestureRecognizer's touch point in `messageContainerView`'s frame | ||
| open override func gestureRecognizerShouldBegin(_ gestureRecognizer: UIGestureRecognizer) -> Bool { | ||
| let touchPoint = gestureRecognizer.location(in: self) | ||
| if gestureRecognizer.isKind(of: UILongPressGestureRecognizer.self) { |
There was a problem hiding this comment.
How about this?
guard gestureRecognizer.isKind(of: UILongPressGestureRecognizer.self) else { return false }
return messageContainerView.frame.contains(touchPoint)
|
@zhongwuzw Think there is any way to prevent the menu from appearing outside the content view? What about adding support for copying the image from |
@SD10 , good question, another problem, what if message's height exceed the screen height?where should we position menu at? |
@zhongwuzw I think we should position it at the bottom of the message. It may work like that already. |
|
@SD10 , I updated the code to handle some situations, like situation below, menu would be blocked by keyboard, |
SD10
left a comment
SD10
left a comment
There was a problem hiding this comment.
@zhongwuzw If the menu is covered by the keyboard in JSQMessagesViewController then we can merge this for now. Let's open an issue to track this behavior?
3 participants
#248