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

done classname for appearing #327

Closed
koba04 opened this issue May 1, 2018 · 8 comments
Closed

done classname for appearing #327

koba04 opened this issue May 1, 2018 · 8 comments
Labels

Comments

@koba04
Copy link
Member

koba04 commented May 1, 2018

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

bug?

What is the current behavior?

    <CSSTransition in appear timeout={timeout} classNames="fade">
        {props.children}
    </CSSTransition>

The above case, CSSTransition applies the following classNames in the order.

  • .fade-appear
  • .fade-appear.fade-appear-active
  • .fade-enter-done

Is adding .fade-enter-done classname to a component an expected behavior?
I think it would be better the classname will be .fade-appear-done instead of .fade-enter-done.

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://codesandbox.io/s/7w59nmnn1x

What is the expected behavior?

A component is added .fade-appear-done instead of .fade-enter-done.

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

@silvenon
Copy link
Collaborator

silvenon commented May 1, 2018

I'm not entirely sure what would be appropriate. Maybe the best solution would be to add both classes. @jquense?

@silvenon silvenon added the bug label Jul 17, 2018
@silvenon
Copy link
Collaborator

I'll change this to fade-appear-done. That's going to be a breaking change for anyone who relied on this, but that's a weird behavior to rely on. 😅

@taion
Copy link
Member

taion commented Jul 24, 2018

This is sort of weird, though – would the "appear done" class ever be different from "enter done"?

I wonder if it would have been better for "appear" to be an extra class, so we'd have ended up with .appearing.enter-active or whatever.

@silvenon
Copy link
Collaborator

There are use cases where you'd want initial appearance to look differently. I think your idea is great, then people who want to apply different styles to appearing vs. entering could do something like :not(.appearing).enter-active, that way they won't have to override anything.

@silvenon
Copy link
Collaborator

Maybe appear instead of appearing, that way it's more generic and clear that it acts as a modifier to enter-* and exit-* classes.

@koba04
Copy link
Member Author

koba04 commented Jul 25, 2018

This issue is for the inconsistent naming.

.appear.enter-active is good for me.
In the case, I think we have to apply the naming convention to other classes like this.

.appear.fade-enter
.appear.fade-enter.fade-enter-active
.appear.fade-enter-done

It seems to be huge breaking changes though.

@silvenon
Copy link
Collaborator

Yeah. Another, less breaking solution would be to add fade-appear-done instead of fade-enter-done to the current behavior.

jquense pushed a commit that referenced this issue Apr 6, 2019
# [2.9.0](v2.8.0...v2.9.0) (2019-04-06)

### Features

* **CSSTransition:** add "done" class for appear ([fe3c156](fe3c156)), closes [#383](#383) [#327](#327) [#327](#327)

### Reverts

* bump semantic release dependencies ([1bdcaec](1bdcaec))
@jquense
Copy link
Collaborator

jquense commented Apr 6, 2019

🎉 This issue has been resolved in version 2.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

johnfrench3 pushed a commit to johnfrench3/transition-group-react that referenced this issue Nov 2, 2022
# [2.9.0](reactjs/react-transition-group@v2.8.0...v2.9.0) (2019-04-06)

### Features

* **CSSTransition:** add "done" class for appear ([fe3c156](reactjs/react-transition-group@fe3c156)), closes [#383](reactjs/react-transition-group#383) [#327](reactjs/react-transition-group#327) [#327](reactjs/react-transition-group#327)

### Reverts

* bump semantic release dependencies ([1bdcaec](reactjs/react-transition-group@1bdcaec))
patrickm68 added a commit to patrickm68/react-transition-group-developer that referenced this issue Dec 1, 2022
# [2.9.0](reactjs/react-transition-group@v2.8.0...v2.9.0) (2019-04-06)

### Features

* **CSSTransition:** add "done" class for appear ([fe3c156](reactjs/react-transition-group@fe3c156)), closes [#383](reactjs/react-transition-group#383) [#327](reactjs/react-transition-group#327) [#327](reactjs/react-transition-group#327)

### Reverts

* bump semantic release dependencies ([1bdcaec](reactjs/react-transition-group@1bdcaec))
shaikdev2 pushed a commit to shaikdev2/transition-group-react that referenced this issue Jun 9, 2023
# [2.9.0](reactjs/react-transition-group@v2.8.0...v2.9.0) (2019-04-06)

### Features

* **CSSTransition:** add "done" class for appear ([fe3c156](reactjs/react-transition-group@fe3c156)), closes [#383](reactjs/react-transition-group#383) [#327](reactjs/react-transition-group#327) [#327](reactjs/react-transition-group#327)

### Reverts

* bump semantic release dependencies ([1bdcaec](reactjs/react-transition-group@1bdcaec))
GreenCat1996 added a commit to GreenCat1996/react-transition-group that referenced this issue Aug 1, 2023
# [2.9.0](reactjs/react-transition-group@v2.8.0...v2.9.0) (2019-04-06)

### Features

* **CSSTransition:** add "done" class for appear ([fe3c156](reactjs/react-transition-group@fe3c156)), closes [#383](reactjs/react-transition-group#383) [#327](reactjs/react-transition-group#327) [#327](reactjs/react-transition-group#327)

### Reverts

* bump semantic release dependencies ([1bdcaec](reactjs/react-transition-group@1bdcaec))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants