Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for keeping letting cell nodes update to new view models when reloaded. #trivial #357

Merged
merged 6 commits into from Jun 13, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions AsyncDisplayKit.xcodeproj/project.pbxproj
Expand Up @@ -326,6 +326,7 @@
CC11F97A1DB181180024D77B /* ASNetworkImageNodeTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CC11F9791DB181180024D77B /* ASNetworkImageNodeTests.m */; };
CC2F65EE1E5FFB1600DA57C9 /* ASMutableElementMap.h in Headers */ = {isa = PBXBuildFile; fileRef = CC2F65EC1E5FFB1600DA57C9 /* ASMutableElementMap.h */; };
CC2F65EF1E5FFB1600DA57C9 /* ASMutableElementMap.m in Sources */ = {isa = PBXBuildFile; fileRef = CC2F65ED1E5FFB1600DA57C9 /* ASMutableElementMap.m */; };
CC311E071EEF81C400A8D7A6 /* ASDisplayNode+OCMock.m in Sources */ = {isa = PBXBuildFile; fileRef = CC311E061EEF81C400A8D7A6 /* ASDisplayNode+OCMock.m */; };
CC3B20841C3F76D600798563 /* ASPendingStateController.h in Headers */ = {isa = PBXBuildFile; fileRef = CC3B20811C3F76D600798563 /* ASPendingStateController.h */; settings = {ATTRIBUTES = (Private, ); }; };
CC3B20861C3F76D600798563 /* ASPendingStateController.mm in Sources */ = {isa = PBXBuildFile; fileRef = CC3B20821C3F76D600798563 /* ASPendingStateController.mm */; };
CC3B208A1C3F7A5400798563 /* ASWeakSet.h in Headers */ = {isa = PBXBuildFile; fileRef = CC3B20871C3F7A5400798563 /* ASWeakSet.h */; settings = {ATTRIBUTES = (Public, ); }; };
Expand Down Expand Up @@ -784,6 +785,7 @@
CC2E317F1DAC353700EEE891 /* ASCollectionView+Undeprecated.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "ASCollectionView+Undeprecated.h"; sourceTree = "<group>"; };
CC2F65EC1E5FFB1600DA57C9 /* ASMutableElementMap.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASMutableElementMap.h; sourceTree = "<group>"; };
CC2F65ED1E5FFB1600DA57C9 /* ASMutableElementMap.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASMutableElementMap.m; sourceTree = "<group>"; };
CC311E061EEF81C400A8D7A6 /* ASDisplayNode+OCMock.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "ASDisplayNode+OCMock.m"; sourceTree = "<group>"; };
CC3B20811C3F76D600798563 /* ASPendingStateController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASPendingStateController.h; sourceTree = "<group>"; };
CC3B20821C3F76D600798563 /* ASPendingStateController.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASPendingStateController.mm; sourceTree = "<group>"; };
CC3B20871C3F7A5400798563 /* ASWeakSet.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASWeakSet.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1117,6 +1119,7 @@
058D09C5195D04C000B7D73C /* Tests */ = {
isa = PBXGroup;
children = (
CC311E061EEF81C400A8D7A6 /* ASDisplayNode+OCMock.m */,
CCB338E51EEE27760081F21A /* ASTestCase.h */,
CCB338E61EEE27760081F21A /* ASTestCase.m */,
CCB338E21EEE11160081F21A /* OCMockObject+ASAdditions.h */,
Expand Down Expand Up @@ -2077,6 +2080,7 @@
058D0A3C195D057000B7D73C /* ASMutableAttributedStringBuilderTests.m in Sources */,
CC8B05D81D73979700F54286 /* ASTextNodePerformanceTests.m in Sources */,
697B315A1CFE4B410049936F /* ASEditableTextNodeTests.m in Sources */,
CC311E071EEF81C400A8D7A6 /* ASDisplayNode+OCMock.m in Sources */,
CCB338E41EEE11160081F21A /* OCMockObject+ASAdditions.m in Sources */,
ACF6ED611B178DC700DA7C62 /* ASOverlayLayoutSpecSnapshotTests.mm in Sources */,
CC8B05D61D73836400F54286 /* ASPerformanceTestContext.m in Sources */,
Expand Down
7 changes: 7 additions & 0 deletions Source/ASCellNode.h
Expand Up @@ -126,6 +126,13 @@ typedef NS_ENUM(NSUInteger, ASCellNodeVisibilityEvent) {
*/
@property (atomic, nullable) id viewModel;

/**
* Asks the node whether it can be updated to the given view model.
*
* The default implementation returns YES if the class matches that of the current view-model.
*/
- (BOOL)canUpdateToViewModel:(id)viewModel;

/**
* The backing view controller, or @c nil if the node wasn't initialized with backing view controller
* @note This property must be accessed on the main thread.
Expand Down
5 changes: 5 additions & 0 deletions Source/ASCellNode.mm
Expand Up @@ -177,6 +177,11 @@ - (void)__setHighlightedFromUIKit:(BOOL)highlighted;
}
}

- (BOOL)canUpdateToViewModel:(id)viewModel
{
return [self.viewModel class] == [viewModel class];
}

- (NSIndexPath *)indexPath
{
return [self.owningNode indexPathForNode:self];
Expand Down
56 changes: 42 additions & 14 deletions Source/Details/ASDataController.mm
Expand Up @@ -303,6 +303,7 @@ - (void)_repopulateSupplementaryNodesIntoMap:(ASMutableElementMap *)map
traitCollection:(ASPrimitiveTraitCollection)traitCollection
indexPathsAreNew:(BOOL)indexPathsAreNew
shouldFetchSizeRanges:(BOOL)shouldFetchSizeRanges
previousMap:(ASElementMap *)previousMap
{
ASDisplayNodeAssertMainThread();

Expand All @@ -325,7 +326,7 @@ - (void)_repopulateSupplementaryNodesIntoMap:(ASMutableElementMap *)map
}

for (NSString *kind in [self supplementaryKindsInSections:newSections]) {
[self _insertElementsIntoMap:map kind:kind forSections:newSections traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges];
[self _insertElementsIntoMap:map kind:kind forSections:newSections traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges changeSet:changeSet previousMap:previousMap];
}
}

Expand All @@ -342,6 +343,8 @@ - (void)_insertElementsIntoMap:(ASMutableElementMap *)map
forSections:(NSIndexSet *)sections
traitCollection:(ASPrimitiveTraitCollection)traitCollection
shouldFetchSizeRanges:(BOOL)shouldFetchSizeRanges
changeSet:(_ASHierarchyChangeSet *)changeSet
previousMap:(ASElementMap *)previousMap
{
ASDisplayNodeAssertMainThread();

Expand All @@ -350,7 +353,7 @@ - (void)_insertElementsIntoMap:(ASMutableElementMap *)map
}

NSArray<NSIndexPath *> *indexPaths = [self _allIndexPathsForItemsOfKind:kind inSections:sections];
[self _insertElementsIntoMap:map kind:kind atIndexPaths:indexPaths traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges];
[self _insertElementsIntoMap:map kind:kind atIndexPaths:indexPaths traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges changeSet:changeSet previousMap:previousMap];
}

/**
Expand All @@ -367,6 +370,8 @@ - (void)_insertElementsIntoMap:(ASMutableElementMap *)map
atIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
traitCollection:(ASPrimitiveTraitCollection)traitCollection
shouldFetchSizeRanges:(BOOL)shouldFetchSizeRanges
changeSet:(_ASHierarchyChangeSet *)changeSet
previousMap:(ASElementMap *)previousMap
{
ASDisplayNodeAssertMainThread();

Expand All @@ -384,11 +389,28 @@ - (void)_insertElementsIntoMap:(ASMutableElementMap *)map
id<ASDataControllerSource> dataSource = self.dataSource;
id<ASRangeManagingNode> node = self.node;
for (NSIndexPath *indexPath in indexPaths) {
id viewModel = [dataSource dataController:self viewModelForItemAtIndexPath:indexPath];

ASCellNodeBlock nodeBlock;
id viewModel;
if (isRowKind) {
nodeBlock = [dataSource dataController:self nodeBlockAtIndexPath:indexPath];
viewModel = [dataSource dataController:self viewModelForItemAtIndexPath:indexPath];

// Get the prior element and attempt to update the existing cell node.
if (viewModel != nil && !changeSet.includesReloadData) {
NSIndexPath *oldIndexPath = [changeSet oldIndexPathForNewIndexPath:indexPath];
if (oldIndexPath != nil) {
ASCollectionElement *oldElement = [previousMap elementForItemAtIndexPath:oldIndexPath];
ASCellNode *oldNode = oldElement.node;
if ([oldNode canUpdateToViewModel:viewModel]) {
// Just wrap the node in a block. The collection element will -setViewModel:
nodeBlock = ^{
return oldNode;
};
}
}
}
if (nodeBlock == nil) {
nodeBlock = [dataSource dataController:self nodeBlockAtIndexPath:indexPath];
}
} else {
nodeBlock = [dataSource dataController:self supplementaryNodeBlockOfKind:kind atIndexPath:indexPath];
}
Expand Down Expand Up @@ -534,14 +556,15 @@ - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
}

// Mutable copy of current data.
ASMutableElementMap *mutableMap = [_pendingMap mutableCopy];
ASElementMap *previousMap = _pendingMap;
ASMutableElementMap *mutableMap = [previousMap mutableCopy];

BOOL canDelegateLayout = (_layoutDelegate != nil);

// Step 1: Update the mutable copies to match the data source's state
[self _updateSectionContextsInMap:mutableMap changeSet:changeSet];
ASPrimitiveTraitCollection existingTraitCollection = [self.node primitiveTraitCollection];
[self _updateElementsInMap:mutableMap changeSet:changeSet traitCollection:existingTraitCollection shouldFetchSizeRanges:(! canDelegateLayout)];
[self _updateElementsInMap:mutableMap changeSet:changeSet traitCollection:existingTraitCollection shouldFetchSizeRanges:(! canDelegateLayout) previousMap:previousMap];

// Step 2: Clone the new data
ASElementMap *newMap = [mutableMap copy];
Expand Down Expand Up @@ -644,6 +667,7 @@ - (void)_updateElementsInMap:(ASMutableElementMap *)map
changeSet:(_ASHierarchyChangeSet *)changeSet
traitCollection:(ASPrimitiveTraitCollection)traitCollection
shouldFetchSizeRanges:(BOOL)shouldFetchSizeRanges
previousMap:(ASElementMap *)previousMap
{
ASDisplayNodeAssertMainThread();

Expand All @@ -653,7 +677,7 @@ - (void)_updateElementsInMap:(ASMutableElementMap *)map
NSUInteger sectionCount = [self itemCountsFromDataSource].size();
if (sectionCount > 0) {
NSIndexSet *sectionIndexes = [NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, sectionCount)];
[self _insertElementsIntoMap:map sections:sectionIndexes traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges];
[self _insertElementsIntoMap:map sections:sectionIndexes traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges changeSet:changeSet previousMap:previousMap];
}
// Return immediately because reloadData can't be used in conjuntion with other updates.
return;
Expand All @@ -666,7 +690,8 @@ - (void)_updateElementsInMap:(ASMutableElementMap *)map
changeSet:changeSet
traitCollection:traitCollection
indexPathsAreNew:NO
shouldFetchSizeRanges:shouldFetchSizeRanges];
shouldFetchSizeRanges:shouldFetchSizeRanges
previousMap:previousMap];
}

for (_ASHierarchySectionChange *change in [changeSet sectionChangesOfType:_ASHierarchyChangeTypeDelete]) {
Expand All @@ -676,24 +701,27 @@ - (void)_updateElementsInMap:(ASMutableElementMap *)map
}

for (_ASHierarchySectionChange *change in [changeSet sectionChangesOfType:_ASHierarchyChangeTypeInsert]) {
[self _insertElementsIntoMap:map sections:change.indexSet traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges];
[self _insertElementsIntoMap:map sections:change.indexSet traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges changeSet:changeSet previousMap:previousMap];
}

for (_ASHierarchyItemChange *change in [changeSet itemChangesOfType:_ASHierarchyChangeTypeInsert]) {
[self _insertElementsIntoMap:map kind:ASDataControllerRowNodeKind atIndexPaths:change.indexPaths traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges];
[self _insertElementsIntoMap:map kind:ASDataControllerRowNodeKind atIndexPaths:change.indexPaths traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges changeSet:changeSet previousMap:previousMap];
// Aggressively reload supplementary nodes (#1773 & #1629)
[self _repopulateSupplementaryNodesIntoMap:map forSectionsContainingIndexPaths:change.indexPaths
changeSet:changeSet
traitCollection:traitCollection
indexPathsAreNew:YES
shouldFetchSizeRanges:shouldFetchSizeRanges];
shouldFetchSizeRanges:shouldFetchSizeRanges
previousMap:previousMap];
}
}

- (void)_insertElementsIntoMap:(ASMutableElementMap *)map
sections:(NSIndexSet *)sectionIndexes
traitCollection:(ASPrimitiveTraitCollection)traitCollection
shouldFetchSizeRanges:(BOOL)shouldFetchSizeRanges
changeSet:(_ASHierarchyChangeSet *)changeSet
previousMap:(ASElementMap *)previousMap
{
ASDisplayNodeAssertMainThread();

Expand All @@ -703,12 +731,12 @@ - (void)_insertElementsIntoMap:(ASMutableElementMap *)map

// Items
[map insertEmptySectionsOfItemsAtIndexes:sectionIndexes];
[self _insertElementsIntoMap:map kind:ASDataControllerRowNodeKind forSections:sectionIndexes traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges];
[self _insertElementsIntoMap:map kind:ASDataControllerRowNodeKind forSections:sectionIndexes traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges changeSet:changeSet previousMap:previousMap];

// Supplementaries
for (NSString *kind in [self supplementaryKindsInSections:sectionIndexes]) {
// Step 2: Populate new elements for all sections
[self _insertElementsIntoMap:map kind:kind forSections:sectionIndexes traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges];
[self _insertElementsIntoMap:map kind:kind forSections:sectionIndexes traitCollection:traitCollection shouldFetchSizeRanges:shouldFetchSizeRanges changeSet:changeSet previousMap:previousMap];
}
}

Expand Down
8 changes: 8 additions & 0 deletions Source/Private/_ASHierarchyChangeSet.h
Expand Up @@ -147,6 +147,14 @@ NSString *NSStringFromASHierarchyChangeType(_ASHierarchyChangeType changeType);
*/
- (NSUInteger)newSectionForOldSection:(NSUInteger)oldSection;

/**
* Get the old item index path for the given new index path.
*
* @precondition The change set must be completed.
* @return The old index path, or nil if the given item was inserted.
*/
- (nullable NSIndexPath *)oldIndexPathForNewIndexPath:(NSIndexPath *)indexPath;

/// Call this once the change set has been constructed to prevent future modifications to the changeset. Calling this more than once is a programmer error.
/// NOTE: Calling this method will cause the changeset to convert all reloads into delete/insert pairs.
- (void)markCompletedWithNewItemCounts:(std::vector<NSInteger>)newItemCounts;
Expand Down
41 changes: 40 additions & 1 deletion Source/Private/_ASHierarchyChangeSet.mm
@@ -1,5 +1,5 @@
//
// _ASHierarchyChangeSet.m
// _ASHierarchyChangeSet.mm
// Texture
//
// Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
Expand Down Expand Up @@ -256,6 +256,45 @@ - (NSUInteger)newSectionForOldSection:(NSUInteger)oldSection
return newIndex;
}

- (NSUInteger)oldSectionForNewSection:(NSUInteger)newSection
{
[self _ensureCompleted];
if ([_insertedSections containsIndex:newSection]) {
return NSNotFound;
}

NSInteger oldIndex = newSection - [_insertedSections as_indexChangeByInsertingItemsBelowIndex:newSection];
oldIndex += [_deletedSections countOfIndexesInRange:NSMakeRange(0, oldIndex)];
return oldIndex;
}

- (NSIndexPath *)oldIndexPathForNewIndexPath:(NSIndexPath *)indexPath
{
[self _ensureCompleted];
// Inserted sections return nil.
NSInteger newSection = indexPath.section;
NSInteger newItem = indexPath.item;
NSInteger oldSection = [self oldSectionForNewSection:newSection];
if (oldSection == NSNotFound) {
return nil;
}

// Inserted items return nil.
for (_ASHierarchyItemChange *change in _originalInsertItemChanges) {
if ([change.indexPaths containsObject:indexPath]) {
return nil;
}
}

// TODO: This is a pretty inefficient way to do this.
NSIndexSet *insertsInSection = [_ASHierarchyItemChange sectionToIndexSetMapFromChanges:_insertItemChanges][@(newSection)];
NSIndexSet *deletesInSection = [_ASHierarchyItemChange sectionToIndexSetMapFromChanges:_deleteItemChanges][@(oldSection)];

NSInteger oldIndex = newItem - [insertsInSection as_indexChangeByInsertingItemsBelowIndex:newItem];
oldIndex += [deletesInSection countOfIndexesInRange:NSMakeRange(0, oldIndex)];
return [NSIndexPath indexPathForItem:oldIndex inSection:oldSection];
}

- (void)reloadData
{
[self _ensureNotCompleted];
Expand Down