Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

[ASCollectionView] Repopulate supplementary views on item-level changes #1629

Merged
merged 7 commits into from
Jun 6, 2016
Merged

[ASCollectionView] Repopulate supplementary views on item-level changes #1629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fc5eef3
Repopulate supplementary views on item-level changes
erichoracek May 5, 2016
8f3e511
Fix warning
erichoracek May 6, 2016
d89504e
Fix index path arithmetic error
erichoracek May 6, 2016
dc7f67c
Populate more aggresively
erichoracek May 6, 2016
30dba28
Remove _pendingContexts dictionary mutation during enumeration
erichoracek May 11, 2016
ed9fcdc
Rebase
erichoracek Jun 6, 2016
70fbbe0
Whitespace
erichoracek Jun 6, 2016
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
101 changes: 83 additions & 18 deletions AsyncDisplayKit/Details/ASCollectionDataController.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,61 @@ - (void)willMoveSection:(NSInteger)section toSection:(NSInteger)newSection
}
}

- (void)prepareForInsertRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
{
for (NSString *kind in [self supplementaryKinds]) {
LOG(@"Populating elements of kind: %@, for index paths: %@", kind, indexPaths);
NSMutableArray<ASIndexedNodeContext *> *contexts = [NSMutableArray array];
[self _populateSupplementaryNodesOfKind:kind atIndexPaths:indexPaths mutableContexts:contexts];
_pendingContexts[kind] = contexts;
}
}

- (void)willInsertRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
{
NSArray *keys = _pendingContexts.allKeys;
for (NSString *kind in keys) {
NSMutableArray<ASIndexedNodeContext *> *contexts = _pendingContexts[kind];

[self batchLayoutNodesFromContexts:contexts ofKind:kind completion:^(NSArray<ASCellNode *> *nodes, NSArray<NSIndexPath *> *indexPaths) {
[self insertNodes:nodes ofKind:kind atIndexPaths:indexPaths completion:nil];
}];
[_pendingContexts removeObjectForKey:kind];
Copy link
Contributor

Choose a reason for hiding this comment

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

You are mutating _pendingContexts while enumerating over it. Shouldn't this be throwing an exception?

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like it doesn't throw an exception but does not do what you expect either. Try this in a playground:

var mutableArray = NSMutableArray(array: [1,2,3,4,5,6])

mutableArray.enumerateObjectsUsingBlock { (object, idx, stop) in
    print("\(idx): \(object)")
    mutableArray.removeObject(object)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I just realized that _pendingContexts is a dictionary not an array. Sorry about that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While I definitely agree that this is an issue, this is the precedent set by the code above in this file at https://github.com/facebook/AsyncDisplayKit/blob/master/AsyncDisplayKit/Details/ASCollectionDataController.mm#L90-L101

Copy link
Contributor

Choose a reason for hiding this comment

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

@erichoracek please do fix that, as it seems to be an oversight.

}
}

- (void)willDeleteRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
{
for (NSString *kind in [self supplementaryKinds]) {
NSArray<NSIndexPath *> *deletedIndexPaths = ASIndexPathsInMultidimensionalArrayIntersectingIndexPaths([self editingNodesOfKind:kind], indexPaths);
[self deleteNodesOfKind:kind atIndexPaths:deletedIndexPaths completion:nil];
}
}

- (void)prepareForReloadRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
{
for (NSString *kind in [self supplementaryKinds]) {
NSMutableArray<ASIndexedNodeContext *> *contexts = [NSMutableArray array];
[self _populateSupplementaryNodesOfKind:kind atIndexPaths:indexPaths mutableContexts:contexts];
_pendingContexts[kind] = contexts;
}
}

- (void)willReloadRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
{
NSArray *keys = _pendingContexts.allKeys;
for (NSString *kind in keys) {
NSMutableArray<ASIndexedNodeContext *> *contexts = _pendingContexts[kind];

[self deleteNodesOfKind:kind atIndexPaths:indexPaths completion:nil];
// reinsert the elements
[self batchLayoutNodesFromContexts:contexts ofKind:kind completion:^(NSArray<ASCellNode *> *nodes, NSArray<NSIndexPath *> *indexPaths) {
[self insertNodes:nodes ofKind:kind atIndexPaths:indexPaths completion:nil];
}];
[_pendingContexts removeObjectForKey:kind];
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about mutating _pendingContexts

}
}

- (void)_populateSupplementaryNodesOfKind:(NSString *)kind withMutableContexts:(NSMutableArray<ASIndexedNodeContext *> *)contexts
{
id<ASEnvironment> environment = [self.environmentDelegate dataControllerEnvironment];
Expand All @@ -167,21 +222,7 @@ - (void)_populateSupplementaryNodesOfKind:(NSString *)kind withMutableContexts:(
NSUInteger rowCount = [self.collectionDataSource dataController:self supplementaryNodesOfKind:kind inSection:i];
for (NSUInteger j = 0; j < rowCount; j++) {
NSIndexPath *indexPath = [sectionIndexPath indexPathByAddingIndex:j];

ASCellNodeBlock supplementaryCellBlock;
if (_dataSourceImplementsSupplementaryNodeBlockOfKindAtIndexPath) {
supplementaryCellBlock = [self.collectionDataSource dataController:self supplementaryNodeBlockOfKind:kind atIndexPath:indexPath];
} else {
ASCellNode *supplementaryNode = [self.collectionDataSource dataController:self supplementaryNodeOfKind:kind atIndexPath:indexPath];
supplementaryCellBlock = ^{ return supplementaryNode; };
}

ASSizeRange constrainedSize = [self constrainedSizeForNodeOfKind:kind atIndexPath:indexPath];
ASIndexedNodeContext *context = [[ASIndexedNodeContext alloc] initWithNodeBlock:supplementaryCellBlock
indexPath:indexPath
constrainedSize:constrainedSize
environmentTraitCollection:environmentTraitCollection];
[contexts addObject:context];
[self _populateSupplementaryNodeOfKind:kind atIndexPath:indexPath mutableContexts:contexts environmentTraitCollection:environmentTraitCollection];
}
}
}
Expand All @@ -196,7 +237,33 @@ - (void)_populateSupplementaryNodesOfKind:(NSString *)kind withSections:(NSIndex
NSIndexPath *sectionIndex = [[NSIndexPath alloc] initWithIndex:idx];
for (NSUInteger i = 0; i < rowNum; i++) {
NSIndexPath *indexPath = [sectionIndex indexPathByAddingIndex:i];
[self _populateSupplementaryNodeOfKind:kind atIndexPath:indexPath mutableContexts:contexts environmentTraitCollection:environmentTraitCollection];
}
}];
}

