Skip to content

[ReactTransitionGroup] Entry transition only when adding to an existing group #536

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

Closed
wants to merge 2 commits into from

Conversation

ide
Copy link
Contributor

@ide ide commented Nov 14, 2013

Currently if you add a new ReactTransitionGroup with some children during a
render pass, the children are transitioned. This is generally not desirable
(e.g. if adding a transition-enabled list that already has some items, you don't
want to transition all of the initial items when first rendering the list).

This diff makes it so that entry transitions are applied only when the
transition group already exists. This is implemented by applying entry
transitions only when the group has already been mounted.

@petehunt
Copy link
Contributor

Talked on IRC; let's add a transitionOnMount prop so we can keep this behavior (but default it to false)

@ide
Copy link
Contributor Author

ide commented Nov 14, 2013

Added transitionOnMount and updated tests (and tested it in a browser).

@ghost ghost assigned ide Nov 19, 2013
@petehunt
Copy link
Contributor

petehunt commented Dec 1, 2013

OK, I'm going to work on getting this in this week.

container
);
expect(a.getDOMNode().childNodes.length).toBe(1);
expect(a.getDOMNode().childNodes[0].className).toBe('yolo-enter');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I run this in our test runner it has some extra whitespace. Can you trim() it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trimmed the className in the latest update, but before that I wasn't able to repro this issue by running grunt test locally.

@petehunt
Copy link
Contributor

petehunt commented Dec 4, 2013

Sadly this is going to have a conflict at the next sync. I think the best thing to do is to wait for @zpao to do it and rebase on top (I'm not sure what will happen if I fix the conflict internally and merge here... will probably "just work" but I'm not 100% sure how that works)

@sophiebits
Copy link
Collaborator

I assume @zpao would then encounter a conflict when syncing…

@ide
Copy link
Contributor Author

ide commented Dec 4, 2013

I think the right thing is for me to update my fork i.e. go ahead and break this.

@zpao
Copy link
Member

zpao commented Dec 5, 2013

We're all synced so do your thang

@ide
Copy link
Contributor Author

ide commented Dec 5, 2013

pusheen_party

@sophiebits
Copy link
Collaborator

image

ide added 2 commits December 16, 2013 15:43
…ng group

Currently if you add a new ReactTransitionGroup with some children during a
render pass, the children are transitioned. This is generally not desirable
(e.g. if adding a transition-enabled list that already has some items, you don't
want to transition all of the initial items when first rendering the list).

This diff makes it so that entry transitions are applied only when the
transition group already exists. This is implemented by applying entry
transitions only when the group has already been mounted.
@brycekahle
Copy link

👍

@sophiebits
Copy link
Collaborator

In ReactCSSTransitionGroup now I believe the transition happens only for new elements, so I'm going to close this out. (And ReactTransitionGroup should be flexible enough to do any custom logic you might want.)

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

Successfully merging this pull request may close these issues.

5 participants