-
Notifications
You must be signed in to change notification settings - Fork 47.4k
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
Conversation
@milesj updated the pull request. |
@milesj updated the pull request. |
Would be nice to have this for React 0.15 👍 |
Bump. |
@milesj updated the pull request. |
Anything? Even a simple note? :P |
@jimfb Can't you review this? |
@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. |
I can vouch that understanding the synthetic event system took quite some time. |
bubbled: keyOf({onAnimationIteration: true}), | ||
captured: keyOf({onAnimationIterationCapture: true}), | ||
}, | ||
}, |
There was a problem hiding this comment.
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
…p MUST_USE_ATTRIBUTE and manage all regular properties as attributes instead
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.
Found a spelling mistake - writting > writing
Because `process` object is globally available in Node.js.
@milesj updated the pull request. |
I have no idea what happened when I rebased off of master. I'm going to do a new pull request. |
@milesj updated the pull request. |
Recreated here: #6005 |
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.