Instagram / IGListKit

A data-driven UICollectionView framework for building fast and flexible lists.
https://instagram.github.io/IGListKit/
MIT License
12.87k stars 1.54k forks source link

[Proposal] Capability to update without reloading cells #451

Closed HarshilShah closed 7 years ago

HarshilShah commented 7 years ago

New issue checklist

First off, I recognise that this proposal is probably beyond IGListKit’s original scope and may not fit with the roadmap, so I’m a-okay with it being ignored or whatever. But anyway, here goes:

Animating changes between the model for when objects to be displayed are updated is, well, what IGListKit was built for, and is excellent at. This proposal intends to make it better at handling changes in properties of model items.

For example, consider a music app, with a collection view showing all the songs within a particular album. You may need to update a cell when selected/when the previous track ends to indicate that it is now playing, and also update another cell which was previously playing. Or you may need to update the cell colours (cough dark mode cough). Or you may need to update a song’s rating when an asynchronous query returns.

The IGListKit way to update cells, using performUpdates on the adapter, means you just have a fresh set of cells dequeued and loaded in instead. You still get the state changes, but not the animations.

(Caveat: You can probably still achieve animations by getting the IGSectionType to talk directly to your model, listening for notifications, etc., but that’s probably architecturally unsound and loses a lot of the elegance of IGListKit.)

I’m not sure how exactly this would work on the API end, but I would very much like if there was a way to pass in new data that is applied to the existing cells in a similar manner to performUpdates, but instead of reloading the model and dequeuing and setting up new cells, a configuration object (could be a parameter or a separate delegate function) is used to update already instantiated cells.

I realise that this adds some issues of its own with respect to maintaining concurrency between the view and model, but I still thought it was at least worth having a discussion about regardless.

Hope all this made sense, thanks for reading!

zhubofei commented 7 years ago

@HarshilShah I think this proposal is duplicated with #127 & #449.

HarshilShah commented 7 years ago

@zhubofei They do look similar for sure, but not the same. What I’m asking for is a bit closer to a new diff notification type.

zhubofei commented 7 years ago

@HarshilShah What about #449?

HarshilShah commented 7 years ago

@zhubofei Yes, that’s pretty damn close! Anything that lets you update cells without starting from 0, basically

zhubofei commented 7 years ago

@HarshilShah Can I close this one?

HarshilShah commented 7 years ago

Yeah sure, #449 is close enough

rnystrom commented 7 years ago

@HarshilShah awesome stuff. Ya I definitely want a good approach to doing this. #449 is a way for IGListKit to handle it for you, but in Instagram we do this manually:

It's really tedious though, and if something comes from the top-down, you're right, the performUpdates: causes animations. It's fine for the most part, but sometimes the animations are overkill.

To do this a "bindable" cell and a view-model are required. I think #449 will be a step in the right direction and there might be more we can do w/ it.

HarshilShah commented 7 years ago

@rnystrom Currently using pretty much the same setup to update cells. Can’t wait for #449 to be ready!