Fix UIResponder handling with view backing ASDisplayNode (TextureGrou…

…p#789) * Add failing tests * Fix responder chain handling in Texture * Add mores tests that horrible fail * Add Changelog.md entry * Some fixes * Update logic * Add tests that prevents infinite loops if a custom view is overwriting UIResponder methods * Add macro to forward methods in ASDisplayNode * Add macro for forwarding responder methods in _ASDisplayView * Remove junk * Address first comments * Update _ASDisplayView to cache responder forwarding methods * Use XCTAssertEqual
mikezucc · Feb 23, 2018 · 236cdd7 · 236cdd7
1 parent 2618c50
commit 236cdd7
Show file tree

Hide file tree

Showing 8 changed files with 368 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,7 @@
 - Make `ASCellNode` tint color apply to table view cell accessories. [Vladyslav Chapaev](https://github.com/ShogunPhyched) [#764](https://github.com/TextureGroup/Texture/pull/764)
 - Fix ASTextNode2 is accessing backgroundColor off main while sizing / layout is happening. [Michael Schneider](https://github.com/maicki) [#794](https://github.com/TextureGroup/Texture/pull/778/)
 - Pass scrollViewWillEndDragging delegation through in ASIGListAdapterDataSource for IGListKit integration. [#796](https://github.com/TextureGroup/Texture/pull/796)
+- Fix UIResponder handling with view backing ASDisplayNode. [Michael Schneider](https://github.com/maicki) [#789] (https://github.com/TextureGroup/Texture/pull/789/)
 
 ## 2.6
 - [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon)

diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm
@@ -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
+    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

diff --git a/Source/Details/_ASDisplayView.h b/Source/Details/_ASDisplayView.h
@@ -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
+// 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
diff --git a/Source/Details/_ASDisplayView.mm b/Source/Details/_ASDisplayView.mm
@@ -23,9 +23,54 @@
 #import <AsyncDisplayKit/ASDisplayNodeInternal.h>
 #import <AsyncDisplayKit/ASDisplayNode+FrameworkPrivate.h>
 #import <AsyncDisplayKit/ASDisplayNode+Subclasses.h>
+#import <AsyncDisplayKit/ASInternalHelpers.h>
 #import <AsyncDisplayKit/ASObjectDescriptionHelpers.h>
 #import <AsyncDisplayKit/ASLayout.h>
 
+#pragma mark - _ASDisplayViewMethodOverrides
+
+typedef NS_OPTIONS(NSUInteger, _ASDisplayViewMethodOverrides)
+{
+  _ASDisplayViewMethodOverrideNone                          = 0,
+  _ASDisplayViewMethodOverrideCanBecomeFirstResponder       = 1 << 0,
+  _ASDisplayViewMethodOverrideBecomeFirstResponder          = 1 << 1,
+  _ASDisplayViewMethodOverrideCanResignFirstResponder       = 1 << 2,
+  _ASDisplayViewMethodOverrideResignFirstResponder          = 1 << 3,
+  _ASDisplayViewMethodOverrideIsFirstResponder              = 1 << 4,
+};
+
+/**
+ *  Returns _ASDisplayViewMethodOverrides for the given class
+ *
+ *  @param c the class, required.
+ *
+ *  @return _ASDisplayViewMethodOverrides.
+ */
+static _ASDisplayViewMethodOverrides GetASDisplayViewMethodOverrides(Class c)
+{
+  ASDisplayNodeCAssertNotNil(c, @"class is required");
+
+  _ASDisplayViewMethodOverrides overrides = _ASDisplayViewMethodOverrideNone;
+  if (ASSubclassOverridesSelector([_ASDisplayView class], c, @selector(canBecomeFirstResponder))) {
+    overrides |= _ASDisplayViewMethodOverrideCanBecomeFirstResponder;
+  }
+  if (ASSubclassOverridesSelector([_ASDisplayView class], c, @selector(becomeFirstResponder))) {
+    overrides |= _ASDisplayViewMethodOverrideBecomeFirstResponder;
+  }
+  if (ASSubclassOverridesSelector([_ASDisplayView class], c, @selector(canResignFirstResponder))) {
+    overrides |= _ASDisplayViewMethodOverrideCanResignFirstResponder;
+  }
+  if (ASSubclassOverridesSelector([_ASDisplayView class], c, @selector(resignFirstResponder))) {
+    overrides |= _ASDisplayViewMethodOverrideResignFirstResponder;
+  }
+  if (ASSubclassOverridesSelector([_ASDisplayView class], c, @selector(isFirstResponder))) {
+    overrides |= _ASDisplayViewMethodOverrideIsFirstResponder;
+  }
+  return overrides;
+}
+
+#pragma mark - _ASDisplayView
+
 @interface _ASDisplayView ()
 
 // Keep the node alive while its view is active.  If you create a view, add its layer to a layer hierarchy, then release
@@ -40,6 +85,21 @@ @implementation _ASDisplayView
 
   NSArray *_accessibleElements;
   CGRect _lastAccessibleElementsFrame;
+
+  _ASDisplayViewMethodOverrides _methodOverrides;
+}
+
+#pragma mark - Class
+
++ (void)initialize
+{
+  __unused Class initializeSelf = self;
+  IMP staticInitialize = imp_implementationWithBlock(^(_ASDisplayView *view) {
+    ASDisplayNodeAssert(view.class == initializeSelf, @"View class %@ does not have a matching _staticInitialize method; check to ensure [super initialize] is called within any custom +initialize implementations!  Overridden methods will not be called unless they are also implemented by superclass %@", view.class, initializeSelf);
+    view->_methodOverrides = GetASDisplayViewMethodOverrides(view.class);
+  });
+
+  class_replaceMethod(self, @selector(_staticInitialize), staticInitialize, "v:@");
 }
 
 + (Class)layerClass
@@ -49,6 +109,26 @@ + (Class)layerClass
 
 #pragma mark - NSObject Overrides
 
+- (instancetype)init
+{
+  if (!(self = [super init]))
+    return nil;
+
+  [self _initializeInstance];
+
+  return self;
+}
+
+- (void)_initializeInstance
+{
+  [self _staticInitialize];
+}
+
+- (void)_staticInitialize
+{
+  ASDisplayNodeAssert(NO, @"_staticInitialize must be overridden");
+}
+
 // e.g. <MYPhotoNodeView: 0xFFFFFF; node = <MYPhotoNode: 0xFFFFFE>; frame = ...>
 - (NSString *)description
 {
@@ -358,15 +438,41 @@ - (void)tintColorDidChange
   [node tintColorDidChange];
 }
 
-- (BOOL)canBecomeFirstResponder {
-  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
-  return [node canBecomeFirstResponder];
-}
-
-- (BOOL)canResignFirstResponder {
-  ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
-  return [node canResignFirstResponder];
-}
+#pragma mark UIResponder Handling
+
+#define IMPLEMENT_RESPONDER_METHOD(__sel, __methodOverride) \
+- (BOOL)__sel\
+{\
+  ASDisplayNode *node = _asyncdisplaykit_node; /* Create strong reference to weak ivar. */ \
+  SEL sel = @selector(__sel); \
+  /* Prevent an infinite loop in here if [super canBecomeFirstResponder] was called on a
+  / _ASDisplayView subclass */ \
+  if (self->_methodOverrides & __methodOverride) { \
+    /* Check if we can call through to ASDisplayNode subclass directly */ \
+    if (ASDisplayNodeSubclassOverridesSelector([node class], sel)) { \
+      return [node __sel]; \
+    } else { \
+    /* Call through to views superclass as we expect super was called from the
+      _ASDisplayView subclass and a node subclass does not overwrite canBecomeFirstResponder */ \
+      return [self __##__sel]; \
+    } \
+  } else { \
+    /* Call through to internal node __canBecomeFirstResponder that will consider the view in responding */ \
+    return [node __##__sel]; \
+  } \
+}\
+/* All __ prefixed methods are called from ASDisplayNode to let the view decide in what UIResponder state they \
+are not overridden by a ASDisplayNode subclass */ \
+- (BOOL)__##__sel \
+{ \
+  return [super __sel]; \
+} \
+
+IMPLEMENT_RESPONDER_METHOD(canBecomeFirstResponder, _ASDisplayViewMethodOverrideCanBecomeFirstResponder);
+IMPLEMENT_RESPONDER_METHOD(becomeFirstResponder, _ASDisplayViewMethodOverrideBecomeFirstResponder);
+IMPLEMENT_RESPONDER_METHOD(canResignFirstResponder, _ASDisplayViewMethodOverrideCanResignFirstResponder);
+IMPLEMENT_RESPONDER_METHOD(resignFirstResponder, _ASDisplayViewMethodOverrideResignFirstResponder);
+IMPLEMENT_RESPONDER_METHOD(isFirstResponder, _ASDisplayViewMethodOverrideIsFirstResponder);
 
 - (BOOL)canPerformAction:(SEL)action withSender:(id)sender
 {

diff --git a/Source/Private/ASDisplayNode+UIViewBridge.mm b/Source/Private/ASDisplayNode+UIViewBridge.mm
@@ -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.
 - (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

diff --git a/Source/Private/ASDisplayNodeInternal.h b/Source/Private/ASDisplayNodeInternal.h
@@ -275,6 +275,13 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo
 - (void)__incrementVisibilityNotificationsDisabled;
 - (void)__decrementVisibilityNotificationsDisabled;
 
+// Helper methods for UIResponder forwarding
+- (BOOL)__canBecomeFirstResponder;
+- (BOOL)__becomeFirstResponder;
+- (BOOL)__canResignFirstResponder;
+- (BOOL)__resignFirstResponder;
+- (BOOL)__isFirstResponder;
+
 /// Helper method to summarize whether or not the node run through the display process
 - (BOOL)_implementsDisplay;