reactjs / react-transition-group

An easy way to perform animations when a React component enters or leaves the DOM
https://reactcommunity.org/react-transition-group/
Other
10.18k stars 649 forks source link

Enter transition breaks when CSSTransition has mountOnEnter and child triggers reflow on componentDidMount #382

Open JohnAlbin opened 6 years ago

JohnAlbin commented 6 years ago

Do you want to request a feature or report a bug?

CSSTransition bug using react-transition-group 2.4.0.

What is the current behavior?

The enter transition fails to run, if:

Here's the smallest demo of the bug I could manage: https://jsfiddle.net/JohnAlbin/fq8oa109/68/

The demo includes 3 examples.

  1. The first example uses the same CSSTransition properties, HTML structure and CSS as the second example. It shows what the transition is supposed to look like.
  2. The second example is the same as example # 1 but the child element has a componentDidMount(). The transition does not work on enter. (It does work on exit, though.)
  3. The third example is the same as example # 2, but includes the "-enter" class on the child directly instead of having CSSTransition apply it. The transition is now fixed.

You need to press the big "Toggle Transition" button to make all three examples transition at the same time.

Regarding the DOM methods that break the enter transition: Not all DOM methods break it. For example, document.querySelector('body') and document.hasFocus() won't break the transition, but getClientRects() and focus() on DOM elements did break the transition. ~¯\_(ツ)_/¯ Not sure why that is.~ [edit: It's this list of DOM methods that force a reflow that breaks the transition: https://gist.github.com/paulirish/5d52fb081b3570c81e3a ]

What is the expected behavior?

The CSSTransition’s enter transition should work no matter the contents of the element that is being animated.

As shown in the first example of the demo above, the transition should have enter transition that has:

Which versions, and which browser / OS are affected by this issue?

Breaks in Chrome, Safari, and Firefox on Mac. Probably the same on Windows and Linux.

Did this work in previous versions?

Dunno. This is my first time using react-transition-group on a complex child. You could fork my demo and try old 2.x versions.

jquense commented 6 years ago

yeah, thats unfortunate :/ I'm not sure we can do anything about this in a general way. If you query the DOM in ways that trigger reflows (like measuring nodes), it's most likely going to interrupt the transition. This more of very annoying quirk of how css transitions work than problem with the transition components unfortunately

JohnAlbin commented 6 years ago

If you query the DOM in ways that trigger reflows (like measuring nodes)

A quick google search results in this list of DOM methods that cause a reflow: https://gist.github.com/paulirish/5d52fb081b3570c81e3aj

It includes all 3 of the DOM methods that I noticed caused this failed transition, so this type of DOM method probably is what triggers the bug.

This more of very annoying quirk of how css transitions work than problem with the transition components unfortunately

Then, why does the work-around (# 3 in the demo) fix the problem?

Specifically, the enter transition does not work with this:

<CSSTransition
  in={this.state.toggle}
  timeout={4000}
  classNames="shift"
  mountOnEnter
>

But the enter transition does work with this:

<CSSTransition
  in={this.state.toggle}
  timeout={4000}
  className="shift-enter"
  classNames="shift"
  mountOnEnter
>

To me, this hints that maybe there is something the react-transition-group component can do?

thomasboyt commented 5 years ago

I lost a decent chunk of a morning to this issue, which I understand is totally not the fault of the library, just a quirk of how CSS reflows work. That said, it'd be great if the docs had a section on this issue. I've been able to work around it with a quick setTimeout(fn, 0) in my componentDidMount() methods, where fn is the bit of code I need that causes a reflow.

As a real-world example of an issue, I use nuka-carousel in one of my pages. My pages have a full-page slide in/out transition powered by <CSSTransition />, but nuka-carousel does some page sizing calculations in componentDidMount(), which was breaking my enter transition. The easy fix, of course:

class CarouselWrapper extends React.Component {
  constructor(props) {
    super(props);
    this.state = { showCarousel: false };
  }

  componentDidMount() {
    setTimeout(() => {
      this.setState({
        showCarousel: true,
      });
    }, 0);
  }

  render() {
    return (
      {this.state.showCarousel && <Carousel>{/* ... */}</Carousel>}
    }
  }
}

It'd be great if CSSTransition had a bit of documentation saying something like:

Due to quirks of how CSS reflows work (see https://github.com/reactjs/react-transition-group/issues/382), rendering child components inside a transition that cause page reflows in their componentDidMount() hook may break entering transitions on mount. Any components that use JS for sizing, such as carousels [...], are likely to do these sorts of calculations. A simple, if ugly, fix is to simply defer rendering these components until after the initial render (...)

sergivillar commented 5 years ago

Hi guys, I face the same issue today, I'm working on a large project and I don't want to add setTimeout(0) for each component that make some maths (reflow) on componentDidMount, did you found any other solution? maybe is it possible to delay the start of the animation? this way the component will have time to execute the componentDid Mount and calculate the reflow

a9udn9u commented 4 years ago

I also lost at least 3 hours on this issue today. +1 to thomasboyt@'s comment, please add this issue to the document.