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

Added support for synthetic animation/transition events #5835

Closed
wants to merge 56 commits into from
Closed

Added support for synthetic animation/transition events #5835

wants to merge 56 commits into from

Conversation

milesj
Copy link
Contributor

@milesj milesj commented Jan 12, 2016

After messing around trying to mimic and polyfill transition start/end/cancel events, I ended up just adding basic support for transitionend and some animations events.

I'm not sure how to test these since they are timed and the current jest/jsdom make it difficult, but they definitely work as I tested them in a browser alongside a few of my projects.

@jimfb
Copy link
Contributor

jimfb commented Jan 12, 2016

@spicyj @syranide

@milesj milesj changed the title Added new DirectElementEventPlugin Added support for synthetic animation/transitions events Jan 13, 2016
@facebook-github-bot
Copy link

@milesj updated the pull request.

@milesj milesj changed the title Added support for synthetic animation/transitions events Added support for synthetic animation/transition events Jan 13, 2016
@facebook-github-bot
Copy link

@milesj updated the pull request.

@milesj
Copy link
Contributor Author

milesj commented Jan 13, 2016

Would be nice to have this for React 0.15 👍

@milesj
Copy link
Contributor Author

milesj commented Jan 18, 2016

Bump.

@facebook-github-bot
Copy link

@milesj updated the pull request.

@milesj
Copy link
Contributor Author

milesj commented Feb 5, 2016

Anything? Even a simple note? :P

@jimfb
Copy link
Contributor

jimfb commented Feb 5, 2016

@spicyj @syranide

@sophiebits
Copy link
Collaborator

@jimfb Can't you review this?

@jimfb
Copy link
Contributor

jimfb commented Feb 6, 2016

@spicyj No, not intelligently. The event dispatch is still an inscrutable mystery to me, and you guys (I guess mainly you and @zpao) always have strong opinions about the way we name events/attributes. There are some diffs in this area that I can review myself, but this probably isn't one of them.

To me, if I had to decide between having this change and not having this change in my copy of React, I'd accept and fix later if needed. But you and @zpao like things to be more perfect before merging, which means this diff needs some more eyeballs.

You and @syranide have the best understanding of this code, which is why I sent it your way from the very beginning.

@milesj
Copy link
Contributor Author

milesj commented Feb 6, 2016

I can vouch that understanding the synthetic event system took quite some time.

bubbled: keyOf({onAnimationIteration: true}),
captured: keyOf({onAnimationIterationCapture: true}),
},
},
Copy link
Member

Choose a reason for hiding this comment

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

Can you keep these ABC ordered please

zeke and others added 21 commits February 8, 2016 19:46
This ensures that broken npm versions won't crash when doing install --production
(cherry picked from commit 7411245)
(cherry picked from commit e48343b)
Context was missing info on how to update the context after the initial render. Added a simple blurb and an example.

[Docs] 12-context.md code review changes
This should not be 1, since boolean properties always get set.
1. Add a handleHidden method to the BootstrapModal component.
2. Add an event listener for 'hidden.bs.modal' on the modal root
3. Add a new onHidden prop to the BootstrapModal component.
4. Add a new 'handleModalDidClose' method to the Example component to be used as the onHidden prop of it's modal component.
- Upgrade babel-eslint
- Ignore coverage
- Fix actual lint warning
Found a spelling mistake -
writting > writing
Because `process` object is globally available in Node.js.
@facebook-github-bot
Copy link

@milesj updated the pull request.

@milesj
Copy link
Contributor Author

milesj commented Feb 9, 2016

I have no idea what happened when I rebased off of master. I'm going to do a new pull request.

@facebook-github-bot
Copy link

@milesj updated the pull request.

@milesj
Copy link
Contributor Author

milesj commented Feb 9, 2016

Recreated here: #6005

@milesj milesj closed this Feb 9, 2016
@milesj milesj deleted the direct-element-events branch February 9, 2016 05:33
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.