- (void)_populateSupplementaryNodesOfKind:(NSString *)kind atIndexPaths:(NSArray<NSIndexPath *> *)indexPaths mutableContexts:(NSMutableArray<ASIndexedNodeContext *> *)contexts
{
id<ASEnvironment> environment = [self.environmentDelegate dataControllerEnvironment];
ASEnvironmentTraitCollection environmentTraitCollection = environment.environmentTraitCollection;

NSMutableIndexSet *sections = [NSMutableIndexSet indexSet];
for (NSIndexPath *indexPath in indexPaths) {
[sections addIndex:indexPath.section];
}

[sections enumerateIndexesUsingBlock:^(NSUInteger idx, BOOL *stop) {
NSUInteger rowNum = [self.collectionDataSource dataController:self supplementaryNodesOfKind:kind inSection:idx];
NSIndexPath *sectionIndex = [[NSIndexPath alloc] initWithIndex:idx];
for (NSUInteger i = 0; i < rowNum; i++) {
NSIndexPath *indexPath = [sectionIndex indexPathByAddingIndex:i];
[self _populateSupplementaryNodeOfKind:kind atIndexPath:indexPath mutableContexts:contexts environmentTraitCollection:environmentTraitCollection];
}
}];
}

- (void)_populateSupplementaryNodeOfKind:(NSString *)kind atIndexPath:(NSIndexPath *)indexPath mutableContexts:(NSMutableArray<ASIndexedNodeContext *> *)contexts environmentTraitCollection:(ASEnvironmentTraitCollection)environmentTraitCollection
{
ASCellNodeBlock supplementaryCellBlock;
if (_dataSourceImplementsSupplementaryNodeBlockOfKindAtIndexPath) {
supplementaryCellBlock = [self.collectionDataSource dataController:self supplementaryNodeBlockOfKind:kind atIndexPath:indexPath];
Expand All @@ -211,8 +278,6 @@ - (void)_populateSupplementaryNodesOfKind:(NSString *)kind withSections:(NSIndex
constrainedSize:constrainedSize
environmentTraitCollection:environmentTraitCollection];
[contexts addObject:context];
}
}];
}

#pragma mark - Sizing query
Expand Down Expand Up @@ -264,4 +329,4 @@ - (void)setDataSource:(id<ASDataControllerSource>)dataSource
ASDisplayNodeAssertTrue(_dataSourceImplementsSupplementaryNodeBlockOfKindAtIndexPath || [self.collectionDataSource respondsToSelector:@selector(dataController:supplementaryNodeOfKind:atIndexPath:)]);
}

