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

Understanding why not-yet-entered div causes an exiting div to "jump" #77

Open
yang opened this issue May 22, 2019 · 9 comments
Open

Understanding why not-yet-entered div causes an exiting div to "jump" #77

yang opened this issue May 22, 2019 · 9 comments

Comments

@yang
Copy link

yang commented May 22, 2019

Thank you for this wonderful library!

I have an extremely basic example here:

https://codesandbox.io/s/serene-hofstadter-rhl0g

Clicking will exit the Page1 div and enter the Page2 div (there actually are no elements with the same flipId). The only difference between them is Page2 has a marginTop:100.

Currently Page1 first "jumps" down by 100px. If you break at the top of onExit, it appears Page1 is positioned at 0,0 relative to the outer container, but the outer container is itself getting nudged down by Page2 due to margin-collapsing.

Things are easy to work around in this toy example, but I was wondering - is this a bug? I'm not really sure what the right mental model is, but I naively thought that react-flip-toolkit was supposed to measure the actual position/size of all Flipped elements before a flip (relative to the document), and then ensure it's positioned there.

When I set the flipId for Page2 to "1" so that they transition, I do see that the starting position for the transition is where Page1 was (not the jumped position), just not if it's exiting.

https://codesandbox.io/s/quizzical-rubin-u3no1

Thanks!

@aholachek
Copy link
Owner

Thank you for this issue and providing illustrative test cases.
I think the "jump" in the first example is how I would think things would behave. FLIP is only available as a technique when one element is being morphed into another, not when an element is entering or leaving.

In the past, I've done e.g. page transitions where a page animates out and then a new page animates in, and I've always needed to use position: absolute on both pages in a relative container to prevent them from affecting eachother's layout. That's the technique I would also use in the first example.

@yang
Copy link
Author

yang commented May 22, 2019

OK, got it.

It would be a huge benefit to be able to exit elements from wherever they were positioned according to the normal layout (rather than only absolutely positioned elements), and it seems like react-flip-toolkit is in a good position to help with this, given that's the value prop of FLIP! The workarounds I can see are complex or require wrapping Flipper/Flipped so that I too can track all the Flipped elements in my app, and trigger a snapshot of their positions + adjust their positions before flipKey changes for exiting elements.

It would be amazing if Flipped had a flag adjustOnExit (hopefully by a better name) to set the position/size/etc. of an exiting element to whatever it would be if it were being transitioned. Does this make sense as a feature request?

@yang
Copy link
Author

yang commented May 22, 2019

Another thing I don't grok is that in the code, it does appear as if FLIP does try to position exiting elements correctly, according to their measured state before the update—the following is from animateUnflippedElements/index.js. It's just that the results are off, since the parent itself shifts. Somehow animateFlippedElements is getting the correct positioning, still wrapping my head around that....

  const onExitCallbacks = exitingElementIds.map((id, i) => {
    const {
      domDataForExitAnimations: {
        element,
        parent,
        childPosition: { top, left, width, height }
      }
    } = flippedElementPositionsBeforeUpdate[id]
    // insert back into dom
    if (getComputedStyle(parent).position === 'static') {
      parent.style.position = 'relative'
    }
    element.style.transform = 'matrix(1, 0, 0, 1, 0, 0)'
    element.style.position = 'absolute'
    element.style.top = top + 'px'
    element.style.left = left + 'px'
    // taken out of the dom flow, the element might have lost these dimensions
    element.style.height = height + 'px'
    element.style.width = width + 'px'
    ...

@aholachek
Copy link
Owner

aholachek commented May 22, 2019

Hmm, so right now I am applying an opacity of 0 to incoming elements , which is what is causing the jump you illustrated in your example. But, I could add an option to make them display:none instead, which would not cause the jump, and only switch to display:visible once exiting had completed.

@yang
Copy link
Author

yang commented May 22, 2019

I thought about that too, but that would prevent being able to perform simultaneous/overlapping enter and exit transitions (important for our use case)....

@yang
Copy link
Author

yang commented May 22, 2019

Also, just to braindump my understanding:

animateFlippedElements uses just the client rect (.rect). All it needs to do is offset the element by the delta between the final and initial client rects.

animateUnflippedElements uses the difference between the child and parent bounding client rects (.domDataForExitAnimations.childPosition). This is because we reinsert the child back into the parent. If we simply insert the child directly into document.body, then inherited styles and nested/contextual styles are lost (are there other reasons?). However it assumes that the parent does not move.

After reading this, I also realized that it assumes the parent itself is still attached. I created another example where the results are a bit surprising to me, where the parent is removed from the DOM—in this case, the exit animation is invisible:

https://codesandbox.io/s/crimson-browser-49d8e

Still thinking this through....

@yang
Copy link
Author

yang commented May 22, 2019

Here's what I'm thinking now:

To animate exiting elements no matter where they are in the DOM tree, even if their parents are removed, we'd need to reinsert them onto document.body (rather than said parents). In order to preserve exactly how it looked in its original place in the DOM tree, we'd need to deep-clone all the computed styles. This is a small code change but heavy handed in performance, but only at the outset, and would open up more flexible and robust transitions within react-flip-toolkit's same declarative interface.

I was curious whether other libraries do this, and saw that ramjet does:

https://github.com/Rich-Harris/ramjet/blob/master/src/utils/node.js

A flag on Flipped elements could opt into this. This flag would instruct getPositionsOfExitingElements to also record the computed styles of the sub-tree, to be applied later in animateUnflippedElements (who would also reinsert into document.body).

Some other tweaks are necessary—we'd also need to ensure that the client rect is accounting for collapsed margins. The reinserted element would be absolutely positioned and thus lose margin collapsing, resulting in an unexpected offset. Not sure if there are other similar things to watch out for.

I've only spent today thinking about this—I'd love to hear your thoughts and know whether this is a viable implementation path.

@aholachek
Copy link
Owner

This is cool, thank you for looking into it.

I have an undocumented helper component called ExitContainer that fixes the issue you pointed out about parents also being removed from the dom -- the exit container wrapper is treated as the parent if it is added. I kept it undocumented since no one has found the issue that you mentioned yet and I didn't want to unnecessarily complicate the API if no one needed it.

Your solution is probably more durable than ExitContainer since 1) it requires nothing additional of the user other than the flag and 2) it would avoid the problem this issue was initially about. However, it would potentially have z-index issues in addition to the margin collapsing you mentioned. I will make an exploratory pr, it may take a while though.

@jessepinho
Copy link

@aholachek just a heads-up that I also needed ExitContainer for transitions between routes where tons of parents get unmounted. Would love to have that documented!

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