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

Nesting Flippers #81

Closed
mayorandrew opened this issue Jun 15, 2019 · 5 comments
Closed

Nesting Flippers #81

mayorandrew opened this issue Jun 15, 2019 · 5 comments

Comments

@mayorandrew
Copy link

I've got an interesting use case for this library, where I nest two flippers in order to have multiple independent flipKeys instead of just one global.

I have a Message component which can be opened or closed. It contains a Flipper and a Flipped.
Inside a Message there are multiple MessageItem components, each of which can also be opened or closed. Therefore I put a second Flipper and a Flipped inside the MessageItem component.

But the MessageItem animation does not work correctly. The inverse transform is applied after rerender, but the item does not animate back to the normal state.

If I remove the second Flipper and propagate the MessageItem state up to the Message, everything works fine.

I've made this codesandbox to show the issue: https://codesandbox.io/s/react-flip-toolkit-wrong-width-fmktp
There are code blocks commented as case 1 (two Flippers) and case 2 (one Flipper) in the demo. You can comment them out in order to switch between cases.

I think nesting Flippers opens more composability possibilities than having just one Flipper per subtree.

Investigation

In hopes of a quick fix I forked you repo and added new branch nested-flipper in which I've added the new demo NestedFlipper similar to the codesandbox above.

I did some quick debugging and nailed the issue down to the following call:

  const topLevelChildren: TopLevelChildren = filterFlipDescendants({
    flipDataDict,
    flippedIds,
    scopedSelector
  })

which returns empty array, so the animations are not applied.

I suppose that somewhere there is an assumption that Flipped could not be parent of Flipper, so the filterFlipDescendants does not correctly extract the downmost Flipped as the topLevelChild of second Flipper.

I've tried to look into that function but got stuck with the selectors logic for now.
So I've decided to open this issue and possibly get some feedback before I'll have an opportunity to dig deeper,

@mayorandrew mayorandrew changed the title Are nested Flippers possible? Nesting Flippers Jun 15, 2019
@aholachek
Copy link
Owner

Hi, thanks for pinpointing this problem. The code you found,"filterFlipDescendants", is some (probably overly complex) code for enabling nested staggers, and it causes the problem by not selecting direct children correctly.

I have a beta release that should fix it, testing it on your example (which i will add to the repo as well): it seems to fix the problem: https://codesandbox.io/s/react-flip-toolkit-wrong-width-696xu

I'll try to make a patch release soon.

@mayorandrew
Copy link
Author

Wow, that is azaming!
I've looked at the new version and it does indeed work great.
And old demos seem to not be broken, which is a good sign.
Thank you very much for fixing that quickly 🛠

@DimitryDushkin
Copy link

Hey @mayorandrew On what version did you manage to fix your nested Flipper problem?

@mayorandrew
Copy link
Author

Hi @DimitryDushkin, this problem in this issue is fixed in version 6.6.4

@DimitryDushkin
Copy link

Thanks! Turned out my problem was in constantly re-mounting Flipper component, so nested thing didn't work.

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

No branches or pull requests

3 participants