@end
@end
66 changes: 66 additions & 0 deletions AsyncDisplayKit/Details/ASDataController+Subclasses.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,4 +160,70 @@ typedef void (^ASDataControllerCompletionBlock)(NSArray<ASCellNode *> *nodes, NS
*/
- (void)willMoveSection:(NSInteger)section toSection:(NSInteger)newSection;

/**
* Notifies the subclass to perform setup before rows are inserted in the data controller.
*
* @discussion This method will be performed before the data controller enters its editing queue.
* The data source is locked at this point and accessing it is safe. Use this method to set up any nodes or
* data stores before entering into editing the backing store on a background thread.
*
* @param indexPaths Index paths for the rows to be inserted.
*/
- (void)prepareForInsertRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths;

/**
* Notifies the subclass that the data controller will insert new rows at the given index paths.
*
* @discussion This method will be performed on the data controller's editing background queue before the parent's
* concrete implementation. This is a great place to perform any additional transformations like supplementary views
* or header/footer nodes.
*
* @param indexPaths Index paths for the rows to be inserted.
*/
- (void)willInsertRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths;

/**
* Notifies the subclass to perform setup before rows are deleted in the data controller.
*
* @discussion This method will be performed before the data controller enters its editing queue.
* The data source is locked at this point and accessing it is safe. Use this method to set up any nodes or
* data stores before entering into editing the backing store on a background thread.
*
* @param indexPaths Index paths for the rows to be deleted.
*/
- (void)prepareForDeleteRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths;

/**
* Notifies the subclass that the data controller will delete rows at the given index paths.
*
* @discussion This method will be performed before the data controller enters its editing queue.
* The data source is locked at this point and accessing it is safe. Use this method to set up any nodes or
* data stores before entering into editing the backing store on a background thread.
*
* @param indexPaths Index paths for the rows to be deleted.
*/
- (void)willDeleteRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths;

/**
* Notifies the subclass to perform any work needed before the given rows will be reloaded.
*
* @discussion This method will be performed before the data controller enters its editing queue, usually on the main
* thread. The data source is locked at this point and accessing it is safe. Use this method to set up any nodes or
* data stores before entering into editing the backing store on a background thread.
*
* @param indexPaths Index paths for the rows to be reloaded.
*/
- (void)prepareForReloadRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths;

/**
* Notifies the subclass that the data controller will reload the rows at the given index paths.
*
* @discussion This method will be performed on the data controller's editing background queue before the parent's
* concrete implementation. This is a great place to perform any additional transformations like supplementary views
* or header/footer nodes.
*
* @param indexPaths Index paths for the rows to be reloaded.
*/
- (void)willReloadRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths;

@end
42 changes: 42 additions & 0 deletions AsyncDisplayKit/Details/ASDataController.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,36 @@ - (void)willMoveSection:(NSInteger)section toSection:(NSInteger)newSection
// Optional template hook for subclasses (See ASDataController+Subclasses.h)
}

- (void)prepareForInsertRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
{
// Optional template hook for subclasses (See ASDataController+Subclasses.h)
}

- (void)willInsertRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
{
// Optional template hook for subclasses (See ASDataController+Subclasses.h)
}

- (void)prepareForDeleteRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
{
// Optional template hook for subclasses (See ASDataController+Subclasses.h)
}

- (void)willDeleteRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
{
// Optional template hook for subclasses (See ASDataController+Subclasses.h)
}

- (void)prepareForReloadRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
{
// Optional template hook for subclasses (See ASDataController+Subclasses.h)
}

- (void)willReloadRowsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
{
// Optional template hook for subclasses (See ASDataController+Subclasses.h)
}

#pragma mark - Row Editing (External API)

- (void)insertRowsAtIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
Expand Down Expand Up @@ -799,7 +829,11 @@ - (void)insertRowsAtIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDat
environmentTraitCollection:environmentTraitCollection]];
}

[self prepareForInsertRowsAtIndexPaths:indexPaths];

[_editingTransactionQueue addOperationWithBlock:^{
[self willInsertRowsAtIndexPaths:indexPaths];

LOG(@"Edit Transaction - insertRows: %@", indexPaths);
[self _batchLayoutNodesFromContexts:contexts withAnimationOptions:animationOptions];
}];
Expand All @@ -819,7 +853,11 @@ - (void)deleteRowsAtIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDat
// FIXME: Shouldn't deletes be sorted in descending order?
NSArray *sortedIndexPaths = [indexPaths sortedArrayUsingSelector:@selector(compare:)];

