-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix UIResponder handling with view backing ASDisplayNode #789
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
Changes from all commits
27a9013
e5dae0f
661a297
e27e300
b6dd993
7b361e2
87d0df2
77cbec0
8336359
cc30688
f405577
4f3e113
c71b3f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -545,6 +545,8 @@ - (UIView *)_locked_viewToLoad | |
|
|
||
| // Special handling of wrapping UIKit components | ||
| if (checkFlag(Synchronous)) { | ||
| [self checkResponderCompatibility]; | ||
|
|
||
| // UIImageView layers. More details on the flags | ||
| if ([_viewClass isSubclassOfClass:[UIImageView class]]) { | ||
| _flags.canClearContentsOfLayer = NO; | ||
|
|
@@ -828,6 +830,97 @@ - (void)nodeViewDidAddGestureRecognizer | |
| _flags.viewEverHadAGestureRecognizerAttached = YES; | ||
| } | ||
|
|
||
| #pragma mark UIResponder | ||
|
|
||
| #define HANDLE_NODE_RESPONDER_METHOD(__sel) \ | ||
| /* All responder methods should be called on the main thread */ \ | ||
| ASDisplayNodeAssertMainThread(); \ | ||
| if (checkFlag(Synchronous)) { \ | ||
| /* If the view is not a _ASDisplayView subclass (Synchronous) just call through to the view as we | ||
| expect it's a non _ASDisplayView subclass that will respond */ \ | ||
| return [_view __sel]; \ | ||
| } else { \ | ||
| if (ASSubclassOverridesSelector([_ASDisplayView class], _viewClass, @selector(__sel))) { \ | ||
| /* If the subclass overwrites canBecomeFirstResponder just call through | ||
| to it as we expect it will handle it */ \ | ||
| return [_view __sel]; \ | ||
| } else { \ | ||
| /* Call through to _ASDisplayView's superclass to get it handled */ \ | ||
| return [(_ASDisplayView *)_view __##__sel]; \ | ||
| } \ | ||
| } \ | ||
|
|
||
| - (void)checkResponderCompatibility | ||
| { | ||
| #if ASDISPLAYNODE_ASSERTIONS_ENABLED | ||
| // There are certain cases we cannot handle and are not supported: | ||
| // 1. If the _view class is not a subclass of _ASDisplayView | ||
| if (checkFlag(Synchronous)) { | ||
| // 2. At least one UIResponder methods are overwritten in the node subclass | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any concerns with touchesBegan:, touchesMoved:, touchesEnded:, or touchesCancelled: ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't look into them or saw any issues related to them. |
||
| NSString *message = @"Overwritting %@ and having a backing view that is not an _ASDisplayView is not supported."; | ||
| ASDisplayNodeAssert(!ASDisplayNodeSubclassOverridesSelector(self.class, @selector(canBecomeFirstResponder)), ([NSString stringWithFormat:message, @"canBecomeFirstResponder"])); | ||
| ASDisplayNodeAssert(!ASDisplayNodeSubclassOverridesSelector(self.class, @selector(becomeFirstResponder)), ([NSString stringWithFormat:message, @"becomeFirstResponder"])); | ||
| ASDisplayNodeAssert(!ASDisplayNodeSubclassOverridesSelector(self.class, @selector(canResignFirstResponder)), ([NSString stringWithFormat:message, @"canResignFirstResponder"])); | ||
| ASDisplayNodeAssert(!ASDisplayNodeSubclassOverridesSelector(self.class, @selector(resignFirstResponder)), ([NSString stringWithFormat:message, @"resignFirstResponder"])); | ||
| ASDisplayNodeAssert(!ASDisplayNodeSubclassOverridesSelector(self.class, @selector(isFirstResponder)), ([NSString stringWithFormat:message, @"isFirstResponder"])); | ||
| } | ||
| #endif | ||
| } | ||
|
|
||
| - (BOOL)__canBecomeFirstResponder | ||
| { | ||
| if (_view == nil) { | ||
| // By default we return NO if not view is created yet | ||
| return NO; | ||
| } | ||
|
|
||
| HANDLE_NODE_RESPONDER_METHOD(canBecomeFirstResponder); | ||
| } | ||
|
|
||
| - (BOOL)__becomeFirstResponder | ||
| { | ||
| if (![self canBecomeFirstResponder]) { | ||
| return NO; | ||
| } | ||
|
|
||
| // Note: This implicitly loads the view if it hasn't been loaded yet. | ||
| [self view]; | ||
|
|
||
| HANDLE_NODE_RESPONDER_METHOD(becomeFirstResponder); | ||
| } | ||
|
|
||
| - (BOOL)__canResignFirstResponder | ||
| { | ||
| if (_view == nil) { | ||
| // By default we return YES if no view is created yet | ||
| return YES; | ||
| } | ||
|
|
||
| HANDLE_NODE_RESPONDER_METHOD(canResignFirstResponder); | ||
| } | ||
|
|
||
| - (BOOL)__resignFirstResponder | ||
| { | ||
| if (![self canResignFirstResponder]) { | ||
| return NO; | ||
| } | ||
|
|
||
| // Note: This implicitly loads the view if it hasn't been loaded yet. | ||
| [self view]; | ||
|
|
||
| HANDLE_NODE_RESPONDER_METHOD(resignFirstResponder); | ||
| } | ||
|
|
||
| - (BOOL)__isFirstResponder | ||
| { | ||
| if (_view == nil) { | ||
| // If no view is created yet we can just return NO as it's unlikely it's the first responder | ||
| return NO; | ||
| } | ||
|
|
||
| HANDLE_NODE_RESPONDER_METHOD(isFirstResponder); | ||
| } | ||
|
|
||
| #pragma mark <ASDebugNameProvider> | ||
|
|
||
| - (NSString *)debugName | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,14 @@ NS_ASSUME_NONNULL_BEGIN | |
| - (void)__forwardTouchesEnded:(NSSet *)touches withEvent:(UIEvent *)event; | ||
| - (void)__forwardTouchesCancelled:(NSSet *)touches withEvent:(UIEvent *)event; | ||
|
|
||
| // These methods expose a way for ASDisplayNode responder methods to let the view call super responder methods | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This first sentence isn't quite clear -- "for ASDisplayNode responder methods to let the view call super responder methods". Clarify if possible, otherwise not exactly sure how to improve it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The sentence is kind of the same as the one for all of the forwarding touch events. In that case we should find a better way for both. |
||
| // They are called from ASDisplayNode to pass through UIResponder methods to the view | ||
| - (BOOL)__canBecomeFirstResponder; | ||
| - (BOOL)__becomeFirstResponder; | ||
| - (BOOL)__canResignFirstResponder; | ||
| - (BOOL)__resignFirstResponder; | ||
| - (BOOL)__isFirstResponder; | ||
|
|
||
| @end | ||
|
|
||
| NS_ASSUME_NONNULL_END | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -96,16 +96,6 @@ ASDISPLAYNODE_INLINE BOOL ASDisplayNodeShouldApplyBridgedWriteToView(ASDisplayNo | |
| */ | ||
| @implementation ASDisplayNode (UIViewBridge) | ||
|
|
||
| - (BOOL)canBecomeFirstResponder | ||
| { | ||
| return NO; | ||
| } | ||
|
|
||
| - (BOOL)canResignFirstResponder | ||
| { | ||
| return YES; | ||
| } | ||
|
|
||
| #if TARGET_OS_TV | ||
| // Focus Engine | ||
| - (BOOL)canBecomeFocused | ||
|
|
@@ -146,23 +136,34 @@ - (UIView *)preferredFocusedView | |
| } | ||
| #endif | ||
|
|
||
| - (BOOL)canBecomeFirstResponder | ||
| { | ||
| ASDisplayNodeAssertMainThread(); | ||
| return [self __canBecomeFirstResponder]; | ||
| } | ||
|
|
||
| - (BOOL)canResignFirstResponder | ||
| { | ||
| ASDisplayNodeAssertMainThread(); | ||
| return [self __canResignFirstResponder]; | ||
| } | ||
|
|
||
| - (BOOL)isFirstResponder | ||
| { | ||
| ASDisplayNodeAssertMainThread(); | ||
| return _view != nil && [_view isFirstResponder]; | ||
| return [self __isFirstResponder]; | ||
| } | ||
|
|
||
| // Note: this implicitly loads the view if it hasn't been loaded yet. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like a good comment to keep.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We had this kind of comment in I changed it to the old comment in there though. Let me know if I should also add it to this place back again. |
||
| - (BOOL)becomeFirstResponder | ||
| { | ||
| ASDisplayNodeAssertMainThread(); | ||
| return !self.layerBacked && [self canBecomeFirstResponder] && [self.view becomeFirstResponder]; | ||
| return [self __becomeFirstResponder]; | ||
| } | ||
|
|
||
| - (BOOL)resignFirstResponder | ||
| { | ||
| ASDisplayNodeAssertMainThread(); | ||
| return !self.layerBacked && [self canResignFirstResponder] && [_view resignFirstResponder]; | ||
| return [self __resignFirstResponder]; | ||
| } | ||
|
|
||
| - (BOOL)canPerformAction:(SEL)action withSender:(id)sender | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO this should go in another file, since ASDisplayNode.mm is quite large - it seems very appropriate for UIViewBridge or perhaps we could introduce another one.
Not a blocking change (feel free to land this if you're just eager to get it in!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are currently have the event handling
touchesBegan:etc. inASDisplayNode.mm, eventually we should move both of them out too in a follow up diff.