Majorly Improve automaticallyAdjustsContentOffset (#3033)

* Majorly improve automaticallyAdjustsContentOffset

* Remove nodes from ASDataControllerDelegate & ASRangeControllerDelegate

* Do it after -endUpdates

AsyncDisplayKit/ASCollectionView.mm

@@ -1697,7 +1697,7 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier";
   _performingBatchUpdates = NO;
 }
 
-- (void)rangeController:(ASRangeController *)rangeController didInsertNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
+- (void)rangeController:(ASRangeController *)rangeController didInsertItemsAtIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
 {
   ASDisplayNodeAssertMainThread();
   if (!self.asyncDataSource || _superIsPendingDataLoad) {
@@ -1719,7 +1719,7 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier";
   }
 }
 
-- (void)rangeController:(ASRangeController *)rangeController didDeleteNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
+- (void)rangeController:(ASRangeController *)rangeController didDeleteItemsAtIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
 {
   ASDisplayNodeAssertMainThread();
   if (!self.asyncDataSource || _superIsPendingDataLoad) {

AsyncDisplayKit/ASTableView.h

@@ -45,9 +45,11 @@ NS_ASSUME_NONNULL_BEGIN
 - (nullable ASCellNode *)nodeForRowAtIndexPath:(NSIndexPath *)indexPath AS_WARN_UNUSED_RESULT;
 
 /**
- * YES to automatically adjust the contentOffset when cells are inserted or deleted "before"
+ * YES to automatically adjust the contentOffset when cells are inserted or deleted above
- * visible cells, maintaining the users' visible scroll position. Currently this feature tracks insertions, moves and deletions of
+ * visible cells, maintaining the users' visible scroll position. 
- * cells, but section edits are ignored.
+ *
+ * @note This is only applied to non-animated updates. For animated updates, there is no way to
+ * synchronize or "cancel out" the appearance of a scroll due to UITableView API limitations.
  *
  * default is NO.
  */

AsyncDisplayKit/ASTableView.mm

@@ -128,8 +128,10 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
 
   NSIndexPath *_pendingVisibleIndexPath;
 
-  NSIndexPath *_contentOffsetAdjustmentTopVisibleRow;
+  // The top cell node that was visible before the update.
-  CGFloat _contentOffsetAdjustment;
+  __weak ASCellNode *_contentOffsetAdjustmentTopVisibleNode;
+  // The y-offset of the top visible row's origin before the update.
+  CGFloat _contentOffsetAdjustmentTopVisibleNodeOffset;
   
   CGPoint _deceleratingVelocity;
   
@@ -778,52 +780,40 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
 
 - (void)beginAdjustingContentOffset
 {
-  ASDisplayNodeAssert(_automaticallyAdjustsContentOffset, @"this method should only be called when _automaticallyAdjustsContentOffset == YES");
+  NSIndexPath *firstVisibleIndexPath = [self.indexPathsForVisibleRows sortedArrayUsingSelector:@selector(compare:)].firstObject;
-  _contentOffsetAdjustment = 0;
+  if (firstVisibleIndexPath) {
-  _contentOffsetAdjustmentTopVisibleRow = self.indexPathsForVisibleRows.firstObject;
+    ASCellNode *node = [self nodeForRowAtIndexPath:firstVisibleIndexPath];
-}
+    if (node) {
-
+      _contentOffsetAdjustmentTopVisibleNode = node;
-- (void)endAdjustingContentOffset
+      _contentOffsetAdjustmentTopVisibleNodeOffset = [self rectForRowAtIndexPath:firstVisibleIndexPath].origin.y - self.bounds.origin.y;
-{
-  ASDisplayNodeAssert(_automaticallyAdjustsContentOffset, @"this method should only be called when _automaticallyAdjustsContentOffset == YES");
-  if (_contentOffsetAdjustment != 0) {
-    self.contentOffset = CGPointMake(0, self.contentOffset.y+_contentOffsetAdjustment);
-  }
-
-  _contentOffsetAdjustment = 0;
-  _contentOffsetAdjustmentTopVisibleRow = nil;
-}
-
-- (void)adjustContentOffsetWithNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths inserting:(BOOL)inserting {
-  // Maintain the users visible window when inserting or deleting cells by adjusting the content offset for nodes
-  // before the visible area. If in a begin/end updates block this will update _contentOffsetAdjustment, otherwise it will
-  // update self.contentOffset directly.
-
-  ASDisplayNodeAssert(_automaticallyAdjustsContentOffset, @"this method should only be called when _automaticallyAdjustsContentOffset == YES");
-
-  CGFloat dir = (inserting) ? +1 : -1;
-  CGFloat adjustment = 0;
-  NSIndexPath *top = _contentOffsetAdjustmentTopVisibleRow ? : self.indexPathsForVisibleRows.firstObject;
-
-  for (int index = 0; index < indexPaths.count; index++) {
-    NSIndexPath *indexPath = indexPaths[index];
-    if ([indexPath compare:top] <= 0) { // if this row is before or equal to the topmost visible row, make adjustments...
-      ASCellNode *cellNode = nodes[index];
-      adjustment += cellNode.calculatedSize.height * dir;
-      if (indexPath.section == top.section) {
-        top = [NSIndexPath indexPathForRow:top.row+dir inSection:top.section];
-      }
     }
   }
-
-  if (_contentOffsetAdjustmentTopVisibleRow) { // true of we are in a begin/end update block (see beginAdjustingContentOffset)
-    _contentOffsetAdjustmentTopVisibleRow = top;
-    _contentOffsetAdjustment += adjustment;
-  } else if (adjustment != 0) {
-    self.contentOffset = CGPointMake(0, self.contentOffset.y+adjustment);
-  }
 }
 
+- (void)endAdjustingContentOffsetAnimated:(BOOL)animated
+{
+  // We can't do this for animated updates.
+  if (animated) {
+    return;
+  }
+  
+  // We can't do this if we didn't have a top visible row before.
+  if (_contentOffsetAdjustmentTopVisibleNode == nil) {
+    return;
+  }
+  
+  NSIndexPath *newIndexPathForTopVisibleRow = [self indexPathForNode:_contentOffsetAdjustmentTopVisibleNode];
+  // We can't do this if our top visible row was deleted
+  if (newIndexPathForTopVisibleRow == nil) {
+    return;
+  }
+  
+  CGFloat newRowOriginYInSelf = [self rectForRowAtIndexPath:newIndexPathForTopVisibleRow].origin.y - self.bounds.origin.y;
+  CGPoint newContentOffset = self.contentOffset;
+  newContentOffset.y += (newRowOriginYInSelf - _contentOffsetAdjustmentTopVisibleNodeOffset);
+  self.contentOffset = newContentOffset;
+  _contentOffsetAdjustmentTopVisibleNode = nil;
+}
 
 #pragma mark - Intercepted selectors
 
@@ -1428,22 +1418,23 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
     return; // if the asyncDataSource has become invalid while we are processing, ignore this request to avoid crashes
   }
 
-  if (_automaticallyAdjustsContentOffset) {
-    [self endAdjustingContentOffset];
-  }
-
   ASPerformBlockWithoutAnimation(!animated, ^{
     [super endUpdates];
     [_rangeController updateIfNeeded];
   });
 
   _performingBatchUpdates = NO;
+
+  if (_automaticallyAdjustsContentOffset) {
+    [self endAdjustingContentOffsetAnimated:animated];
+  }
+  
   if (completion) {
     completion(YES);
   }
 }
 
-- (void)rangeController:(ASRangeController *)rangeController didInsertNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
+- (void)rangeController:(ASRangeController *)rangeController didInsertItemsAtIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
 {
   ASDisplayNodeAssertMainThread();
   LOG(@"UITableView insertRows:%ld rows", indexPaths.count);
@@ -1463,13 +1454,9 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
     }
     [self _scheduleCheckForBatchFetchingForNumberOfChanges:indexPaths.count];
   });
-
-  if (_automaticallyAdjustsContentOffset) {
-    [self adjustContentOffsetWithNodes:nodes atIndexPaths:indexPaths inserting:YES];
-  }
 }
 
-- (void)rangeController:(ASRangeController *)rangeController didDeleteNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
+- (void)rangeController:(ASRangeController *)rangeController didDeleteItemsAtIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
 {
   ASDisplayNodeAssertMainThread();
   LOG(@"UITableView deleteRows:%ld rows", indexPaths.count);
@@ -1489,10 +1476,6 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
     }
     [self _scheduleCheckForBatchFetchingForNumberOfChanges:indexPaths.count];
   });
-
-  if (_automaticallyAdjustsContentOffset) {
-    [self adjustContentOffsetWithNodes:nodes atIndexPaths:indexPaths inserting:NO];
-  }
 }
 
 - (void)rangeController:(ASRangeController *)rangeController didInsertSectionsAtIndexSet:(NSIndexSet *)indexSet withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions

AsyncDisplayKit/Details/ASDataController.h

@@ -85,12 +85,12 @@ extern NSString * const ASCollectionInvalidUpdateException;
 /**
  Called for insertion of elements.
  */
-- (void)dataController:(ASDataController *)dataController didInsertNodes:(NSArray<ASCellNode *> *)nodes atIndexPaths:(NSArray<NSIndexPath *> *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions;
+- (void)dataController:(ASDataController *)dataController didInsertItemsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions;
 
 /**
  Called for deletion of elements.
  */
-- (void)dataController:(ASDataController *)dataController didDeleteNodes:(NSArray<ASCellNode *> *)nodes atIndexPaths:(NSArray<NSIndexPath *> *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions;
+- (void)dataController:(ASDataController *)dataController didDeleteItemsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions;
 
 /**
  Called for insertion of sections.

AsyncDisplayKit/Details/ASDataController.mm

@@ -59,11 +59,6 @@ NSString * const ASCollectionInvalidUpdateException = @"ASCollectionInvalidUpdat
   dispatch_group_t _editingTransactionGroup;     // Group of all edit transaction blocks. Useful for waiting.
   
   BOOL _initialReloadDataHasBeenCalled;
-
-  BOOL _delegateDidInsertNodes;
-  BOOL _delegateDidDeleteNodes;
-  BOOL _delegateDidInsertSections;
-  BOOL _delegateDidDeleteSections;
 }
 
 @end
@@ -110,21 +105,6 @@ NSString * const ASCollectionInvalidUpdateException = @"ASCollectionInvalidUpdat
   return [self initWithDataSource:fakeDataSource eventLog:eventLog];
 }
 
-- (void)setDelegate:(id<ASDataControllerDelegate>)delegate
-{
-  if (_delegate == delegate) {
-    return;
-  }
-  
-  _delegate = delegate;
-  
-  // Interrogate our delegate to understand its capabilities, optimizing away expensive respondsToSelector: calls later.
-  _delegateDidInsertNodes     = [_delegate respondsToSelector:@selector(dataController:didInsertNodes:atIndexPaths:withAnimationOptions:)];
-  _delegateDidDeleteNodes     = [_delegate respondsToSelector:@selector(dataController:didDeleteNodes:atIndexPaths:withAnimationOptions:)];
-  _delegateDidInsertSections  = [_delegate respondsToSelector:@selector(dataController:didInsertSections:atIndexSet:withAnimationOptions:)];
-  _delegateDidDeleteSections  = [_delegate respondsToSelector:@selector(dataController:didDeleteSectionsAtIndexSet:withAnimationOptions:)];
-}
-
 + (NSUInteger)parallelProcessorCount
 {
   static NSUInteger parallelProcessorCount;
@@ -349,8 +329,7 @@ NSString * const ASCollectionInvalidUpdateException = @"ASCollectionInvalidUpdat
   [self insertNodes:nodes ofKind:ASDataControllerRowNodeKind atIndexPaths:indexPaths completion:^(NSArray *nodes, NSArray *indexPaths) {
     ASDisplayNodeAssertMainThread();
     
-    if (_delegateDidInsertNodes)
+    [_delegate dataController:self didInsertItemsAtIndexPaths:indexPaths withAnimationOptions:animationOptions];
-      [_delegate dataController:self didInsertNodes:nodes atIndexPaths:indexPaths withAnimationOptions:animationOptions];
   }];
 }
 
@@ -367,8 +346,7 @@ NSString * const ASCollectionInvalidUpdateException = @"ASCollectionInvalidUpdat
   [self deleteNodesOfKind:ASDataControllerRowNodeKind atIndexPaths:indexPaths completion:^(NSArray *nodes, NSArray *indexPaths) {
     ASDisplayNodeAssertMainThread();
     
-    if (_delegateDidDeleteNodes)
+    [_delegate dataController:self didDeleteItemsAtIndexPaths:indexPaths withAnimationOptions:animationOptions];
-      [_delegate dataController:self didDeleteNodes:nodes atIndexPaths:indexPaths withAnimationOptions:animationOptions];
   }];
 }
 
@@ -385,8 +363,7 @@ NSString * const ASCollectionInvalidUpdateException = @"ASCollectionInvalidUpdat
   [self insertSections:sections ofKind:ASDataControllerRowNodeKind atIndexSet:indexSet completion:^(NSArray *sections, NSIndexSet *indexSet) {
     ASDisplayNodeAssertMainThread();
     
-    if (_delegateDidInsertSections)
+    [_delegate dataController:self didInsertSections:sections atIndexSet:indexSet withAnimationOptions:animationOptions];
-      [_delegate dataController:self didInsertSections:sections atIndexSet:indexSet withAnimationOptions:animationOptions];
   }];
 }
 
@@ -403,8 +380,7 @@ NSString * const ASCollectionInvalidUpdateException = @"ASCollectionInvalidUpdat
   [self deleteSections:indexSet completion:^() {
     ASDisplayNodeAssertMainThread();
     
-    if (_delegateDidDeleteSections)
+    [_delegate dataController:self didDeleteSectionsAtIndexSet:indexSet withAnimationOptions:animationOptions];
-      [_delegate dataController:self didDeleteSectionsAtIndexSet:indexSet withAnimationOptions:animationOptions];
   }];
 }
 

AsyncDisplayKit/Details/ASRangeController.h

@@ -167,26 +167,22 @@ AS_SUBCLASSING_RESTRICTED
  *
  * @param rangeController Sender.
  *
- * @param nodes Inserted nodes.
- *
  * @param indexPaths Index path of inserted nodes.
  *
  * @param animationOptions Animation options. See ASDataControllerAnimationOptions.
  */
-- (void)rangeController:(ASRangeController *)rangeController didInsertNodes:(NSArray<ASCellNode *> *)nodes atIndexPaths:(NSArray<NSIndexPath *> *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions;
+- (void)rangeController:(ASRangeController *)rangeController didInsertItemsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions;
 
 /**
  * Called for nodes deletion.
  *
  * @param rangeController Sender.
  *
- * @param nodes Deleted nodes.
- *
  * @param indexPaths Index path of deleted nodes.
  *
  * @param animationOptions Animation options. See ASDataControllerAnimationOptions.
  */
-- (void)rangeController:(ASRangeController *)rangeController didDeleteNodes:(NSArray<ASCellNode *> *)nodes atIndexPaths:(NSArray<NSIndexPath *> *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions;
+- (void)rangeController:(ASRangeController *)rangeController didDeleteItemsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions;
 
 /**
  * Called for section insertion.

AsyncDisplayKit/Details/ASRangeController.mm

@@ -500,19 +500,18 @@ static UIApplicationState __ApplicationState = UIApplicationStateActive;
   [_delegate rangeController:self didEndUpdatesAnimated:animated completion:completion];
 }
 
-- (void)dataController:(ASDataController *)dataController didInsertNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
+- (void)dataController:(ASDataController *)dataController didInsertItemsAtIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
 {
-  ASDisplayNodeAssert(nodes.count == indexPaths.count, @"Invalid index path");
   ASDisplayNodeAssertMainThread();
   _rangeIsValid = NO;
-  [_delegate rangeController:self didInsertNodes:nodes atIndexPaths:indexPaths withAnimationOptions:animationOptions];
+  [_delegate rangeController:self didInsertItemsAtIndexPaths:indexPaths withAnimationOptions:animationOptions];
 }
 
-- (void)dataController:(ASDataController *)dataController didDeleteNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
+- (void)dataController:(ASDataController *)dataController didDeleteItemsAtIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
 {
   ASDisplayNodeAssertMainThread();
   _rangeIsValid = NO;
-  [_delegate rangeController:self didDeleteNodes:nodes atIndexPaths:indexPaths withAnimationOptions:animationOptions];
+  [_delegate rangeController:self didDeleteItemsAtIndexPaths:indexPaths withAnimationOptions:animationOptions];
 }
 
 - (void)dataController:(ASDataController *)dataController didInsertSections:(NSArray *)sections atIndexSet:(NSIndexSet *)indexSet withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions

AsyncDisplayKitTests/ASTableViewTests.mm

@@ -797,6 +797,35 @@
   }
 }
 
+- (void)testAutomaticallyAdjustingContentOffset
+{
+  ASTableNode *node = [[ASTableNode alloc] initWithStyle:UITableViewStylePlain];
+  node.view.automaticallyAdjustsContentOffset = YES;
+  node.bounds = CGRectMake(0, 0, 100, 100);
+  ASTableViewFilledDataSource *ds = [[ASTableViewFilledDataSource alloc] init];
+  node.dataSource = ds;
+  
+  [node.view layoutIfNeeded];
+  [node waitUntilAllUpdatesAreCommitted];
+  CGFloat rowHeight = [node.view rectForRowAtIndexPath:[NSIndexPath indexPathForRow:0 inSection:0]].size.height;
+  // Scroll to row (0,1) + 10pt
+  node.view.contentOffset = CGPointMake(0, rowHeight + 10);
+  
+  [node performBatchAnimated:NO updates:^{
+    // Delete row 0 from all sections.
+    // This is silly but it's a consequence of how ASTableViewFilledDataSource is built.
+    ds.rowsPerSection -= 1;
+    for (NSInteger i = 0; i < NumberOfSections; i++) {
+      [node deleteRowsAtIndexPaths:@[ [NSIndexPath indexPathForItem:0 inSection:i]] withRowAnimation:UITableViewRowAnimationAutomatic];
+    }
+  } completion:nil];
+  [node waitUntilAllUpdatesAreCommitted];
+  
+  // Now that row (0,0) is deleted, we should have slid up to be at just 10
+  // i.e. we should have subtracted the deleted row height from our content offset.
+  XCTAssertEqual(node.view.contentOffset.y, 10);
+}
+
 @end
 
 @implementation UITableView (Testing)