[ASCellNode] Fix selection / highlight implementation

AsyncDisplayKit/ASCellNode+Internal.h

@@ -12,7 +12,7 @@
 
 #import "ASCellNode.h"
 
-@protocol ASCellNodeLayoutDelegate <NSObject>
+@protocol ASCellNodeInteractionDelegate <NSObject>
 
 /**
  * Notifies the delegate that the specified cell node has done a relayout.
@@ -27,14 +27,19 @@
  */
 - (void)nodeDidRelayout:(ASCellNode *)node sizeChanged:(BOOL)sizeChanged;
 
+/*
+ * Methods to be called whenever the selection or highlight state changes
+ * on ASCellNode. UIKit internally stores these values to update reusable cells.
+ */
+
+- (void)nodeSelectedStateDidChange:(ASCellNode *)node;
+- (void)nodeHighlightedStateDidChange:(ASCellNode *)node;
+
 @end
 
 @interface ASCellNode ()
 
-/*
+@property (nonatomic, weak) id <ASCellNodeInteractionDelegate> interactionDelegate;
- * A delegate to be notified (on main thread) after a relayout.
- */
-@property (nonatomic, weak) id<ASCellNodeLayoutDelegate> layoutDelegate;
 
 /*
  * Back-pointer to the containing scrollView instance, set only for visible cells.  Used for Cell Visibility Event callbacks.

AsyncDisplayKit/ASCellNode.h

@@ -81,14 +81,16 @@ typedef NS_ENUM(NSUInteger, ASCellNodeVisibilityEvent) {
 @property (nonatomic) UITableViewCellSelectionStyle selectionStyle;
 
 /**
- * A Boolean value that indicates whether the node is selected.
+ * A Boolean value that is synchronized with the underlying collection or tableView cell property.
+ * Setting this value is equivalent to calling selectItem / deselectItem on the collection or table.
  */
-@property (nonatomic, assign) BOOL selected;
+@property (nonatomic, assign, getter=isSelected) BOOL selected;
 
 /**
- * A Boolean value that indicates whether the node is highlighted.
+ * A Boolean value that is synchronized with the underlying collection or tableView cell property.
+ * Setting this value is equivalent to calling highlightItem / unHighlightItem on the collection or table.
  */
-@property (nonatomic, assign) BOOL highlighted;
+@property (nonatomic, assign, getter=isHighlighted) BOOL highlighted;
 
 /*
  * ASCellNode must forward touch events in order for UITableView and UICollectionView tap handling to work. Overriding
@@ -148,6 +150,9 @@ typedef NS_ENUM(NSUInteger, ASCellNodeVisibilityEvent) {
  */
 @property (nonatomic, assign) UIEdgeInsets textInsets;
 
+- (BOOL)selected ASDISPLAYNODE_DEPRECATED;
+- (BOOL)highlighted ASDISPLAYNODE_DEPRECATED;
+
 @end
 
 NS_ASSUME_NONNULL_END

AsyncDisplayKit/ASCellNode.mm

@@ -36,7 +36,7 @@
 @end
 
 @implementation ASCellNode
