Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions AsyncDisplayKit/ASCollectionView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1696,7 +1696,7 @@ - (void)rangeController:(ASRangeController *)rangeController didEndUpdatesAnimat
_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) {
Expand All @@ -1718,7 +1718,7 @@ - (void)rangeController:(ASRangeController *)rangeController didInsertNodes:(NSA
}
}

- (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) {
Expand Down
8 changes: 5 additions & 3 deletions AsyncDisplayKit/ASTableView.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
* visible cells, maintaining the users' visible scroll position. Currently this feature tracks insertions, moves and deletions of
* cells, but section edits are ignored.
* YES to automatically adjust the contentOffset when cells are inserted or deleted above
* visible cells, maintaining the users' visible scroll position.
*
* @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.
*/
Expand Down
93 changes: 38 additions & 55 deletions AsyncDisplayKit/ASTableView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,10 @@ @interface ASTableView () <ASRangeControllerDataSource, ASRangeControllerDelegat

NSIndexPath *_pendingVisibleIndexPath;

NSIndexPath *_contentOffsetAdjustmentTopVisibleRow;
CGFloat _contentOffsetAdjustment;
// The top cell node that was visible before the update.
__weak ASCellNode *_contentOffsetAdjustmentTopVisibleNode;
// The y-offset of the top visible row's origin before the update.
CGFloat _contentOffsetAdjustmentTopVisibleNodeOffset;

CGPoint _deceleratingVelocity;

Expand Down Expand Up @@ -778,53 +780,41 @@ - (void)moveRowAtIndexPath:(NSIndexPath *)indexPath toIndexPath:(NSIndexPath *)n

- (void)beginAdjustingContentOffset
{
ASDisplayNodeAssert(_automaticallyAdjustsContentOffset, @"this method should only be called when _automaticallyAdjustsContentOffset == YES");
_contentOffsetAdjustment = 0;
_contentOffsetAdjustmentTopVisibleRow = self.indexPathsForVisibleRows.firstObject;
NSIndexPath *firstVisibleIndexPath = [self.indexPathsForVisibleRows sortedArrayUsingSelector:@selector(compare:)].firstObject;
if (firstVisibleIndexPath) {
ASCellNode *node = [self nodeForRowAtIndexPath:firstVisibleIndexPath];
if (node) {
_contentOffsetAdjustmentTopVisibleNode = node;
_contentOffsetAdjustmentTopVisibleNodeOffset = [self rectForRowAtIndexPath:firstVisibleIndexPath].origin.y - self.bounds.origin.y;
}
}
}

- (void)endAdjustingContentOffset
- (void)endAdjustingContentOffsetAnimated:(BOOL)animated
{
ASDisplayNodeAssert(_automaticallyAdjustsContentOffset, @"this method should only be called when _automaticallyAdjustsContentOffset == YES");
if (_contentOffsetAdjustment != 0) {
self.contentOffset = CGPointMake(0, self.contentOffset.y+_contentOffsetAdjustment);
// We can't do this for animated updates.
if (animated) {
return;
}

_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];
}
}

// We can't do this if we didn't have a top visible row before.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great that this edge case is handled.

Non-blocking feedback: I'm wondering if we should store 1 or maybe even 2 more nodes? That way we would rarely give up because all of the anchor nodes were gone. The last visible node and the one in the middle are good candidates for this purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! For the time being, let's leave it as-is to reduce complexity since cases where they delete the anchor row will be somewhat rare, and I have a suspicion that not many people are using this flag.

if (_contentOffsetAdjustmentTopVisibleNode == nil) {
return;
}

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);

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

- (void)setTableHeaderView:(UIView *)tableHeaderView
Expand Down Expand Up @@ -1427,22 +1417,23 @@ - (void)rangeController:(ASRangeController *)rangeController didEndUpdatesAnimat
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);
Expand All @@ -1462,13 +1453,9 @@ - (void)rangeController:(ASRangeController *)rangeController didInsertNodes:(NSA
}
[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);
Expand All @@ -1488,10 +1475,6 @@ - (void)rangeController:(ASRangeController *)rangeController didDeleteNodes:(NSA
}
[self _scheduleCheckForBatchFetchingForNumberOfChanges:indexPaths.count];
});

if (_automaticallyAdjustsContentOffset) {
[self adjustContentOffsetWithNodes:nodes atIndexPaths:indexPaths inserting:NO];
}
}

- (void)rangeController:(ASRangeController *)rangeController didInsertSectionsAtIndexSet:(NSIndexSet *)indexSet withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
Expand Down
4 changes: 2 additions & 2 deletions AsyncDisplayKit/Details/ASDataController.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
32 changes: 4 additions & 28 deletions AsyncDisplayKit/Details/ASDataController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,6 @@ @interface ASDataController () {
dispatch_group_t _editingTransactionGroup; // Group of all edit transaction blocks. Useful for waiting.

BOOL _initialReloadDataHasBeenCalled;

BOOL _delegateDidInsertNodes;
BOOL _delegateDidDeleteNodes;
BOOL _delegateDidInsertSections;
BOOL _delegateDidDeleteSections;
}

@end
Expand Down Expand Up @@ -110,21 +105,6 @@ - (instancetype)init
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;
Expand Down Expand Up @@ -349,8 +329,7 @@ - (void)_insertNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAni
[self insertNodes:nodes ofKind:ASDataControllerRowNodeKind atIndexPaths:indexPaths completion:^(NSArray *nodes, NSArray *indexPaths) {
ASDisplayNodeAssertMainThread();

if (_delegateDidInsertNodes)
[_delegate dataController:self didInsertNodes:nodes atIndexPaths:indexPaths withAnimationOptions:animationOptions];
[_delegate dataController:self didInsertItemsAtIndexPaths:indexPaths withAnimationOptions:animationOptions];
}];
}

Expand All @@ -367,8 +346,7 @@ - (void)_deleteNodesAtIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASD
[self deleteNodesOfKind:ASDataControllerRowNodeKind atIndexPaths:indexPaths completion:^(NSArray *nodes, NSArray *indexPaths) {
ASDisplayNodeAssertMainThread();

if (_delegateDidDeleteNodes)
[_delegate dataController:self didDeleteNodes:nodes atIndexPaths:indexPaths withAnimationOptions:animationOptions];
[_delegate dataController:self didDeleteItemsAtIndexPaths:indexPaths withAnimationOptions:animationOptions];
}];
}

Expand All @@ -385,8 +363,7 @@ - (void)_insertSections:(NSMutableArray *)sections atIndexSet:(NSIndexSet *)inde
[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];
}];
}

Expand All @@ -403,8 +380,7 @@ - (void)_deleteSectionsAtIndexSet:(NSIndexSet *)indexSet withAnimationOptions:(A
[self deleteSections:indexSet completion:^() {
ASDisplayNodeAssertMainThread();

if (_delegateDidDeleteSections)
[_delegate dataController:self didDeleteSectionsAtIndexSet:indexSet withAnimationOptions:animationOptions];
[_delegate dataController:self didDeleteSectionsAtIndexSet:indexSet withAnimationOptions:animationOptions];
}];
}

Expand Down
8 changes: 2 additions & 6 deletions AsyncDisplayKit/Details/ASRangeController.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 4 additions & 5 deletions AsyncDisplayKit/Details/ASRangeController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -500,19 +500,18 @@ - (void)dataController:(ASDataController *)dataController endUpdatesAnimated:(BO
[_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
Expand Down
29 changes: 29 additions & 0 deletions AsyncDisplayKitTests/ASTableViewTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,35 @@ - (void)testThatInvalidUpdateExceptionReasonContainsDataSourceClassName
}
}

- (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)
Expand Down