search

joshwcomeau / react-flip-move

Effortless animation between DOM changes (eg. list reordering) using the FLIP technique.
http://joshwcomeau.github.io/react-flip-move/examples
MIT License
4.09k stars 258 forks source link

Fix Issue120 #180

Closed tobilen closed 7 years ago

tobilen commented 7 years ago

Adressing https://github.com/joshwcomeau/react-flip-move/issues/120

So after spending some time tracking down the issue demonstrated in https://github.com/joshwcomeau/react-flip-move/pull/180/files#diff-5bc397e1b32daa8794012a8ea12ce91aR17 (based off of @ConneXNL work ❤️ ), i found that upon deletion of the childData entry, state is not updated. this means no rerender is triggered, and when a rerender is eventually triggered, we still have a child without corresponding childData (i.e. a zombie).

i've console.logged the lifecycle of such an element in this commit: https://github.com/joshwcomeau/react-flip-move/pull/180/commits/7f6f5972a9141e9c29b2fd5bdbe137bcb2bcf49a

the fix implemented here updates state in the removeChildData call. this seems to be working fine, but i'd appreciate some feedback.

joshwcomeau commented 7 years ago

Cool, thanks for your work on this!! Don't have time to dig into this now, but I'll try to go through this weekend and give it a proper review.

Curious why so much of the diff appears to be style changes, though? While I happen to agree with many of the tweaks (this project is a couple years old now and so my conventions have become more community-aligned in that time), I'd like for those changes to be done in a separate PR, so they can be discussed/debated outside of the underlying fix.

tobilen commented 7 years ago

Curious why so much of the diff appears to be style changes, though?

muscle memory (i have prettier on a hotkey) :P

i've removed the style changes

Hypnosphi commented 7 years ago

FWIW, I would welcome adding Prettier to our setup (in a separate PR, of course)

joshwcomeau commented 7 years ago

FWIW, I would welcome adding Prettier to our setup (in a separate PR, of course)

Ditto. Been using Prettier on other projects and it's delightful.

joshwcomeau commented 7 years ago

@tobilen this is released, in v2.9.15 🎉

I've also sent an invitation to add you as a collaborator on this project. It's a policy I've seen others do with success, where once a high quality PR is landed, the author is invited to collaborate on the project. Collaborators can review code, merge PRs, etc. Note that there are no "strings" or anything, accepting the invitation doesn't mean you'll be tasked with anything. Totally up to you how much or how little you'd like to do :)

Feel free to review and merge any PRs that come in that you're confident won't break anything, but please don't merge any of your own work unless it's been reviewed.

Thanks again for your work on this!