Merge pull request #1365 from facebook/ASCellNodeVisibilitySync

[ASCellNode] Upgrades to ASCellNodeVisibilityEvent to ensure it is always synchronized with visibilityDidChange:
facebookarchive · Mar 11, 2016 · 6bd4a75 · 6bd4a75
2 parents c2b0670 + ff8ffff
commit 6bd4a75
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 31 deletions.
diff --git a/AsyncDisplayKit/ASCellNode+Internal.h b/AsyncDisplayKit/ASCellNode+Internal.h
@@ -32,4 +32,9 @@
  */
 @property (nonatomic, weak) id<ASCellNodeLayoutDelegate> layoutDelegate;
 
+/*
+ * Back-pointer to the containing scrollView instance, set only for visible cells.  Used for Cell Visibility Event callbacks.
+ */
+@property (nonatomic, weak) UIScrollView *scrollView;
+
 @end
diff --git a/AsyncDisplayKit/ASCellNode.m b/AsyncDisplayKit/ASCellNode.m
@@ -188,6 +188,20 @@ - (void)cellNodeVisibilityEvent:(ASCellNodeVisibilityEvent)event
     // To be overriden by subclasses
 }
 
+- (void)visibilityDidChange:(BOOL)isVisible
+{
+  [super visibilityDidChange:isVisible];
+
+  CGRect cellFrame = CGRectZero;
+  if (_scrollView) {
+    // It is not safe to message nil with a structure return value, so ensure our _scrollView has not died.
+    cellFrame = [self.view convertRect:self.bounds toView:_scrollView];
+  }
+  [self cellNodeVisibilityEvent:isVisible ? ASCellNodeVisibilityEventVisible : ASCellNodeVisibilityEventInvisible
+                   inScrollView:_scrollView
+                  withCellFrame:cellFrame];
+}
+
 @end
 
 

diff --git a/AsyncDisplayKit/ASCollectionView.mm b/AsyncDisplayKit/ASCollectionView.mm
@@ -532,39 +532,35 @@ - (UICollectionViewCell *)collectionView:(UICollectionView *)collectionView cell
 
 - (void)collectionView:(UICollectionView *)collectionView willDisplayCell:(_ASCollectionViewCell *)cell forItemAtIndexPath:(NSIndexPath *)indexPath
 {
-  [_rangeController visibleNodeIndexPathsDidChangeWithScrollDirection:self.scrollDirection];
+  ASCellNode *cellNode = [cell node];
+  cellNode.scrollView = collectionView;
 
   if ([_asyncDelegate respondsToSelector:@selector(collectionView:willDisplayNodeForItemAtIndexPath:)]) {
     [_asyncDelegate collectionView:self willDisplayNodeForItemAtIndexPath:indexPath];
   }
 
-  ASCellNode *cellNode = [cell node];
+  [_rangeController visibleNodeIndexPathsDidChangeWithScrollDirection:self.scrollDirection];
+
   if (cellNode.neverShowPlaceholders) {
     [cellNode recursivelyEnsureDisplaySynchronously:YES];
   }
   if (ASSubclassOverridesSelector([ASCellNode class], [cellNode class], @selector(cellNodeVisibilityEvent:inScrollView:withCellFrame:))) {
     [_cellsForVisibilityUpdates addObject:cell];
-    [cellNode cellNodeVisibilityEvent:ASCellNodeVisibilityEventVisible
-                         inScrollView:collectionView
-                        withCellFrame:cell.frame];
   }
 }
 
-- (void)collectionView:(UICollectionView *)collectionView didEndDisplayingCell:(UICollectionViewCell *)cell forItemAtIndexPath:(NSIndexPath *)indexPath
+- (void)collectionView:(UICollectionView *)collectionView didEndDisplayingCell:(_ASCollectionViewCell *)cell forItemAtIndexPath:(NSIndexPath *)indexPath
 {
   [_rangeController visibleNodeIndexPathsDidChangeWithScrollDirection:self.scrollDirection];
+
+  ASCellNode *cellNode = [cell node];
 
   if ([_asyncDelegate respondsToSelector:@selector(collectionView:didEndDisplayingNode:forItemAtIndexPath:)]) {
-    ASCellNode *node = ((_ASCollectionViewCell *)cell).node;
-    ASDisplayNodeAssertNotNil(node, @"Expected node associated with removed cell not to be nil.");
-    [_asyncDelegate collectionView:self didEndDisplayingNode:node forItemAtIndexPath:indexPath];
+    ASDisplayNodeAssertNotNil(cellNode, @"Expected node associated with removed cell not to be nil.");
+    [_asyncDelegate collectionView:self didEndDisplayingNode:cellNode forItemAtIndexPath:indexPath];
   }
 
   if ([_cellsForVisibilityUpdates containsObject:cell]) {
-    ASCellNode *node = ((_ASCollectionViewCell *)cell).node;
-    [node cellNodeVisibilityEvent:ASCellNodeVisibilityEventInvisible
-                     inScrollView:collectionView
-                    withCellFrame:cell.frame];
     [_cellsForVisibilityUpdates removeObject:cell];
   }
 
@@ -574,6 +570,8 @@ - (void)collectionView:(UICollectionView *)collectionView didEndDisplayingCell:(
     [_asyncDelegate collectionView:self didEndDisplayingNodeForItemAtIndexPath:indexPath];
   }
 #pragma clang diagnostic pop
+
+  cellNode.scrollView = nil;
 }
 
 #pragma mark -

diff --git a/AsyncDisplayKit/ASTableView.mm b/AsyncDisplayKit/ASTableView.mm
@@ -615,46 +615,42 @@ - (void)scrollViewDidScroll:(UIScrollView *)scrollView
 - (void)tableView:(UITableView *)tableView willDisplayCell:(_ASTableViewCell *)cell forRowAtIndexPath:(NSIndexPath *)indexPath
 {
   _pendingVisibleIndexPath = indexPath;
-
-  [_rangeController visibleNodeIndexPathsDidChangeWithScrollDirection:self.scrollDirection];
+
+  ASCellNode *cellNode = [cell node];
+  cellNode.scrollView = tableView;
 
   if ([_asyncDelegate respondsToSelector:@selector(tableView:willDisplayNodeForRowAtIndexPath:)]) {
     [_asyncDelegate tableView:self willDisplayNodeForRowAtIndexPath:indexPath];
   }
+
+  [_rangeController visibleNodeIndexPathsDidChangeWithScrollDirection:self.scrollDirection];
 
-  ASCellNode *cellNode = [cell node];
-
-  if (ASSubclassOverridesSelector([ASCellNode class], [cellNode class], @selector(cellNodeVisibilityEvent:inScrollView:withCellFrame:))) {
-    [_cellsForVisibilityUpdates addObject:cell];
-    [cellNode cellNodeVisibilityEvent:ASCellNodeVisibilityEventVisible
-                         inScrollView:tableView
-                        withCellFrame:cell.frame];
-  }
   if (cellNode.neverShowPlaceholders) {
     [cellNode recursivelyEnsureDisplaySynchronously:YES];
   }
+
+  if (ASSubclassOverridesSelector([ASCellNode class], [cellNode class], @selector(cellNodeVisibilityEvent:inScrollView:withCellFrame:))) {
+    [_cellsForVisibilityUpdates addObject:cell];
+  }
 }
 
 - (void)tableView:(UITableView *)tableView didEndDisplayingCell:(_ASTableViewCell *)cell forRowAtIndexPath:(NSIndexPath*)indexPath
 {
   if ([_pendingVisibleIndexPath isEqual:indexPath]) {
     _pendingVisibleIndexPath = nil;
   }
+
+  ASCellNode *cellNode = [cell node];
 
   [_rangeController visibleNodeIndexPathsDidChangeWithScrollDirection:self.scrollDirection];
 
   if ([_asyncDelegate respondsToSelector:@selector(tableView:didEndDisplayingNode:forRowAtIndexPath:)]) {
-    ASCellNode *node = ((_ASTableViewCell *)cell).node;
-    ASDisplayNodeAssertNotNil(node, @"Expected node associated with removed cell not to be nil.");
-    [_asyncDelegate tableView:self didEndDisplayingNode:node forRowAtIndexPath:indexPath];
+    ASDisplayNodeAssertNotNil(cellNode, @"Expected node associated with removed cell not to be nil.");
+    [_asyncDelegate tableView:self didEndDisplayingNode:cellNode forRowAtIndexPath:indexPath];
   }
 
   if ([_cellsForVisibilityUpdates containsObject:cell]) {
     [_cellsForVisibilityUpdates removeObject:cell];
-    ASCellNode *node = ((_ASTableViewCell *)cell).node;
-    [node cellNodeVisibilityEvent:ASCellNodeVisibilityEventInvisible
-                     inScrollView:tableView
-                    withCellFrame:cell.frame];
   }
 
 #pragma clang diagnostic push
@@ -663,6 +659,8 @@ - (void)tableView:(UITableView *)tableView didEndDisplayingCell:(_ASTableViewCel
     [_asyncDelegate tableView:self didEndDisplayingNodeForRowAtIndexPath:indexPath];
   }
 #pragma clang diagnostic pop
+
+  cellNode.scrollView = nil;
 }
 
 

diff --git a/AsyncDisplayKit/Details/ASRangeController.mm b/AsyncDisplayKit/Details/ASRangeController.mm
@@ -90,7 +90,8 @@ - (void)visibleNodeIndexPathsDidChangeWithScrollDirection:(ASScrollDirection)scr
 {
   _scrollDirection = scrollDirection;
 
-  [self scheduleRangeUpdate];
+  // Perform update immediately, so that cells receive a visibilityDidChange: call before their first pixel is visible.
+  [self performRangeUpdate];
 }
 
 - (void)updateCurrentRangeWithMode:(ASLayoutRangeMode)rangeMode