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.1k stars 652 forks source link

Element appears with blinked when using CSSTransition with `unmountOnExit` #846

Closed Mookiepiece closed 1 year ago

Mookiepiece commented 1 year ago

version: 4.4.4

What is the current behavior?

CSSTransition with unmountOnExit will render it's child without -appear or -enter className attached for the first frame when it appears.

What is the expected behavior?

Element appears with -appear or -enter.

Could you provide a CodeSandbox demo reproducing the bug?

This is caused by #749

koba04 commented 1 year ago

@Mookiepiece Thank you! I'll figure out how react-transition-group should behave. I've created a CodeSandbox project that works with v4.4.4 based on your example. https://codesandbox.io/s/jovial-marco-17xo6r I hope this helps you.

Mookiepiece commented 1 year ago

@koba04

Yes I'll share what I found with you.

As we know, enter + enter-active transition works well with CSSTransition, and not working with Transition. This is because we triggered reflow in CSSTransition.

https://github.com/reactjs/react-transition-group/blob/master/src/CSSTransition.js#L198

but in that pr, you introduced another approach which is more modern which is rQA. which delayed the entire performEnter process and CSSTransition is not the host now.

Here's my suggestion which respects our original approach (I don't fully tested it ):

      if (nextStatus === ENTERING) {
        if (this.props.unmountOnExit || this.props.mountOnEnter) {
          document.body?.scrollTop; // <- replace with this
        }
        this.performEnter(mounting);      
      } else {
        this.performExit();
      }

Or we can add an before entered hook ?

        if (this.props.unmountOnExit || this.props.mountOnEnter) {
          this.beforeEnter('Hey CSSTransition its your turn now');
          nextTick(() => this.performEnter(mounting));
        }
koba04 commented 1 year ago

@Mookiepiece Thank you! Triggering a forced reflow is not good from a performance perspective, but it might make sense to avoid introducing another issue. We'll need to verify the approach works fine with all cases, so I'll work on it.

koba04 commented 1 year ago

I've created a PR for this. We need more tests to merge #847.

alejandrofdiaz commented 1 year ago

I've experienced test regressions during react-testing-library unit tests. Testing for component behavior after mounting a Transition element caused the mounting to be delayed. In case anybody found this helpful when looking for the error, obtaining Unable to find node on an unmounted component. error from react-testing-library. I expect this to be fixed after #847 .

koba04 commented 1 year ago

@alejandrofdiaz Thank you for your feedback! I'll merge #847 and reconsider a way to avoid forcing reflow later.

koba04 commented 1 year ago

I've merged #847, so this'll be fixed at v4.4.5

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 4.4.5 :tada:

The release is available on:

Your semantic-release bot :package::rocket: