Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

[ASDisplayNode] Support UIMenuController #11

Closed
czechboy0 opened this issue Jul 26, 2014 · 13 comments
Closed

[ASDisplayNode] Support UIMenuController #11

czechboy0 opened this issue Jul 26, 2014 · 13 comments
Milestone

Comments

@czechboy0
Copy link

I found two identifiers that were only mentioned in comments but the types themselves aren't defined anywhere:

ASDisplayNodeView, seems like a protocol that any view usable in a async display node would need to implement, mentioned in _ASPendingState.h:25.

ASDisplayNodeAsyncView, seems like a subclass of UIView that would too be usable in a node (also the way of adding more async views by subclassing), mentioned in ASDisplayNode.h:22.

Could you please add more details on these types? Plus some sort of a documentation of how to add a custom async view class. Thanks!

@czechboy0
Copy link
Author

Incomplete comment on addSubnodeAsynchronously:completion: in ASDisplayNode.h:82.

@appleguy
Copy link
Contributor

Thanks for reporting this! We are working in documentation now, as part of taking the framework out of Beta soon.

There should be no need to create a different asyncview subclass. If you think of one, please let us know and maybe we can add the desired functionality in a different way.

Check out -initWithViewClass: for a convenient way to wrap synchronous views such as UIScrollView, UIWebView, UITextField, etc.

@czechboy0
Copy link
Author

Good to hear that. I don't mind reading just code, but docs are definitely going to be useful once this goes public.

I was thinking about some of the problems we discussed on Thursday re height in table views, cell recycling ownership issues and I wanted to try to build an async collection view which could do a lot of the slow stuff (computing cell layout attributes, scroll indicator position) asynchronously, plus provide a first-class citizen container view for ASDN.

Assuming the collection view would be the first async view in the hierarchy, I would need the communication (dispatching from main to bg queue) from view to node (built into _ASDisplayView already) for UICollectionView-specific methods as well. I could put most of the logic into the 'collection view node', but I think I'd still need a special 'async collection view' in the hierarchy to forward the calls to the node. Or am I missing something?

From how I understand it, initWithViewClass: would automatically make the node synchronous (since the initializer checks whether the view class is a subclass of _ASDisplayView). That's why I assumed that the only way to build a truly async collection view would be to have a view subclassing _ASDisplayView.

By the way, thanks for putting this out there. I read through most of the source and was glad to find many solutions to similar problems I've been trying to tackle for some time. Making UI code fast is pretty hard - I'm happy to see you guys did it. I really hope this makes it back to UIKit somehow.

@secretiverhyme
Copy link
Contributor

Whoops, these are codemod goofs — ASDisplayNodeAsyncView should be _ASDisplayView, and the ASDisplayNodeView protocol is now ASDisplayNodeViewProperties. Nice catch.

secretiverhyme added a commit that referenced this issue Jul 30, 2014
* ASDisplayNodeAsyncView -> _ASDisplayView
* ASDisplayNodeView -> ASDisplayNodeViewProperties
@zetachang
Copy link

@appleguy , I subclass _ASDisplayView to achieve,

  1. Make the node remain asynchronous , I thought -initWithViewClass will make it synchronous by default.
  2. Override UIResponder methods (canPerformAction:withSender:, etc) and UIResponderStandardEditActions to make a node copiable. (presenting UIMenuController)

Any other suggestion to do this without subclassing _ASDisplayView?

@secretiverhyme
Copy link
Contributor

I suspect the ideal solution here would be to add UIResponderStandardEditActions and these override points to ASDisplayNode itself — that would let you enable UIMenuController on an arbitrary node without needing to set a custom view class. @appleguy probably has more thoughts here.

@toulouse
Copy link
Contributor

toulouse commented Oct 7, 2014

UIMenuController doesn't support arbitrary objects; it seems to use the responder chain, so you'd probably need to implement methods on _ASDisplayView to trampoline to the node if appropriate.

@secretiverhyme secretiverhyme changed the title Clarification needed on ASDisplayNodeView and ASDisplayNodeAsyncView Add UIMenuController support to _ASDisplayView Oct 7, 2014
@appleguy
Copy link
Contributor

