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

Appear breaks if descendant computes styles in componentDidMount #10

Open
TrevorBurnham opened this issue Jan 29, 2017 · 7 comments
Open

Comments

@TrevorBurnham
Copy link

Previously reported as facebook/react#3672. I've created a CodePen with a simple example: http://codepen.io/TrevorBurnham/pen/ZLvGXz

The problem is that the CSS Transition Group's descendants' componentDidMount operations are performed after the CSS Transition Group child is in the DOM but before that child receives the "appear" class. Since CSS transitions begin whenever a DOM element's computed styles change (per the spec), that causes a transition from the child's pre-"appear" state into its "appear" state, which is likely to affect the subsequent transition to the "appear-active" state.

For example, in the CodePen above, the pre-"appear" state has transform: scale(1), the "appear" state has transform: scale(0), and the "appear-active" state has transform: scale(1). If we don't force style computation between the first two states, then we get a single transition from the "appear" state to the "appear-active" state, going from 0-scale to full-scale over 2s. But if a descendant calls document.body.clientWidth during its componentDidMount, we get 2 CSS transitions: one from the pre-"appear" state to the "appear" state, then a reversal just a few milliseconds later when the "appear-active" class is applied. When that happens, it looks like no transition occurred at all!

It should be possible to prevent this from happening by applying the "appear" classes to the CSS Transition Group child(ren) during render instead of waiting until componentDidMount.

@jquense
Copy link
Collaborator

jquense commented Mar 16, 2017

Hi there, sorry for the delay. We really can't apply the classes during render, since there is no DOM node during the first "appear" render.

I'm not sure tho that the current behavior (while definitely annoying for you) is wrong. I'm really not sure what's actually happening though. I think your explanation is a bit off. Checking the order, there isn't two transitions, since no transition property is changed. It is causing a reflow which does seem to cancel the upcoming animation but I am not sure why since a reflow before adding the classes really shouldn't do anything.

@TrevorBurnham
Copy link
Author

Thanks for getting back to me. I have a pen that demonstrates the same phenomenon with pure DOM methods rather than React, which may help clarify matters: http://codepen.io/TrevorBurnham/pen/GrOmBX

Per the CSS transition spec:

When a style change event occurs, implementations must start transitions based on the computed styles that changed in that event.

A style change event is defined as "when the computed value of an animatable property changes." So simply calling

document.body.clientWidth

between the element being mounted and the class being added to it triggers a style change event (because it forces style properties to be computed), starting a transition. Another style change event occurs after the class is added, starting a second transition.


Unfortunately, I think you're right that there's no way for TransitionGroup to add the class to the child DOM node before the child component's componentDidMount is called. Both TransitionGroup::componentDidMount and the ref callback are invoked too late. We'd need some kind of change in React itself to give us a hook with the proper timing.

It is possible to provide the "appear" className as a prop to the child. Here's a proof of concept: http://codepen.io/TrevorBurnham/pen/jBajBY But that'd be an API change, and an odd one at that.


So I'm reluctantly going to say that this problem can't be solved within the existing react-transition-group API. Any developers who stumble on to this issue should be relieved that there's an easy fix: Only define transition rules in the -active classes. Here's that change in action: http://codepen.io/TrevorBurnham/pen/rypNRg

The react-transition-group doc examples already employ this CSS pattern. It's very tempting to "de-duplicate" those rules by moving them to the element's base selector, though. This may be worth explicitly warning against in the docs.

@jquense
Copy link
Collaborator

jquense commented Mar 17, 2017

I'm generally aware of how checking certain properties causes the browser to reflow and cancel/break animations. However checking an unrelated property won't suddenly trigger an animation for properties that haven't changed. In your example, transform isn't altered or affected by a check to clientWidth so it's not going to try and animate it (because there is nothing to animate to).

We intentionally do a check like this between adding appear and appear-active actually to ensure that the classes are added in different reflows of the browser (otherwise they can start at the same time). I'm really not sure tho why a reflow triggered before the class is added would have any effect.

@jquense
Copy link
Collaborator

jquense commented Mar 17, 2017

I'd be happy to just add all classes on requestAnimationFrame but I'd need some expert insight on the effects of doing that. These quirks are just the worst :P Thanks for taking the time to put everything together

@TrevorBurnham
Copy link
Author

I'd be happy to just add all classes on requestAnimationFrame

I don't see what you mean. The child has 3 discrete states:

  1. Mounted in the DOM
  2. Has the appear class
  3. Has the appear-active class

Styles need to be computed between 2 and 3 in order for the transition from appear styles to appear-active styles to occur. That's accomplished by adding appear-active in a requestAnimationFrame callback.

The problem here is that the child's componentDidMount runs during state 1, potentially triggering a transition by causing styles to be computed. Adding the appear class in a requestAnimationFrame callback wouldn't change the order of events, because the callback would fire after the child's componentDidMount.

@jquense
Copy link
Collaborator

jquense commented Mar 18, 2017

I really don't think the reflow triggers any transition. why would it? if causing a reflow just started any transition, no animations would hardly ever run. the spec says a reflow will trigger an animation if the property changed, which isn't the case here. The general reason an animation is broken is because the initial state and end state are done in the same reflow, e.g.the browser paints the initial state then immediately the end state.

@TrevorBurnham
Copy link
Author

the spec says a reflow will trigger an animation if the property changed, which isn't the case here.

The computed style property changes when you trigger reflow. And per the spec, that's what starts a CSS transition.

Here's another example: http://codepen.io/TrevorBurnham/pen/GrOmBX The key code is:

  document.body.appendChild(box);
  box.style.backgroundColor = 'red';
  box.style.backgroundColor = 'blue';
  box.style.backgroundColor = 'gold';
  document.body.clientWidth;  // trigger reflow
  box.style.backgroundColor = 'green';

The visual result is a transition from gold to green. Move the trigger reflow up a line, and you'll see a transition from blue to green. Whatever backgroundColor is set to before reflow is triggered becomes the initial computed value of the element. Then when the JS finishes executing, the computed value becomes green, triggering a transition between the two computed values.

If you change the code to

  document.body.appendChild(box);
  box.style.backgroundColor = 'red';
  document.body.clientWidth;  // trigger reflow
  box.style.backgroundColor = 'blue';
  document.body.clientWidth;  // trigger reflow
  box.style.backgroundColor = 'gold';
  document.body.clientWidth;  // trigger reflow
  box.style.backgroundColor = 'green';

then you'll see a transition from red to green. The extra reflows trigger transitions to blue and gold, but those don't matter because no time elapses before they're superseded by the transition to green.

Or at least, that's my mental model of how reflows and transitions interact. If you have a better understanding of what's going on here, I'd like to hear it.

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

2 participants