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

Add reload in place capability #65

Closed
wants to merge 1 commit into from

Conversation

igor-makarov
Copy link

As a user of TableViewDiffCalculator, I'd like to minimize unnecessary animations.

This PR transforms subsequent delete+insert calls to reload calls.

This also addresses #60.

@jflinter
Copy link
Owner

Thanks for the PR! While I'd very much like this functionality to exist, I have some issues with this implementation:

  • Information shouldn't be passed around in strings a la encodedValue; this loses a lot of type safety and makes the code harder to understand.
  • These changes should also appear in CollectionViewDiffCalculator.
  • This implementation will only handle insertions/deletions that are adjacent to one another, but Dwifft orders the edits by processing all deletions followed by all insertions. Thus many legitimate reloads would be missed by this approach. What is truly needed is the ability to "coalesce" an array of DiffSteps containing arbitrarily-ordered insertions and deletions into one containing insertions, deletions, and (presumably) some sort of new reload case. Unfortunately this is challenging with the involved index math.

The last of those points is the most serious, so I'd suggest thinking about it pretty carefully (and posting your thoughts here) before starting to implement anything. (It's a hard problem, which is why I've avoided it for so long.)

@jflinter jflinter closed this Jul 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants