From 4fc753a4585d5138a313dc5a7cd02f96a1f992a8 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Fri, 3 Nov 2017 16:24:48 -0700 Subject: [PATCH] Make it possible to map between sections even if they're empty (#660) * Make section mapping work even for empty sections * Unlock more cases and update changelog * Fix complexity documentation --- CHANGELOG.md | 1 + Source/ASCollectionView.mm | 59 +++++++++++----------------- Source/ASTableView.mm | 24 +++-------- Source/Details/ASDataController.mm | 29 ++++++-------- Source/Details/ASElementMap.h | 12 +++++- Source/Details/ASElementMap.m | 23 ++++++++++- Source/Private/ASMutableElementMap.h | 4 +- Source/Private/ASMutableElementMap.m | 4 +- Source/Private/ASSection.h | 22 +++++++++-- 9 files changed, 95 insertions(+), 83 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3e86c4e14..1e4f027f9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - [Layout] Fixes a deadlock in layout. [#638](https://github.com/TextureGroup/Texture/pull/638) [Garrett Moon](https://github.com/garrettmoon) - Updated to be backwards compatible with Xcode 8. [Adlai Holler](https://github.com/Adlai-Holler) - [API CHANGES] `ASPerformMainThreadDeallocation` and `ASPerformBackgroundDeallocation` functions take `id *` instead of `id` and they're now more reliable. Also, in Swift, `ASDeallocQueue.sharedDeallocationQueue() -> ASDeallocQueue.sharedDeallocationQueue`. [Adlai Holler](https://github.com/Adlai-Holler) [#651](https://github.com/TextureGroup/Texture/pull/651) +- [Collection/Table] Added direct support for mapping section indexes between data spaces. [Adlai Holler](https://github.com/Adlai-Holler) [#651](https://github.com/TextureGroup/Texture/pull/660) ## 2.6 - [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon) diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm index d3857ae451..c1751baf7a 100644 --- a/Source/ASCollectionView.mm +++ b/Source/ASCollectionView.mm @@ -64,8 +64,6 @@ return __val; \ } -#define ASIndexPathForSection(section) [NSIndexPath indexPathForItem:0 inSection:section] - #define ASFlowLayoutDefault(layout, property, default) \ ({ \ UICollectionViewFlowLayout *flowLayout = ASDynamicCast(layout, UICollectionViewFlowLayout); \ @@ -679,19 +677,13 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier"; if (indexPath == nil) { return nil; } - - // If this is a section index path, we don't currently have a method - // to do a mapping. - if (indexPath.item == NSNotFound) { - return indexPath; - } else { - NSIndexPath *viewIndexPath = [_dataController.visibleMap convertIndexPath:indexPath fromMap:_dataController.pendingMap]; - if (viewIndexPath == nil && wait) { - [self waitUntilAllUpdatesAreCommitted]; - return [self convertIndexPathFromCollectionNode:indexPath waitingIfNeeded:NO]; - } - return viewIndexPath; + + NSIndexPath *viewIndexPath = [_dataController.visibleMap convertIndexPath:indexPath fromMap:_dataController.pendingMap]; + if (viewIndexPath == nil && wait) { + [self waitUntilAllUpdatesAreCommitted]; + return [self convertIndexPathFromCollectionNode:indexPath waitingIfNeeded:NO]; } + return viewIndexPath; } /** @@ -725,13 +717,7 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier"; return nil; } - // If this is a section index path, we don't currently have a method - // to do a mapping. - if (indexPath.item == NSNotFound) { - return indexPath; - } else { - return [_dataController.pendingMap convertIndexPath:indexPath fromMap:_dataController.visibleMap]; - } + return [_dataController.pendingMap convertIndexPath:indexPath fromMap:_dataController.visibleMap]; } - (NSArray *)convertIndexPathsToCollectionNode:(NSArray *)indexPaths @@ -1050,7 +1036,7 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier"; ASDisplayNodeAssertMainThread(); ASElementMap *map = _dataController.visibleMap; ASCollectionElement *e = [map supplementaryElementOfKind:UICollectionElementKindSectionHeader - atIndexPath:ASIndexPathForSection(section)]; + atIndexPath:[NSIndexPath indexPathForItem:0 inSection:section]]; return e ? [self sizeForElement:e] : ASFlowLayoutDefault(l, headerReferenceSize, CGSizeZero); } @@ -1060,29 +1046,28 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier"; ASDisplayNodeAssertMainThread(); ASElementMap *map = _dataController.visibleMap; ASCollectionElement *e = [map supplementaryElementOfKind:UICollectionElementKindSectionFooter - atIndexPath:ASIndexPathForSection(section)]; + atIndexPath:[NSIndexPath indexPathForItem:0 inSection:section]]; return e ? [self sizeForElement:e] : ASFlowLayoutDefault(l, footerReferenceSize, CGSizeZero); } // For the methods that call delegateIndexPathForSection:withSelector:, translate the section from // visibleMap to pendingMap. If the section no longer exists, or the delegate doesn't implement -// the selector, we will return a nil indexPath (and then use the ASFlowLayoutDefault). -- (NSIndexPath *)delegateIndexPathForSection:(NSInteger)section withSelector:(SEL)selector +// the selector, we will return NSNotFound (and then use the ASFlowLayoutDefault). +- (NSInteger)delegateIndexForSection:(NSInteger)section withSelector:(SEL)selector { if ([_asyncDelegate respondsToSelector:selector]) { - return [_dataController.pendingMap convertIndexPath:ASIndexPathForSection(section) - fromMap:_dataController.visibleMap]; + return [_dataController.pendingMap convertSection:section fromMap:_dataController.visibleMap]; } else { - return nil; + return NSNotFound; } } - (UIEdgeInsets)collectionView:(UICollectionView *)cv layout:(UICollectionViewLayout *)l insetForSectionAtIndex:(NSInteger)section { - NSIndexPath *indexPath = [self delegateIndexPathForSection:section withSelector:_cmd]; - if (indexPath) { - return [(id)_asyncDelegate collectionView:cv layout:l insetForSectionAtIndex:indexPath.section]; + section = [self delegateIndexForSection:section withSelector:_cmd]; + if (section != NSNotFound) { + return [(id)_asyncDelegate collectionView:cv layout:l insetForSectionAtIndex:section]; } return ASFlowLayoutDefault(l, sectionInset, UIEdgeInsetsZero); } @@ -1090,10 +1075,10 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier"; - (CGFloat)collectionView:(UICollectionView *)cv layout:(UICollectionViewLayout *)l minimumInteritemSpacingForSectionAtIndex:(NSInteger)section { - NSIndexPath *indexPath = [self delegateIndexPathForSection:section withSelector:_cmd]; - if (indexPath) { + section = [self delegateIndexForSection:section withSelector:_cmd]; + if (section != NSNotFound) { return [(id)_asyncDelegate collectionView:cv layout:l - minimumInteritemSpacingForSectionAtIndex:indexPath.section]; + minimumInteritemSpacingForSectionAtIndex:section]; } return ASFlowLayoutDefault(l, minimumInteritemSpacing, 10.0); // Default is documented as 10.0 } @@ -1101,10 +1086,10 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier"; - (CGFloat)collectionView:(UICollectionView *)cv layout:(UICollectionViewLayout *)l minimumLineSpacingForSectionAtIndex:(NSInteger)section { - NSIndexPath *indexPath = [self delegateIndexPathForSection:section withSelector:_cmd]; - if (indexPath) { + section = [self delegateIndexForSection:section withSelector:_cmd]; + if (section != NSNotFound) { return [(id)_asyncDelegate collectionView:cv layout:l - minimumLineSpacingForSectionAtIndex:indexPath.section]; + minimumLineSpacingForSectionAtIndex:section]; } return ASFlowLayoutDefault(l, minimumLineSpacing, 10.0); // Default is documented as 10.0 } diff --git a/Source/ASTableView.mm b/Source/ASTableView.mm index 594d6a9966..4d9d01f81b 100644 --- a/Source/ASTableView.mm +++ b/Source/ASTableView.mm @@ -588,18 +588,12 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; - (NSIndexPath *)convertIndexPathFromTableNode:(NSIndexPath *)indexPath waitingIfNeeded:(BOOL)wait { - // If this is a section index path, we don't currently have a method - // to do a mapping. - if (indexPath == nil || indexPath.row == NSNotFound) { - return indexPath; - } else { - NSIndexPath *viewIndexPath = [_dataController.visibleMap convertIndexPath:indexPath fromMap:_dataController.pendingMap]; - if (viewIndexPath == nil && wait) { - [self waitUntilAllUpdatesAreCommitted]; - return [self convertIndexPathFromTableNode:indexPath waitingIfNeeded:NO]; - } - return viewIndexPath; + NSIndexPath *viewIndexPath = [_dataController.visibleMap convertIndexPath:indexPath fromMap:_dataController.pendingMap]; + if (viewIndexPath == nil && wait) { + [self waitUntilAllUpdatesAreCommitted]; + return [self convertIndexPathFromTableNode:indexPath waitingIfNeeded:NO]; } + return viewIndexPath; } - (NSIndexPath *)convertIndexPathToTableNode:(NSIndexPath *)indexPath @@ -608,13 +602,7 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; return nil; } - // If this is a section index path, we don't currently have a method - // to do a mapping. - if (indexPath.row == NSNotFound) { - return indexPath; - } else { - return [_dataController.pendingMap convertIndexPath:indexPath fromMap:_dataController.visibleMap]; - } + return [_dataController.pendingMap convertIndexPath:indexPath fromMap:_dataController.visibleMap]; } - (NSArray *)convertIndexPathsToTableNode:(NSArray *)indexPaths diff --git a/Source/Details/ASDataController.mm b/Source/Details/ASDataController.mm index c8b489e295..51f35d49a4 100644 --- a/Source/Details/ASDataController.mm +++ b/Source/Details/ASDataController.mm @@ -540,7 +540,7 @@ typedef dispatch_block_t ASDataControllerCompletionBlock; ASMutableElementMap *mutableMap = [previousMap mutableCopy]; // Step 1.1: Update the mutable copies to match the data source's state - [self _updateSectionContextsInMap:mutableMap changeSet:changeSet]; + [self _updateSectionsInMap:mutableMap changeSet:changeSet]; ASPrimitiveTraitCollection existingTraitCollection = [self.node primitiveTraitCollection]; [self _updateElementsInMap:mutableMap changeSet:changeSet traitCollection:existingTraitCollection shouldFetchSizeRanges:(! canDelegate) previousMap:previousMap]; @@ -599,43 +599,38 @@ typedef dispatch_block_t ASDataControllerCompletionBlock; /** * Update sections based on the given change set. */ -- (void)_updateSectionContextsInMap:(ASMutableElementMap *)map changeSet:(_ASHierarchyChangeSet *)changeSet +- (void)_updateSectionsInMap:(ASMutableElementMap *)map changeSet:(_ASHierarchyChangeSet *)changeSet { ASDisplayNodeAssertMainThread(); - if (!_dataSourceFlags.contextForSection) { - return; - } - if (changeSet.includesReloadData) { - [map removeAllSectionContexts]; + [map removeAllSections]; NSUInteger sectionCount = [self itemCountsFromDataSource].size(); NSIndexSet *sectionIndexes = [NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, sectionCount)]; - [self _insertSectionContextsIntoMap:map indexes:sectionIndexes]; + [self _insertSectionsIntoMap:map indexes:sectionIndexes]; // Return immediately because reloadData can't be used in conjuntion with other updates. return; } for (_ASHierarchySectionChange *change in [changeSet sectionChangesOfType:_ASHierarchyChangeTypeDelete]) { - [map removeSectionContextsAtIndexes:change.indexSet]; + [map removeSectionsAtIndexes:change.indexSet]; } for (_ASHierarchySectionChange *change in [changeSet sectionChangesOfType:_ASHierarchyChangeTypeInsert]) { - [self _insertSectionContextsIntoMap:map indexes:change.indexSet]; + [self _insertSectionsIntoMap:map indexes:change.indexSet]; } } -- (void)_insertSectionContextsIntoMap:(ASMutableElementMap *)map indexes:(NSIndexSet *)sectionIndexes +- (void)_insertSectionsIntoMap:(ASMutableElementMap *)map indexes:(NSIndexSet *)sectionIndexes { ASDisplayNodeAssertMainThread(); - - if (!_dataSourceFlags.contextForSection) { - return; - } - + [sectionIndexes enumerateIndexesUsingBlock:^(NSUInteger idx, BOOL * _Nonnull stop) { - id context = [_dataSource dataController:self contextForSection:idx]; + id context; + if (_dataSourceFlags.contextForSection) { + context = [_dataSource dataController:self contextForSection:idx]; + } ASSection *section = [[ASSection alloc] initWithSectionID:_nextSectionID context:context]; [map insertSection:section atIndex:idx]; _nextSectionID++; diff --git a/Source/Details/ASElementMap.h b/Source/Details/ASElementMap.h index fb53ee818d..eed7eff262 100644 --- a/Source/Details/ASElementMap.h +++ b/Source/Details/ASElementMap.h @@ -64,10 +64,20 @@ AS_SUBCLASSING_RESTRICTED @property (copy, readonly) NSArray *itemElements; /** - * Returns the index path that corresponds to the same element in @c map at the given @c indexPath. O(1) + * Returns the index path that corresponds to the same element in @c map at the given @c indexPath. + * O(1) for items, fast O(N) for sections. + * + * Note you can pass "section index paths" of length 1 and get a corresponding section index path. */ - (nullable NSIndexPath *)convertIndexPath:(NSIndexPath *)indexPath fromMap:(ASElementMap *)map; +/** + * Returns the section index into the receiver that corresponds to the same element in @c map at @c sectionIndex. Fast O(N). + * + * Returns @c NSNotFound if the section does not exist in the receiver. + */ +- (NSInteger)convertSection:(NSInteger)sectionIndex fromMap:(ASElementMap *)map; + /** * Returns the index path for the given element. O(1) */ diff --git a/Source/Details/ASElementMap.m b/Source/Details/ASElementMap.m index ddcd18c99e..08e90d401e 100644 --- a/Source/Details/ASElementMap.m +++ b/Source/Details/ASElementMap.m @@ -47,6 +47,8 @@ - (instancetype)initWithSections:(NSArray *)sections items:(ASCollectionElementTwoDimensionalArray *)items supplementaryElements:(ASSupplementaryElementDictionary *)supplementaryElements { + NSCParameterAssert(items.count == sections.count); + if (self = [super init]) { _sections = [sections copy]; _sectionsOfItems = [[NSArray alloc] initWithArray:items copyItems:YES]; @@ -157,8 +159,25 @@ - (NSIndexPath *)convertIndexPath:(NSIndexPath *)indexPath fromMap:(ASElementMap *)map { - id element = [map elementForItemAtIndexPath:indexPath]; - return [self indexPathForElement:element]; + if (indexPath.item == NSNotFound) { + // Section index path + NSInteger result = [self convertSection:indexPath.section fromMap:map]; + return (result != NSNotFound ? [NSIndexPath indexPathWithIndex:result] : nil); + } else { + // Item index path + ASCollectionElement *element = [map elementForItemAtIndexPath:indexPath]; + return [self indexPathForElement:element]; + } +} + +- (NSInteger)convertSection:(NSInteger)sectionIndex fromMap:(ASElementMap *)map +{ + if (![map sectionIndexIsValid:sectionIndex assert:YES]) { + return NSNotFound; + } + + ASSection *section = map.sections[sectionIndex]; + return [_sections indexOfObjectIdenticalTo:section]; } #pragma mark - NSCopying diff --git a/Source/Private/ASMutableElementMap.h b/Source/Private/ASMutableElementMap.h index 3b5c9bd7f8..10225a4556 100644 --- a/Source/Private/ASMutableElementMap.h +++ b/Source/Private/ASMutableElementMap.h @@ -37,10 +37,10 @@ AS_SUBCLASSING_RESTRICTED - (void)insertSection:(ASSection *)section atIndex:(NSInteger)index; -- (void)removeAllSectionContexts; +- (void)removeAllSections; /// Only modifies the array of ASSection * objects -- (void)removeSectionContextsAtIndexes:(NSIndexSet *)indexes; +- (void)removeSectionsAtIndexes:(NSIndexSet *)indexes; - (void)removeAllElements; diff --git a/Source/Private/ASMutableElementMap.m b/Source/Private/ASMutableElementMap.m index 7ce12dca85..c7742bbecc 100644 --- a/Source/Private/ASMutableElementMap.m +++ b/Source/Private/ASMutableElementMap.m @@ -48,7 +48,7 @@ typedef NSMutableDictionary +#import @protocol ASSectionContext; +NS_ASSUME_NONNULL_BEGIN + +/** + * An object representing the metadata for a section of elements in a collection. + * + * Its sectionID is namespaced to the data controller that created the section. + * + * These are useful for tracking the movement & lifetime of sections, independent of + * their contents. + */ +AS_SUBCLASSING_RESTRICTED @interface ASSection : NSObject -@property (nonatomic, assign, readonly) NSInteger sectionID; -@property (nonatomic, strong, nullable, readonly) id context; +@property (assign, readonly) NSInteger sectionID; +@property (strong, nullable, readonly) id context; -- (nullable instancetype)init __unavailable; -- (nullable instancetype)initWithSectionID:(NSInteger)sectionID context:(nullable id)context NS_DESIGNATED_INITIALIZER; +- (instancetype)init NS_UNAVAILABLE; +- (instancetype)initWithSectionID:(NSInteger)sectionID context:(nullable id)context NS_DESIGNATED_INITIALIZER; @end + +NS_ASSUME_NONNULL_END