The reordering animations are not canceled when all the cells fit to screen

[fitch]

There is a problem with LXReorderableCollectionViewFlowLayout when all your cells fit to one screen and you do the following:

• Use the simulator and turn on slow animations
• Drag one cell (for example the 1st) to 2nd position and immediately when the cell reordering has begun drag the cell back to the 1st position
• This will lead to a situation where the first cell reordering animation is not canceled and continues to move

However, if you increase the number of cells so, that they all don't fit to the screen - the previous animations are canceled.

You can replicate this behavior with the LXRCVFL Example using Storyboard by modifying LXCollectionViewController.m to supply only 6 cells (6 cards). Then start the example with the simulator, turn on slow animations and drag the first cell to second position and immediately back (and then again). You'll see that the old animations are not canceled. However, if you run the example with, say 52 cards, the behavior is gone.

It seems that, for some odd reason, [collectionView performBatchUpdates:] is not canceled in invalidateLayoutIfNecessary (LXReorderableCollectionViewFlowLayout line 97), when the cells fit to one screen.

[fitch]

Here's a screenshot of the example with 6 cards and the first card has been dragged a couple of times between the 1st and 2nd position.

[lukescott]

This issue has been fixed in https://github.com/lukescott/LXReorderableCollectionViewFlowLayout/issues/3. Just waiting on confirmation, then I'll push it to my master branch and make it part of pull request https://github.com/lxcid/LXReorderableCollectionViewFlowLayout/pull/14.

[lukescott]

It turns out this was more difficult to fix. The insert/delete (or move) commands cause animations themselves. The fix, doing a blank performBatchUpdates, simply stopped all the animations, which caused other problems. It became clear that insert/delete commands were not the best way to do cell re-ordering, so I started over from scratch and DraggableCollectionView is the result.

Instead of using insert/delete commands DraggableCollectionView "warps" the result from layoutAttributesForElementsInRect by changing the NSIndexPath of each UICollectionViewLayoutAttributes object. There are many benefits of doing it this way: (1) The data source is only modified once - when the user lifts their finger. (2) Animations from other cells are not canceled, so the animations are more fluid. (3) The logic can be applied to any custom layout. I have a demo that features Apple's CircleLayout.

[lxcid]

Thanks @lukescott, if you may, I'll like to check out your repo and attempts to merge your solution in here. I knew there's weird animation for certain case and if your solution seems a lot better.

It won't be that soon that I'm back on my desk (still on holiday), but once I'm back I'll check it out! Thanks for sharing!

[lukescott]

@lxcid, I'm not sure if that's possible without completely changing your delegate/dataSource calls. LXReorderableCollectionViewFlowLayout assumes that the data source changes for every movement, and DraggableCollectionView assumes that it doesn't. The index path of the item being moved doesn't change; just its physical position changes. So the point I was trying to make is: I don't think the animation bugs can be fixed in LXReorderableCollectionViewFlowLayout.

Because of how different DraggableCollectionView works, it didn't make sense to try and merge it into LXReorderableCollectionViewFlowLayout. If it were that simple I would have done that. "DraggableCollectionView" is more of a 2.0 than anything else. I think it would be best to keep LXReorderableCollectionViewFlowLayout the way it is for backwards compatibility.

[fitch]

Thanks @lukescott, I'll try to check with our code base tomorrow to see how DraggableCollectionView works!

[lxcid]

True though. It was based on iBook and thus the reason it data source got modified instead of keeping a temporary reference when dragging. Could you send a PR update the README with a link to your repo? I like users to be able to choose either or though. Thanks!

[lukescott]

@lxcid That sounds good. Although it might be best for you to update the README. I've already changed it on my fork, and I think you probably want to present it in a different way.

[fitch]

@lukescott I noticed that you've stated that "Multiple sections are not (yet) supported" - any news on that?

[lukescott]

@fitch Support for multiple sections has been added.

[lxcid]

So sorry I took so long but I had add the link in the README. Thanks @lukescott!