Agree with @toulouse. @czechboy0, do you have any interest in crafting a diff to _ASDisplayView that achieves the same purpose as your subclass? It seems like this would only be moderately complex, although we have a few things that are higher priority than this right now, so it would take a while for the internal team to get to.

This is definitely useful functionality and a very good suggestion I hadn't thought of :)

@zetachang
Copy link

Below are methods I implemented in the subclass (just to make a ASTextNode copiable)

- (BOOL)canBecomeFirstResponder {
    return YES;
}

- (BOOL)resignFirstResponder {
    UIMenuController *menuController = [UIMenuController sharedMenuController];
    if (menuController.menuVisible) {
        [menuController setMenuVisible:NO animated:YES];
    }

    return YES;
}

- (BOOL)canPerformAction:(SEL)action
              withSender:(id)sender
{
    return (action == @selector(copy:));
}

#pragma mark - UIResponderStandardEditActions

- (void)copy:(id)sender {
    ASTextNode* textNode = (ASTextNode*)ASViewToDisplayNode(self);
    [[UIPasteboard generalPasteboard] setString:textNode.attributedString.string];
}

But currently I got user reported that they want the "Speak" button (which is available in text view) back. So to support the flexibility of customizing the behavior (Cut, Paste, etc), this may not be enough though.

Also, I've noticed that in Paper, you've used a customized menu for doing "Copy" action. Is there any chance to have this component available for AsyncDisplayKit?

@toulouse
Copy link
Contributor

Using the above as a guide, and adding those three methods to _ASDisplayView for forwarding to default ASDisplayNode, I've managed to get a UIMenuController to show up.

Two problems:

  1. When - (BOOL)canPerformAction:(SEL)action withSender:(id)sender returns YES, it calls that method on the view. Might need to implement - (id)targetForAction:(SEL)action withSender:(id)sender ?
  2. Scrolling the scrollview doesn't make the menu disappear. I didn't include the - (BOOL)resignFirstResponder snippet above, because I wasn't sure if that was hacky or not.

EDIT: For 1) I've just verified that forwarding targetForAction to the node works. However, I'm getting a strange issue where every possible default action is showing up in the UIMenuController. Have some work to do there to see what's up.
EDIT 2: It's because canPerformAction is something that targetForAction calls. I need to only return 'self' when canPerformAction is true, and forward it to nextResponder otherwise.

@appleguy appleguy changed the title Add UIMenuController support to _ASDisplayView [ASDisplayNode] Support UIMenuController Nov 1, 2015
@appleguy
Copy link
Contributor

Here's another duplicate - #1017 Task #11, meet task #1017!

This feature has been rarely requested, but the fact that it's come up about three times now definitely indicates we should find a good solution. Any thoughts from the community on the tradeoffs / requirements that we should define?

From my note to @tuyuan2012 -- I think it is possible to hook up the UIMenuController, though it is not very flexible and so there was some discussion about whether it would be best to build a custom callout bubble (that style of button selector). Are you wanting to use the full selection system with ASEditableTextNode, or just the menu controller with static ASTextNode ? Do you need to be able to use the selection handles to select a range, or just copy all in a given block? If you're able to upload an image of the case you're looking at in the UI that you'd like to implement, that would be a helpful guide for us to think through the possibilities (the two main branches being - integrate with UIKit's menu controller stuff, or instead just make something custom that is more flexible).

@appleguy
Copy link
Contributor

@czechboy0 @secretiverhyme @zetachang @toulouse - The community is planning an exciting long term road map for the project and getting organized around how to deliver these feature requests. We are moving to a new issue tracker with more sophisticated planning capabilities (check out www.realArtists.com and their new product: Ship!).

If you are interested in helping contribute to this component or any other, don’t hesitate to send me an email at AsyncDisplayKit@gmail.com. If you would like to contribute for a few weeks, I can also add you to our Ship bug tracker so that you can see what everyone is working on and actively coordinate with us.

As always, keep filing issues and submitting pull requests here on Github and we will only move things to the new tracker if they require long term coordination.

@appleguy
Copy link
Contributor

Closing issue.

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

Successfully merging a pull request may close this issue.

5 participants