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

Conversation

@jflinter
Copy link

(Note: this is the same PR as #513, which I've closed - I wanted to issue this PR from a different fork, and don't know of a more elegant way to do it).

@appleguy: thanks for the review. I'm not 100% sure of things here, since I'm not insanely experienced working with the responder chain, but here are my thoughts so far regarding your questions:

  • layer-backed/rasterized nodes shouldn't ever care about becoming first responder, as this only really pertains to UIView hierarchies. To be more explicit about this, I now check if a node is layerBacked when calling becomeFirstResponder/resignFirstResponder and immediately return NO if so.
  • I'm not really sure how to measure the performance degradation you're worried about. I don't know how UIKit keeps track of what views can become the first responder - presumably it tracks some kind of internal state on each view in the hierarchy to avoid having to recompute the first responder every time it's handling a touch event. That feels pretty lightweight in concept to me, but I suppose you could probably devise a worst-case scenario that might require traversing a lot of views when adding/removing children. Honestly, I'd be surprised if the responder chain was really a performance bottleneck when dealing with complex view hierarchies, but ¯_(ツ)_/¯
  • My intended use case is actually the same as [ASDisplayNode] Support UIMenuController #11 - I'd like to allow long-pressing on an ASTextNode subclass in order to show a UIMenuController and copy its contents. I'm currently doing this is in the app I'm working on (pointing at my branch of ASDK) and it's working well. Also, not sure if this really counts as a use case, but I've also updated ASEditableTextNode to implement these methods properly and it feels sane to me.
  • I've added a couple of simple unit tests. They mostly just test that the default/subclassed/layerBacked behavior works as intended.

Ultimately, given that the changes here require opting-in via subclassing anyway, I'm in favor of merging early and waiting to see if anyone actually encounters any performance issues in practice, but I'm obviously a bit biased : )

appleguy added a commit that referenced this pull request Jul 5, 2015
Add UIResponder methods to ASDisplayNode
@appleguy appleguy merged commit 3f574a3 into facebookarchive:master Jul 5, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants