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

transitioning class is sometimes not removed #23

Open
Megamouse opened this issue Feb 3, 2020 · 2 comments
Open

transitioning class is sometimes not removed #23

Megamouse opened this issue Feb 3, 2020 · 2 comments

Comments

@Megamouse
Copy link

Chrome and Firefox:
Sometimes, when putting nested children with conditional maps inside of the open Slidedown, the endTransition function won't be called after some child component was mounted.

My guess is that the onTransitionEnd event won't always be fired for some reason, but I haven't found out why yet.

@DiFuks
Copy link

DiFuks commented May 27, 2020

This problem occurs in this line:

https://github.com/frankwallis/react-slidedown/blob/master/lib/slidedown.tsx#L92

Suppose that when changing the component props, its height has not changed, and closed = true. Then

    endHeight = getComputedStyle(this.outerRef).height

The endHeight variable will turn out to be "auto".
The line below parseFloat (endHeight) will return "NaN", the condition will be satisfied, and the transitioning class will be assigned to the component.

if (parseFloat(endHeight).toFixed(2) !== parseFloat(prevHeight).toFixed(2)) {
    this.outerRef.classList.add('transitioning')
    // ...
}

The animation is not triggered, and the endTransition method will not be called, since the height has not changed. Therefore, the transitioning class will remain with the component.

As a solution, I see:
To get the height of a component is not using getComputedStyle, but getBoundingClientRect. So it is used in getSnapshotBeforeUpdate.

@jameswilson
Copy link

jameswilson commented Feb 25, 2021

I can confirm this bug affects nested <SlideDown> components where the outer component defaults to open, and the inner component defaults to closed.

Here is a sandbox demonstrating the bug:
https://codesandbox.io/s/eloquent-zhukovsky-p2lo7?file=/src/App.js

slidedown-bug

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