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

Not enough time between 'exited' and 'entering' for DOM to perform enter transition #159

Closed
jalooc opened this issue Aug 14, 2017 · 5 comments

Comments

@jalooc
Copy link

jalooc commented Aug 14, 2017

Do you want to request a feature or report a bug?
Bug (or there's sth wrong with my code)

What is the current behavior?
During entering, the exited state lasts for minimal amount of time, but sometimes it appears to be too little, such that the browser doesn't render the style associated to that state and the enter transition doesn't work.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/m4mb9Lg3/4/).
https://jsfiddle.net/m4mb9Lg3/25/
Note, that exit transition works properly, whilst entering is not transitioned. In console you can see however, that the state prop is passed correctly and goes through it's whole cycle.

The other day, doing enter animations in pure React stateful component (no react-transition-group), something similar occurred:

I would have an enteredActive state, initially set to false. In componentDidMount, inside requestAniationFrame callback I would set it to true. This would trigger rendered styles change and thus animation. That's fine except it's not, because oftentimes (not always) this wouldn't work. What fixed it permanently, was to put the rAF inside another rAF, like this:

componentDidMount() {
  requestAnimationFrame(() =>
    requestAnimationFrame(
      () => this.setState({ enteredActive: true })
    )
  )
}

My suspicion is that it has something to do with React internals.

So to the point: maybe the double-raf-technique inside Transition component would help? Or yet, there's sth wrong with my codepen?

Which versions, and which browser / OS are affected by this issue? Did this work in previous versions?
All of them.

@orballo
Copy link

orballo commented Aug 16, 2017

I have this same problem. I found an old issue about this same topic but in the CSSTransition component. I can't locate it right now but they solved it forcing a reflow using node.scrollTop. Here is the code:

reflowAndAddClass(node, className) {
    // This is for to force a repaint,
    // which is necessary in order to transition styles when adding a class name.
    /* eslint-disable no-unused-expressions */
    node.scrollTop;
    /* eslint-enable no-unused-expressions */
    addClass(node, className);
}

So, we used this same solution in the onEnter callback and works fine:

<Transition
  in={isOpen}
  timeout={350}
  mountOnEnter
  unmountOnExit
  onEnter={node => node.scrollTop}
>

The <Transition> is ideal for us to work with styled-components instead of using <CSSTransition>. So, my suggestion would be to add a prop called reflow that forces this same behaviour with <Transition> like this:

<Transition
  in={isOpen}
  timeout={350}
  mountOnEnter
  unmountOnExit
  reflow
>

@jalooc
Copy link
Author

jalooc commented Aug 16, 2017

@orballo Nice, I also moved to <Transition> due to styled-components :)

As to the reflow technique - it seems hacky as heck. I'm almost sure that double rAF() solves the problem, although I don't like the fact that I don't know why (any hints anybody?). Since my suspicion being the internals of React and React internals change a lot in 16.x, this solution still can be a subject to failure.

@jquense
Copy link
Collaborator

jquense commented Aug 16, 2017

Hey folks, There is a reason why Transition doesn't have any of these bits in it and CssTransition does. Transition is intended as a generic, potentially platform agnostic, transition component. Meaning you it handles the state tracking between mounted and unmounted (or in/out) and you can apply sematic meaning there,

Adding something like a rAF call would invariably only support the CSS transition case, and even then not entirely. Same with with adding a reflow directly to the component, it would be very DOM specific and not always the choice for even css animations.

It is intended that ya'll implement the reflow/rAF logic on top of the Transition component, in a way that meets your specific needs. For folks that want something a bit more together, we offer the CSSTransition component to cover a fair number of general uses. In terms of which appaoch is correct for your own needs, either are ok. Personally I think the reflow technique, while hacky seeming. is the most correct approach. Using rAF accomplishes the same thing by waiting until the browser reflows itself, but takes some time and can be inconsistent. The reflow approach is a fairly common way to handle this (we do it in CSSTransition) tho if you're worried about the hack

@im-martijn
Copy link

im-martijn commented Oct 20, 2017

This looks like the exact problm I have. The style is applied when entering from an exited state, but the transition I'm giving in my styles isn't being shown. I just get the entering style without it transitioning. I tried the reflow method. I used a function

function reflow(node) {
  /* eslint-disable no-unused-expressions */
  node.scrollTop;
}

and set that on onEntering and onExiting in my Transition component, just like in CSSTransition.js.

But I still get no css transition when entering from an exited state. I do get the css transition if I go to the entering state from the exiting state instead.

I'm using the Transition instead of CSSTransition because I'm using Glamor for the styling. Any thoughts on why the reflow function might not be working for me like it does for you?


EDIT: for now I'm working around this by using CSSTransition. Using glamor to apply styles to the classNames.

@oliviertassinari
Copy link
Contributor

oliviertassinari commented Oct 22, 2017

For anyone needing the rAF workaround, we have been using the following code on Material-UI:

/**
 * A helper function that calls back when any pending animations have started. This is needed as the
 * callback hooks might be setting some style properties that needs a frame to take effect.
 */
function requestAnimationStart(callback) {
  // Feature detect rAF, fallback to setTimeout
  if (window.requestAnimationFrame) {
    // Chrome and Safari have a bug where calling rAF once returns the current
    // frame instead of the next frame, so we need to call a double rAF here.
    // See https://crbug.com/675795 for more.
    window.requestAnimationFrame(() => {
      window.requestAnimationFrame(callback);
    });
  } else {
    setTimeout(callback, 0);
  }
}

However, we had to fork react-transition-group/Transition to make it work. This was some time ago (1 year ago). Let's see if we can do better with the new version (v2) of the lib :).

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

5 participants