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

Fix collection editing #1081

Merged
merged 27 commits into from
Sep 5, 2018
Merged
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
929d118
fix SIMULATE_WEB_RESPONSE not imported #449
wsdwsd0829 Jul 16, 2017
dd24d8f
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Sep 6, 2017
b8eaffa
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Oct 4, 2017
2918ea0
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Oct 11, 2017
9c42266
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Oct 12, 2017
329f35f
Fix to make rangeMode update in right time
wsdwsd0829 Feb 5, 2018
5f8b7ec
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Feb 7, 2018
d87bb11
merge master from upstream
wsdwsd0829 Feb 15, 2018
269c2ab
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Mar 10, 2018
24c1ce8
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Mar 12, 2018
233169e
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Mar 26, 2018
b19f90d
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Mar 28, 2018
b50cec4
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Apr 5, 2018
b75a5f3
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 May 3, 2018
64b46e0
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 May 16, 2018
5fabc1e
remove uncessary assert
wsdwsd0829 May 17, 2018
3d5b84b
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 May 21, 2018
098b978
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Jun 8, 2018
77eefd6
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Jun 13, 2018
0310ed7
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Jul 3, 2018
0b886de
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Jul 25, 2018
a63d438
Merge branch 'master' of github.com:TextureGroup/Texture
wsdwsd0829 Aug 21, 2018
a015a7c
Fix collection cell editing bug for iOS 9 & 10
wsdwsd0829 Aug 22, 2018
87bae2f
Rename to reordering.
wsdwsd0829 Aug 27, 2018
ca74b44
Adjust _reordering more acuratedly
wsdwsd0829 Sep 4, 2018
230e37d
Add change log
wsdwsd0829 Sep 5, 2018
b7bbe97
Merge branch 'master' into fix-collection-editting
wsdwsd0829 Sep 5, 2018
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
32 changes: 29 additions & 3 deletions Source/ASCollectionView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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 _reordering;

/**
* Counter used to keep track of nested batch updates.
Expand Down Expand Up @@ -1021,9 +1027,29 @@ - (void)reloadItemsAtIndexPaths:(NSArray *)indexPaths
- (void)moveItemAtIndexPath:(NSIndexPath *)indexPath toIndexPath:(NSIndexPath *)newIndexPath
{
ASDisplayNodeAssertMainThread();
[self performBatchUpdates:^{
[_changeSet moveItemAtIndexPath:indexPath toIndexPath:newIndexPath animationOptions:kASCollectionViewAnimationNone];
} completion:nil];
if (!_reordering) {
[self performBatchUpdates:^{
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 {
BOOL success = [super beginInteractiveMovementForItemAtIndexPath:indexPath];
_reordering = success;
return success;
}

- (void)endInteractiveMovement {
_reordering = NO;
[super endInteractiveMovement];
}

- (void)cancelInteractiveMovement {
_reordering = NO;
[super cancelInteractiveMovement];
}

#pragma mark -
Expand Down