-@synthesize layoutDelegate = _layoutDelegate;
+@synthesize interactionDelegate = _interactionDelegate;
 
 - (instancetype)init
 {
@@ -170,14 +170,40 @@
 
 - (void)didRelayoutFromOldSize:(CGSize)oldSize toNewSize:(CGSize)newSize
 {
-  if (_layoutDelegate != nil) {
+  if (_interactionDelegate != nil) {
     ASPerformBlockOnMainThread(^{
       BOOL sizeChanged = !CGSizeEqualToSize(oldSize, newSize);
-      [_layoutDelegate nodeDidRelayout:self sizeChanged:sizeChanged];
+      [_interactionDelegate nodeDidRelayout:self sizeChanged:sizeChanged];
     });
   }
 }
 
+- (void)setSelected:(BOOL)selected
+{
+  if (_selected != selected) {
+    _selected = selected;
+    [_interactionDelegate nodeSelectedStateDidChange:self];
+  }
+}
+
+- (void)setHighlighted:(BOOL)highlighted
+{
+  if (_highlighted != highlighted) {
+      _highlighted = highlighted;
+      [_interactionDelegate nodeHighlightedStateDidChange:self];
+    }
+}
+
+- (BOOL)selected
+{
+  return self.isSelected;
+}
+
+- (BOOL)highlighted
+{
+  return self.isSelected;
+}
+
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wobjc-missing-super-calls"
 

AsyncDisplayKit/ASCollectionView.mm

@@ -42,20 +42,39 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
 - (void)setNode:(ASCellNode *)node
 {
   _node = node;
+  if (node.selected != self.selected) {
     node.selected = self.selected;
+  }
+  if (node.highlighted != self.highlighted) {
     node.highlighted = self.highlighted;
+  }
 }
 
 - (void)setSelected:(BOOL)selected
 {
+  if (selected != self.selected) {
     [super setSelected:selected];
+  }
+  if (selected != _node.selected) {
     _node.selected = selected;
+  }
 }
 
 - (void)setHighlighted:(BOOL)highlighted
 {
+  if (highlighted != self.highlighted) {
     [super setHighlighted:highlighted];
+  }
+  if (highlighted != _node.highlighted) {
     _node.highlighted = highlighted;
+  }
+}
+
+- (void)prepareForReuse
+{
+  // Need to clear node pointer before UIKit calls setSelected:NO / setHighlighted:NO on its cells
+  self.node = nil;
+  [super prepareForReuse];
 }
 
 - (void)applyLayoutAttributes:(UICollectionViewLayoutAttributes *)layoutAttributes
@@ -98,7 +117,7 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
 #pragma mark -
 #pragma mark ASCollectionView.
 
-@interface ASCollectionView () <ASRangeControllerDataSource, ASRangeControllerDelegate, ASDataControllerSource, ASCellNodeLayoutDelegate, ASDelegateProxyInterceptor, ASBatchFetchingScrollView, ASDataControllerEnvironmentDelegate> {
+@interface ASCollectionView () <ASRangeControllerDataSource, ASRangeControllerDelegate, ASDataControllerSource, ASCellNodeInteractionDelegate, ASDelegateProxyInterceptor, ASBatchFetchingScrollView, ASDataControllerEnvironmentDelegate> {
   ASCollectionViewProxy *_proxyDataSource;
   ASCollectionViewProxy *_proxyDelegate;
   
@@ -864,8 +883,8 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
     return ^{
       __typeof__(self) strongSelf = weakSelf;
       [node enterHierarchyState:ASHierarchyStateRangeManaged];
-      if (node.layoutDelegate == nil) {
+      if (node.interactionDelegate == nil) {
-        node.layoutDelegate = strongSelf;
+        node.interactionDelegate = strongSelf;
       }
       return node;
     };
@@ -879,8 +898,8 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
 
     ASCellNode *node = block();
     [node enterHierarchyState:ASHierarchyStateRangeManaged];
-    if (node.layoutDelegate == nil) {
+    if (node.interactionDelegate == nil) {
-      node.layoutDelegate = strongSelf;
+      node.interactionDelegate = strongSelf;
     }
     return node;
   };
@@ -1127,6 +1146,25 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
 }
 
 #pragma mark - ASCellNodeDelegate
+- (void)nodeSelectedStateDidChange:(ASCellNode *)node
+{
+  NSIndexPath *indexPath = [self indexPathForNode:node];
+  if (indexPath) {
+    if (node.isSelected) {
+      [self selectItemAtIndexPath:indexPath animated:NO scrollPosition:UICollectionViewScrollPositionNone];
+    } else {
+      [self deselectItemAtIndexPath:indexPath animated:NO];
+    }
+  }
+}
+
+- (void)nodeHighlightedStateDidChange:(ASCellNode *)node
+{
+  NSIndexPath *indexPath = [self indexPathForNode:node];
+  if (indexPath) {
+    [self cellForItemAtIndexPath:indexPath].highlighted = node.isHighlighted;
+  }
+}
 
 - (void)nodeDidRelayout:(ASCellNode *)node sizeChanged:(BOOL)sizeChanged
 {

AsyncDisplayKit/ASTableView.mm

@@ -66,20 +66,39 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
 - (void)setNode:(ASCellNode *)node
 {
   _node = node;
+  if (node.selected != self.selected) {
     node.selected = self.selected;
+  }
+  if (node.highlighted != self.highlighted) {
     node.highlighted = self.highlighted;
+  }
 }
 
 - (void)setSelected:(BOOL)selected animated:(BOOL)animated
 {
+  if (selected != self.selected) {
     [super setSelected:selected animated:animated];
+  }
+  if (selected != _node.selected) {
     _node.selected = selected;
+  }
 }
 
 - (void)setHighlighted:(BOOL)highlighted animated:(BOOL)animated
 {
+  if (highlighted != self.highlighted) {
     [super setHighlighted:highlighted animated:animated];
+  }
+  if (highlighted != _node.highlighted) {
     _node.highlighted = highlighted;
+  }
+}
+
+- (void)prepareForReuse
+{
+  // Need to clear node pointer before UIKit calls setSelected:NO / setHighlighted:NO on its cells
+  self.node = nil;
+  [super prepareForReuse];
 }
 
 @end
@@ -91,7 +110,7 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
 - (instancetype)_initWithTableView:(ASTableView *)tableView;
 @end
 
-@interface ASTableView () <ASRangeControllerDataSource, ASRangeControllerDelegate, ASDataControllerSource, _ASTableViewCellDelegate, ASCellNodeLayoutDelegate, ASDelegateProxyInterceptor, ASBatchFetchingScrollView, ASDataControllerEnvironmentDelegate>
+@interface ASTableView () <ASRangeControllerDataSource, ASRangeControllerDelegate, ASDataControllerSource, _ASTableViewCellDelegate, ASCellNodeInteractionDelegate, ASDelegateProxyInterceptor, ASBatchFetchingScrollView, ASDataControllerEnvironmentDelegate>
 {
   ASTableViewProxy *_proxyDataSource;
   ASTableViewProxy *_proxyDelegate;
@@ -1046,8 +1065,8 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
     return ^{
       __typeof__(self) strongSelf = weakSelf;
       [node enterHierarchyState:ASHierarchyStateRangeManaged];
-      if (node.layoutDelegate == nil) {
+      if (node.interactionDelegate == nil) {
-        node.layoutDelegate = strongSelf;
+        node.interactionDelegate = strongSelf;
       }
       return node;
     };
@@ -1059,8 +1078,8 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
     __typeof__(self) strongSelf = weakSelf;
     ASCellNode *node = block();
     [node enterHierarchyState:ASHierarchyStateRangeManaged];
-    if (node.layoutDelegate == nil) {
+    if (node.interactionDelegate == nil) {
-      node.layoutDelegate = strongSelf;
+      node.interactionDelegate = strongSelf;
     }
     return node;
   };
@@ -1126,7 +1145,27 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
   }
 }
 
-#pragma mark - ASCellNodeLayoutDelegate
+#pragma mark - ASCellNodeDelegate
+
+- (void)nodeSelectedStateDidChange:(ASCellNode *)node
+{
+  NSIndexPath *indexPath = [self indexPathForNode:node];
+  if (indexPath) {
+    if (node.isSelected) {
+      [self selectRowAtIndexPath:indexPath animated:NO scrollPosition:UITableViewScrollPositionNone];
+    } else {
+      [self deselectRowAtIndexPath:indexPath animated:NO];
+    }
+  }
+}
+
+- (void)nodeHighlightedStateDidChange:(ASCellNode *)node
+{
+  NSIndexPath *indexPath = [self indexPathForNode:node];
+  if (indexPath) {
+    [self cellForRowAtIndexPath:indexPath].highlighted = node.isHighlighted;
+  }
+}
 
 - (void)nodeDidRelayout:(ASCellNode *)node sizeChanged:(BOOL)sizeChanged
 {

AsyncDisplayKitTests/ASCollectionViewTests.m

@@ -13,6 +13,22 @@
 #import "ASCollectionDataController.h"
 #import "ASCollectionViewFlowLayoutInspector.h"
 
+@interface ASTextCellNodeWithSetSelectedCounter : ASTextCellNode
+
+@property (nonatomic, assign) NSUInteger setSelectedCounter;
+
+@end
+
+@implementation ASTextCellNodeWithSetSelectedCounter
+
+- (void)setSelected:(BOOL)selected
+{
+  [super setSelected:selected];
+  _setSelectedCounter++;
+}
+
+@end
+
 @interface ASCollectionViewTestDelegate : NSObject <ASCollectionViewDataSource, ASCollectionViewDelegate>
 
 @property (nonatomic, assign) NSInteger numberOfSections;
@@ -32,7 +48,7 @@
 }
 
 - (ASCellNode *)collectionView:(ASCollectionView *)collectionView nodeForItemAtIndexPath:(NSIndexPath *)indexPath {
-  ASTextCellNode *textCellNode = [ASTextCellNode new];
+  ASTextCellNodeWithSetSelectedCounter *textCellNode = [ASTextCellNodeWithSetSelectedCounter new];
   textCellNode.text = indexPath.description;
 
   return textCellNode;
@@ -41,7 +57,7 @@
 
 - (ASCellNodeBlock)collectionView:(ASCollectionView *)collectionView nodeBlockForItemAtIndexPath:(NSIndexPath *)indexPath {
   return ^{
-    ASTextCellNode *textCellNode = [ASTextCellNode new];
+    ASTextCellNodeWithSetSelectedCounter *textCellNode = [ASTextCellNodeWithSetSelectedCounter new];
     textCellNode.text = indexPath.description;
     return textCellNode;
   };
@@ -134,4 +150,54 @@
   [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:1.0]];
 }
 
+- (void)testSelection
+{
+  ASCollectionViewTestController *testController = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil];
+  UIWindow *window = [[UIWindow alloc] initWithFrame:[[UIScreen mainScreen] bounds]];
+  [window setRootViewController:testController];
+  [window makeKeyAndVisible];
+  
+  [testController.collectionView reloadDataImmediately];
+  [testController.collectionView layoutIfNeeded];
+  
+  NSIndexPath *indexPath = [NSIndexPath indexPathForItem:0 inSection:0];
+  ASCellNode *node = [testController.collectionView nodeForItemAtIndexPath:indexPath];
+  
+  // selecting node should select cell
+  node.selected = YES;
+  XCTAssertTrue([[testController.collectionView indexPathsForSelectedItems] containsObject:indexPath], @"Selecting node should update cell selection.");
+  
+  // deselecting node should deselect cell
+  node.selected = NO;
+  XCTAssertTrue([[testController.collectionView indexPathsForSelectedItems] isEqualToArray:@[]], @"Deselecting node should update cell selection.");
+
+  // selecting cell via collectionView should select node
+  [testController.collectionView selectItemAtIndexPath:indexPath animated:NO scrollPosition:UICollectionViewScrollPositionNone];
+  XCTAssertTrue(node.isSelected == YES, @"Selecting cell should update node selection.");
+  
+  // deselecting cell via collectionView should deselect node
+  [testController.collectionView deselectItemAtIndexPath:indexPath animated:NO];
+  XCTAssertTrue(node.isSelected == NO, @"Deselecting cell should update node selection.");
+  
+  // selecting cell should select node
+  UICollectionViewCell *cell = [testController.collectionView cellForItemAtIndexPath:indexPath];
+  cell.selected = YES;
+  XCTAssertTrue(node.isSelected == YES, @"Selecting cell should update node selection.");
+  
+  // reload cell (-prepareForReuse is called) & check that selected state is preserved
+  [testController.collectionView setContentOffset:CGPointMake(0,testController.collectionView.bounds.size.height)];
+  [testController.collectionView layoutIfNeeded];
+  [testController.collectionView setContentOffset:CGPointMake(0,0)];
+  [testController.collectionView layoutIfNeeded];
+  XCTAssertTrue(node.isSelected == YES, @"Reloaded cell should preserve state.");
+  
+  // deselecting cell should deselect node
+  cell = [testController.collectionView cellForItemAtIndexPath:indexPath];
+  cell.selected = NO;
+  XCTAssertTrue(node.isSelected == NO, @"Deselecting cell should update node selection.");
+  
+  // check setSelected not called extra times
+  XCTAssertTrue([(ASTextCellNodeWithSetSelectedCounter *)node setSelectedCounter] == 6, @"setSelected: should not be called on node multiple times.");
+}
+
 @end