[self prepareForDeleteRowsAtIndexPaths:sortedIndexPaths];

[_editingTransactionQueue addOperationWithBlock:^{
[self willDeleteRowsAtIndexPaths:sortedIndexPaths];

LOG(@"Edit Transaction - deleteRows: %@", indexPaths);
[self _deleteNodesAtIndexPaths:sortedIndexPaths withAnimationOptions:animationOptions];
}];
Expand Down Expand Up @@ -853,8 +891,12 @@ - (void)reloadRowsAtIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDat
constrainedSize:constrainedSize
environmentTraitCollection:environmentTraitCollection]];
}

[self prepareForReloadRowsAtIndexPaths:indexPaths];

[_editingTransactionQueue addOperationWithBlock:^{
[self willReloadRowsAtIndexPaths:indexPaths];

LOG(@"Edit Transaction - reloadRows: %@", indexPaths);
[self _deleteNodesAtIndexPaths:sortedIndexPaths withAnimationOptions:animationOptions];
[self _batchLayoutNodesFromContexts:contexts withAnimationOptions:animationOptions];
Expand Down
7 changes: 6 additions & 1 deletion AsyncDisplayKit/Private/ASMultidimensionalArrayUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ extern NSArray *ASFindElementsInMultidimensionalArrayAtIndexPaths(NSMutableArray
/**
* Return all the index paths of mutable multidimensional array at given index set, in ascending order.
*/
extern NSArray *ASIndexPathsForMultidimensionalArrayAtIndexSet(NSArray *MultidimensionalArray, NSIndexSet *indexSet);
extern NSArray *ASIndexPathsForMultidimensionalArrayAtIndexSet(NSArray *multidimensionalArray, NSIndexSet *indexSet);

/**
* Return the index paths of the given multidimensional array that are present in the given index paths array.
*/
extern NSArray<NSIndexPath *> *ASIndexPathsInMultidimensionalArrayIntersectingIndexPaths(NSArray *multidimensionalArray, NSArray<NSIndexPath *> *indexPaths);

/**
* Return all the index paths of a two-dimensional array, in ascending order.
Expand Down
34 changes: 34 additions & 0 deletions AsyncDisplayKit/Private/ASMultidimensionalArrayUtils.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,28 @@ static void ASRecursivelyFindIndexPathsForMultidimensionalArray(NSObject *obj, N
}
}

static BOOL ASElementExistsAtIndexPathForMultidimensionalArray(NSArray *array, NSIndexPath *indexPath) {
NSUInteger indexLength = indexPath.length;
ASDisplayNodeCAssert(indexLength != 0, @"Must have a non-zero indexPath length");
NSUInteger firstIndex = [indexPath indexAtPosition:0];
BOOL elementExists = firstIndex < array.count;

if (indexLength == 1) {
return elementExists;
}

if (!elementExists) {
return NO;
}

NSUInteger indexesLength = indexLength - 1;
NSUInteger indexes[indexesLength];
[indexPath getIndexes:indexes range:NSMakeRange(1, indexesLength)];
NSIndexPath *indexPathByRemovingFirstIndex = [NSIndexPath indexPathWithIndexes:indexes length:indexesLength];

return ASElementExistsAtIndexPathForMultidimensionalArray(array[firstIndex], indexPathByRemovingFirstIndex);
}

#pragma mark - Public Methods

NSObject<NSCopying> *ASMultidimensionalArrayDeepMutableCopy(NSObject<NSCopying> *obj)
Expand Down Expand Up @@ -142,6 +164,18 @@ void ASDeleteElementsInMultidimensionalArrayAtIndexPaths(NSMutableArray *mutable
return res;
}

NSArray<NSIndexPath *> *ASIndexPathsInMultidimensionalArrayIntersectingIndexPaths(NSArray *multidimensionalArray, NSArray<NSIndexPath *> *indexPaths)
{
NSMutableArray *res = [NSMutableArray array];
for (NSIndexPath *indexPath in indexPaths) {
if (ASElementExistsAtIndexPathForMultidimensionalArray(multidimensionalArray, indexPath)) {
[res addObject:indexPath];
}
}

return res;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe [res copy] here to make sure we don't return a mutable array here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Returning mutable arrays is the precedent set by the code in this file—I wasn't sure if I should change it as part of this

}

NSArray *ASIndexPathsForTwoDimensionalArray(NSArray <NSArray *>* twoDimensionalArray)
{
NSMutableArray *result = [NSMutableArray array];
Expand Down