[ASDataController] "Move" Operations Animate Incorrectly (Delete + Insert)

[Adlai-Holler]

This is a minor issue but I want to get it on the board while we're thinking through table view updates in #684 / #693. It results from us splitting moves into delete+insert in ASDataController.

I believe this issue is important because it highlights a really common architectural problem when dealing with table views: interpreting changes merely as array updates and discarding change metadata. For example, interpreting move as delete,insert or interpreting a reload section as remove all items, add all items which ignores section metadata like header views.

Since our updates are already required to be on the main thread, maybe we should split each intercepted call into two parts: the intact update, which we forward to the table view, and the array update where we interpret the update as above (e.g. move -> delete + insert).

Naturally all this applies to collection views as well.

[appleguy]

@nguyenhuy @levi @eanagel have any of you thought about how we might fix this, or encountered it in your apps? I'm not certain how high priority this is compared to the many other improvements / refinements we might implement, especially since ASDataController is now substantially improved and appears to be mostly stable (no crashes in Pinterest after a week of heavy public use). Still though, it would be really great to ensure ASDK remains super animation-friendly.

[smyrgl]

@appleguy @Adlai-Holler I was about to open a new issue for this but I found this while searching so I decided to drop my own take on this here.

So here is the problem as I see it with this behavior: because ADK treats moves as delete->insert that means that there is no way to tell in the context of a custom UICollectionViewLayout that this is a move. When I perform a move op prepareForCollectionViewUpdates: is called on my UICollectionViewLayout with an update array with a pair of updates with the action set to .Delete and .Insert. That means that all context is lost. So lets say I wanted to do something like this:

    override func initialLayoutAttributesForAppearingItemAtIndexPath(itemIndexPath: NSIndexPath) -> UICollectionViewLayoutAttributes? {
        if let attributes = layoutAttributesForItemAtIndexPath(itemIndexPath)?.copy() as? UICollectionViewLayoutAttributes {
            let returnAttrs = attributes
            var returnFrame = attributes.frame
            returnFrame.origin.y -= returnFrame.height
            returnAttrs.frame = returnFrame
            returnAttrs.alpha = 0.1
            return returnAttrs
        }
        return super.initialLayoutAttributesForAppearingItemAtIndexPath(itemIndexPath)
    }

    override func finalLayoutAttributesForDisappearingItemAtIndexPath(itemIndexPath: NSIndexPath) -> UICollectionViewLayoutAttributes? {
        if let attributes = layoutAttributesForItemAtIndexPath(itemIndexPath)?.copy() as? UICollectionViewLayoutAttributes {
            let returnAttrs = attributes
            var returnFrame = attributes.frame
            returnFrame.origin.y -= returnFrame.height
            returnAttrs.frame = returnFrame
            returnAttrs.alpha = 0.1
            return returnAttrs
        }
        return super.finalLayoutAttributesForDisappearingItemAtIndexPath(itemIndexPath)
    }

Unfortunately because insert/delete is being used this will trigger the wrong animation. As a general rule it doesn't much matter how ADK operates internally as long as it doesn't leak implementation details through to the UITableView/UICollectionView but I think this would definitely qualify as one of those cases.

[leotumwattana]

As @garrettmoon mentioned, might be better to keep duplicate issues together. So I'll repost my previous issue here: #2845

We've been rebuilding parts of our app as a feasibility test for using ASDK. So far ASDK has been awesome and we've resolved most of the issues except for one thing. The moveItem: animation for ASCollectionNode.

For our app, the moveItem: animation is actually quite critical to help inform our users to changes. A simple fade-out-fade-in animation provides a lot less visual cue to the eyes.

@Adlai-Holler: I have a colleague who's much more comfortable with Objc than I am. If you will not be able to get to this issue anytime soon, mind if my colleague contact you and see if he can help get this issue moving?

Thanks very much in advance!

[Adlai-Holler]

Hey @leotumwattana, yes please do point your colleague to me and we can talk about what will need to happen to support moves natively!

[harryworld]

@Adlai-Holler Thanks for reply. I am sending you a message on Slack, as well as checking the PR before.