diff --git a/AsyncDisplayKit/ASTableNode.mm b/AsyncDisplayKit/ASTableNode.mm index 6fd58035a7..bd52b31f06 100644 --- a/AsyncDisplayKit/ASTableNode.mm +++ b/AsyncDisplayKit/ASTableNode.mm @@ -388,19 +388,24 @@ ASEnvironmentCollectionTableSetEnvironmentState(_environmentStateLock) - (void)reloadDataInitiallyIfNeeded { + ASDisplayNodeAssertMainThread(); if (!self.dataController.initialReloadDataHasBeenCalled) { - [self reloadData]; + // Note: Just calling reloadData isn't enough here – we need to + // ensure that _nodesConstrainedWidth is updated first. + [self.view layoutIfNeeded]; } } - (NSInteger)numberOfRowsInSection:(NSInteger)section { + ASDisplayNodeAssertMainThread(); [self reloadDataInitiallyIfNeeded]; return [self.dataController numberOfRowsInSection:section]; } - (NSInteger)numberOfSections { + ASDisplayNodeAssertMainThread(); [self reloadDataInitiallyIfNeeded]; return [self.dataController numberOfSections]; } diff --git a/AsyncDisplayKit/ASTableView.mm b/AsyncDisplayKit/ASTableView.mm index 8a3334eac5..04d335c1c6 100644 --- a/AsyncDisplayKit/ASTableView.mm +++ b/AsyncDisplayKit/ASTableView.mm @@ -129,6 +129,10 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; BOOL _isDeallocating; BOOL _performingBatchUpdates; NSMutableSet *_cellsForVisibilityUpdates; + + // The section index overlay view, if there is one present. + // This is useful because we need to measure our row nodes against (width - indexView.width). + __weak UIView *_sectionIndexView; struct { unsigned int scrollViewDidScroll:1; @@ -181,6 +185,7 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; unsigned int tableNodeCanMoveRow:1; unsigned int tableViewMoveRow:1; unsigned int tableNodeMoveRow:1; + unsigned int sectionIndexMethods:1; // if both section index methods are implemented } _asyncDataSourceFlags; } @@ -321,6 +326,7 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; _asyncDataSourceFlags.tableNodeNodeBlockForRow = [_asyncDataSource respondsToSelector:@selector(tableNode:nodeBlockForRowAtIndexPath:)]; _asyncDataSourceFlags.tableViewCanMoveRow = [_asyncDataSource respondsToSelector:@selector(tableView:canMoveRowAtIndexPath:)]; _asyncDataSourceFlags.tableViewMoveRow = [_asyncDataSource respondsToSelector:@selector(tableView:moveRowAtIndexPath:toIndexPath:)]; + _asyncDataSourceFlags.sectionIndexMethods = [_asyncDataSource respondsToSelector:@selector(sectionIndexTitlesForTableView:)] && [_asyncDataSource respondsToSelector:@selector(tableView:sectionForSectionIndexTitle:atIndex:)]; ASDisplayNodeAssert(_asyncDataSourceFlags.tableViewNodeBlockForRow || _asyncDataSourceFlags.tableViewNodeForRow @@ -559,8 +565,9 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; - (void)layoutSubviews { - if (_nodesConstrainedWidth != self.bounds.size.width) { - _nodesConstrainedWidth = self.bounds.size.width; + CGFloat constrainedWidth = self.bounds.size.width - [self sectionIndexWidth]; + if (_nodesConstrainedWidth != constrainedWidth) { + _nodesConstrainedWidth = constrainedWidth; // First width change occurs during initial configuration. An expensive relayout pass is unnecessary at that time // and should be avoided, assuming that the initial data loading automatically runs shortly afterward. @@ -1512,10 +1519,15 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; // is the same for all cells (because there is no easy way to get that individual value before the node being assigned to a _ASTableViewCell). // Also, in many cases, some nodes may not need to be re-measured at all, such as when user enters and then immediately leaves editing mode. // To avoid premature optimization and making such assumption, as well as to keep ASTableView simple, re-measurement is strictly done on main. - [self beginUpdates]; + CGSize oldSize = node.bounds.size; const CGSize calculatedSize = [node layoutThatFits:constrainedSize].size; - node.frame = CGRectMake(0, 0, calculatedSize.width, calculatedSize.height); - [self endUpdates]; + node.frame = { .size = calculatedSize }; + + // If the node height changed, trigger a height requery. + if (oldSize.height != calculatedSize.height) { + [self beginUpdates]; + [self endUpdates]; + } } } @@ -1579,6 +1591,31 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; #pragma mark - Helper Methods +// Note: This is called every layout, and so it is very performance sensitive. +- (CGFloat)sectionIndexWidth +{ + // If they don't implement the methods, then there's no section index. + if (_asyncDataSourceFlags.sectionIndexMethods == NO) { + return 0; + } + + UIView *indexView = _sectionIndexView; + if (indexView.superview == self) { + return indexView.frame.size.width; + } + + CGRect bounds = self.bounds; + for (UIView *view in self.subviews) { + CGRect frame = view.frame; + // Section index is right-aligned and less than half-width. + if (CGRectGetMaxX(frame) == CGRectGetMaxX(bounds) && frame.size.width * 2 < bounds.size.width) { + _sectionIndexView = view; + return frame.size.width; + } + } + return 0; +} + /// @note This should be a UIKit index path. - (BOOL)isIndexPath:(NSIndexPath *)indexPath immediateSuccessorOfIndexPath:(NSIndexPath *)anchor { diff --git a/AsyncDisplayKit/ASTableViewInternal.h b/AsyncDisplayKit/ASTableViewInternal.h index 8b36014a52..bc3a6145d6 100644 --- a/AsyncDisplayKit/ASTableViewInternal.h +++ b/AsyncDisplayKit/ASTableViewInternal.h @@ -59,4 +59,7 @@ */ - (NSArray *)convertIndexPathsToTableNode:(NSArray *)indexPaths; +/// Returns the width of the section index view on the right-hand side of the table, if one is present. +- (CGFloat)sectionIndexWidth; + @end diff --git a/AsyncDisplayKitTests/ASTableViewTests.m b/AsyncDisplayKitTests/ASTableViewTests.m index 752a9f94d4..27e1b4198f 100644 --- a/AsyncDisplayKitTests/ASTableViewTests.m +++ b/AsyncDisplayKitTests/ASTableViewTests.m @@ -18,6 +18,7 @@ #import "ASTableNode.h" #import "ASTableView+Undeprecated.h" #import +#import "ASXCTExtensions.h" #define NumberOfSections 10 #define NumberOfRowsPerSection 20 @@ -117,10 +118,20 @@ @end @interface ASTableViewFilledDataSource : NSObject +@property (nonatomic) BOOL usesSectionIndex; @end @implementation ASTableViewFilledDataSource +- (BOOL)respondsToSelector:(SEL)aSelector +{ + if (aSelector == @selector(sectionIndexTitlesForTableView:) || aSelector == @selector(tableView:sectionForSectionIndexTitle:atIndex:)) { + return _usesSectionIndex; + } else { + return [super respondsToSelector:aSelector]; + } +} + - (NSInteger)numberOfSectionsInTableView:(UITableView *)tableView { return NumberOfSections; @@ -148,6 +159,16 @@ }; } +- (nullable NSArray *)sectionIndexTitlesForTableView:(UITableView *)tableView +{ + return @[ @"A", @"B", @"C" ]; +} + +- (NSInteger)tableView:(UITableView *)tableView sectionForSectionIndexTitle:(NSString *)title atIndex:(NSInteger)index +{ + return 0; +} + @end @interface ASTableViewFilledDelegate : NSObject @@ -614,6 +635,75 @@ [UITableView deswizzleAllInstanceMethods]; } +/** + * This tests an issue where, if the table is loaded before the first layout pass, + * the nodes are first measured with a constrained width of 0 which isn't ideal. + */ +- (void)testThatNodeConstrainedSizesAreCorrectIfReloadIsPreempted +{ + ASTableNode *node = [[ASTableNode alloc] initWithStyle:UITableViewStylePlain]; + + ASTableViewFilledDataSource *dataSource = [ASTableViewFilledDataSource new]; + CGFloat cellWidth = 320; + node.frame = CGRectMake(0, 0, cellWidth, 480); + + node.dataSource = dataSource; + node.delegate = dataSource; + + // Trigger data load BEFORE first layout pass, to ensure constrained size is correct. + XCTAssertGreaterThan(node.numberOfSections, 0); + [node waitUntilAllUpdatesAreCommitted]; + + ASSizeRange expectedSizeRange = ASSizeRangeMakeExactSize(CGSizeMake(cellWidth, 0)); + expectedSizeRange.max.height = CGFLOAT_MAX; + + for (NSInteger i = 0; i < node.numberOfSections; i++) { + for (NSInteger j = 0; j < [node numberOfRowsInSection:i]; j++) { + NSIndexPath *indexPath = [NSIndexPath indexPathForItem:j inSection:i]; + ASTestTextCellNode *cellNode = (id)[node nodeForRowAtIndexPath:indexPath]; + ASXCTAssertEqualSizeRanges(cellNode.constrainedSizeForCalculatedLayout, expectedSizeRange); + XCTAssertEqual(cellNode.numberOfLayoutsOnMainThread, 0); + } + } +} + +- (void)testSectionIndexHandling +{ + ASTableNode *node = [[ASTableNode alloc] initWithStyle:UITableViewStylePlain]; + + ASTableViewFilledDataSource *dataSource = [ASTableViewFilledDataSource new]; + dataSource.usesSectionIndex = YES; + node.frame = CGRectMake(0, 0, 320, 480); + + node.dataSource = dataSource; + node.delegate = dataSource; + + // Trigger data load + XCTAssertGreaterThan(node.numberOfSections, 0); + XCTAssertGreaterThan([node numberOfRowsInSection:0], 0); + [node waitUntilAllUpdatesAreCommitted]; + + UITableViewCell *cell = [node.view cellForRowAtIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]]; + XCTAssertNotNil(cell); + + CGFloat cellWidth = cell.contentView.frame.size.width; + XCTAssert(cellWidth > 0 && cellWidth < 320, @"Expected cell width to be about 305. Width: %@", @(cellWidth)); + + ASSizeRange expectedSizeRange = ASSizeRangeMakeExactSize(CGSizeMake(cellWidth, 0)); + expectedSizeRange.max.height = CGFLOAT_MAX; + + for (NSInteger i = 0; i < node.numberOfSections; i++) { + for (NSInteger j = 0; j < [node numberOfRowsInSection:i]; j++) { + NSIndexPath *indexPath = [NSIndexPath indexPathForItem:j inSection:i]; + ASTestTextCellNode *cellNode = (id)[node nodeForRowAtIndexPath:indexPath]; + ASXCTAssertEqualSizeRanges(cellNode.constrainedSizeForCalculatedLayout, expectedSizeRange); + // We will have to accept a relayout on main thread, since the index bar won't show + // up until some of the cells are inserted. + XCTAssertLessThanOrEqual(cellNode.numberOfLayoutsOnMainThread, 1); + } + } +} + @end @implementation UITableView (Testing) diff --git a/AsyncDisplayKitTests/ASXCTExtensions.h b/AsyncDisplayKitTests/ASXCTExtensions.h index a1a53dde1e..574b7a7175 100644 --- a/AsyncDisplayKitTests/ASXCTExtensions.h +++ b/AsyncDisplayKitTests/ASXCTExtensions.h @@ -31,3 +31,6 @@ #define ASXCTAssertNotEqualDimensions(r0, r1, ...) \ _XCTPrimitiveAssertNotEqualObjects(self, NSStringFromASDimension(r0), @#r0, NSStringFromASDimension(r1), @#r1, __VA_ARGS__) + +#define ASXCTAssertEqualSizeRanges(r0, r1, ...) \ + _XCTPrimitiveAssertEqualObjects(self, NSStringFromASSizeRange(r0), @#r0, NSStringFromASSizeRange(r1), @#r1, __VA_ARGS__)