[ASTableView] Add Support for Interactive Reordering

[Adlai-Holler]

No description provided.

[Adlai-Holler]

This is confirmed to work in Pinterest. Ready for review @levi @appleguy @maicki

[ay8s]

@Adlai-Holler Awesome work on this, curious if this would be something fairly easy to add support for under UICollectionView interactive movement? #2117

[Adlai-Holler]

@ay8s Great idea! I've updated the diff to add support for collection view reordering as well.

[ay8s]

Nice @Adlai-Holler. Going to give this a go and see if it fixes the issues I was seeing. Curious if you know if it'd be possible to relayout the items when reordering is initiated to shrink the cells a little to make ordering quicker for the user as some of our cells are quite large so may explore hiding nodes or truncating the ASCellNode to a set height? Might be out of the scope of AsyncDisplayKit.

[ay8s]

@Adlai-Holler Curious if you're seeing an assertion failure when moving within the ASCollectionView?

Assertion failure in -[BFRUpdateNode dealloc], /Users/andrewyates/Apps/Buffer/buffer-ios/App/Pods/AsyncDisplayKit/AsyncDisplayKit/ASDisplayNode.mm:431

[Adlai-Holler]

@ay8s Thanks for testing this out!

• Resizing all the cells should be possible by changing your cell nodes' layouts, and/or changing the return value for constrainedSizeForNodeAtIndexPath: if you implement it, and then calling relayoutAllItems on the collection view. If you're using flow layout, or if your custom layout knows about the change, all the items should be resized.
• Could you provide more detail about the assertion failure? Is it Node should be marked invisible before deallocation? Would you be willing to provide sample code to demo the issue?

[levi]

Can you simplify this duplication with an inline function or method or something?

[levi]

Looks good and really simple!

[ay8s]

@Adlai-Holler Probably could have done a better job at an example here but if you move the items around a few times you should run into that assertion failure. I'm working on implementing it still using a custom flow layout to move the cells as you scroll.

Example: https://cl.ly/2w1Y1b3i3f1c

[Adlai-Holler]

@ay8s OK great, your sample app helped me iron out some remaining issues with interactive movement. Thank you!

I never reproduced the assertion failure you mentioned. Now, with the latest code, the interactive movements and reloadDatas seem to be running very well.

[ay8s]

@Adlai-Holler Glad I could help with something. Will give it a whirl tomorrow and see how it fairs in the Buffer iOS app. Will try and put together something if I come across any oddness.

Edit: @Adlai-Holler curious if you have a demo for this at all? I tried hooking it up and while it doesn't trigger any crashes now it doesn't seem to handle cells of different heights too well which could totally be down to requiring a relayout. The cells seem to overlap and sometimes vanish during/after a reorder.

[Adlai-Holler]

@ay8s Hi! I continued building collection view reordering support, but unfortunately it's going to take more development and testing than I think belongs in this diff. So I reduced this diff back to just table view reordering and I'll land it.

I left the AHCollectionReordering branch up so we don't lose the knowledge gained. Here's an info dump about what will be required for collection view interactive reordering:

• Collection view doesn't call data source collectionView:moveItemAtIndexPath:toIndexPath until -endInteractiveMovement.
• In updateInteractiveMovementPosition collection view calls -moveItemAtIndexPath:toIndexPath: on itself if the pending target index path has changed.
• It's a special kind of move, where the data source counts are required not to change (since the data source shouldn't know about it). So we'll have to fake the numberOfItemsInSection: return value during interactive moves, to pretend we don't know about the pending move.
• At the same time, we DO need the data controller to move the node after each move update or else the range controller will make mistakes in assigning interface states to each node.
• We need to update the range controller after each such move. This is easily accomplished by adding -setNeedsUpdate after calling [super moveItem:] if this is an interactive move.
• Just before endInteractiveMove we need to put the node back where it was and reset our fake data source counts, so that when we get the real collectionView:moveItemAtIndexPath:toIndexPath: call, we do the update like normal.
• We'll probably need to add two ivars: sourceInteractiveMoveIndexPath and pendingInteractiveMoveIndexPath. We can use these to fake our data source item counts after each interactive move update, and to move the dragged node around to follow the cell.
• All of the above would work assuming they don't edit the collection view during interactive movement. I'm not sure how well UICollectionView handles this case, even.

@ay8s Hopefully I or someone on the team can get collection view interactive reordering built soon. If you're interested in working on it, please ping me on the asyncdisplaykit slack channel! We can pick up the discussion at #2117

[ay8s]

@Adlai-Holler Sounds good. We actually ended up switching back to the table view as it seems to handle orientation changes and bounds changes a little smoother for our next release. I believe we'll still need the CollectionView reordering in the future however so I'll be sure to ping you if I get chance to take a look. Still early days with ASDK so keen to get to grips with more of it.

Thanks for exploring it as far as you did! 👍