-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix collection editing #1081
Fix collection editing #1081
Changes from 23 commits
929d118
dd24d8f
b8eaffa
2918ea0
9c42266
329f35f
5f8b7ec
d87bb11
269c2ab
24c1ce8
233169e
b19f90d
b50cec4
b75a5f3
64b46e0
5fabc1e
3d5b84b
098b978
77eefd6
0310ed7
0b886de
a63d438
a015a7c
87bae2f
ca74b44
230e37d
b7bbe97
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -147,6 +147,12 @@ @interface ASCollectionView () <ASRangeControllerDataSource, ASRangeControllerDe | |
* (0 sections) we always check at least once after each update (initial reload is the first update.) | ||
*/ | ||
BOOL _hasEverCheckedForBatchFetchingDueToUpdate; | ||
|
||
/** | ||
* Set during beginInteractiveMovementForItemAtIndexPath and UIGestureRecognizerStateEnded | ||
* (or UIGestureRecognizerStateFailed, UIGestureRecognizerStateCancelled. | ||
*/ | ||
BOOL _editing; | ||
|
||
/** | ||
* Counter used to keep track of nested batch updates. | ||
|
@@ -1021,9 +1027,28 @@ - (void)reloadItemsAtIndexPaths:(NSArray *)indexPaths | |
- (void)moveItemAtIndexPath:(NSIndexPath *)indexPath toIndexPath:(NSIndexPath *)newIndexPath | ||
{ | ||
ASDisplayNodeAssertMainThread(); | ||
[self performBatchUpdates:^{ | ||
[_changeSet moveItemAtIndexPath:indexPath toIndexPath:newIndexPath animationOptions:kASCollectionViewAnimationNone]; | ||
} completion:nil]; | ||
if (!_editing) { | ||
[self performBatchUpdates:^{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should performBatchUpdates itself change behavior when interactiveMovement is in progress? Imagine a reorder starts at the same time that a new page of content comes in, creating an insertion. I'm not sure what's actually supposed to happen there, but maybe we would want to just have performBatchUpdates internally run the batch operations without actually calling performBatchUpdates on the parent view? That said, doing this only for moves right now seems reasonable if there is not a clear answer to this other question. Ideally in the case of the insertion, we at least know the UI will end up in a good state and not crash, even if it forces the user out of interactive mode. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this method is called on main thread and should not be happening at same time? One case should be blocked waiting to the other to finish? |
||
[_changeSet moveItemAtIndexPath:indexPath toIndexPath:newIndexPath animationOptions:kASCollectionViewAnimationNone]; | ||
} completion:nil]; | ||
} else { | ||
[super moveItemAtIndexPath:indexPath toIndexPath:newIndexPath]; | ||
} | ||
} | ||
|
||
- (BOOL)beginInteractiveMovementForItemAtIndexPath:(NSIndexPath *)indexPath { | ||
_editing = YES; | ||
return [super beginInteractiveMovementForItemAtIndexPath:indexPath]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If super returns |
||
} | ||
|
||
- (void)endInteractiveMovement { | ||
_editing = NO; | ||
return [super endInteractiveMovement]; | ||
} | ||
|
||
- (void)cancelInteractiveMovement { | ||
_editing = NO; | ||
[super cancelInteractiveMovement]; | ||
} | ||
|
||
#pragma mark - | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's call this reordering, or maybe _interactiveMovementInProgress ?