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

fix: ordering of deleted items #626

Closed
wants to merge 1 commit into from

Conversation

aocenas
Copy link
Contributor

@aocenas aocenas commented Apr 7, 2019

This tries to fix the ordering of deleted items after merging them to out array. First tried in #605 but that had some issues. This tries to be more comprehensive fix of the reconciliation logic.

This seems to work on all things I tried (but again somebody should check after me as I did not notice the last issue in the examples), but I feel it can still be flaky. When there is a single item, its sibling pointer points to itself. So if items are added and deleted one by one there is no way to know the ordering. Right now the only example with notifications work, but I think it only works because the items are added/deleted in one specific order and would again break in other cases.

So I think this is a bit of an upgrade and should solve ordering when deleting multiple items in any order, but definitely does not solve all the ordering issues. For that there will probably need to be some refactoring of the items tracking in the transition animation.

@aocenas aocenas force-pushed the fix-ordering-of-deleted-items branch from b35580b to 9c52743 Compare April 7, 2019 12:12
@drcmda
Copy link
Member

drcmda commented Apr 11, 2019

thanks @aocenas ! @aleclarson let's merge this for v9?

@aleclarson
Copy link
Contributor

Sounds good! I'll review this PR sometime soon.

@aleclarson aleclarson self-requested a review April 11, 2019 13:41
@aleclarson aleclarson mentioned this pull request Apr 15, 2019
5 tasks
@aleclarson
Copy link
Contributor

Merged into #632 🎉

Great work! Thanks for contributing. ❤️

@aleclarson aleclarson closed this Apr 16, 2019
@aleclarson aleclarson removed their request for review April 16, 2019 11:27
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.

3 participants