Experimental event API: rework the propagation system for event components #15462
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
be56d10 to
cff6f3e
Compare
Correct Drag and Swipe modules Fix tests Fix typo
cff6f3e to
6c5f318
Compare
|
@necolas it would be good if you could validate this PR locally when you get a chance against your codesandbox. Thanks! |
|
ReactDOM: size: 0.0%, gzip: -0.0% Details of bundled changes.Comparing: 5876769...c9a7fa7 react-dom
react-events
Generated by 🚫 dangerJS |
necolas
reviewed
Apr 22, 2019
There was a problem hiding this comment.
Looks good. I can test this patch in the sandbox after the merge conflicts with master are resolved.
About the names, we could probably follow existing naming convention in React for bubble vs capture phase, e.g., onClick/onClickCapture.
onBubbledTargetEvent -> onEvent or onTargetEvent
onCapturedTargetEvent -> onEventCapture or onTargetEventCapture
Might be worth avoiding the word target though as it's used in a few places to mean different things at the moment.
|
@necolas I've fixed the conflicts with master and applied the name change that you suggested. I'll let you test it first – if it's all good, feel free to merge or wait till I'm back in the office. :) |
necolas
approved these changes
Apr 22, 2019
There was a problem hiding this comment.
Sandbox (updated to use this patch) seems to be working as before: https://fburl.com/nfye4390
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After much discussion with @necolas and quite a bit of internal testing, we came up with an idea to improve the experimental event API and its propagation system. Rather than return a
booleanfrom the event responderonHandleto indicate if an event should be propagated, all events are now propagated depending on the responder and the event component.Responders now have the
stopLocalPropagationoption (required). This tells the event system what it should do we encountering this particular event responder. If this flag is set totrue, then when we encounter the same event responder during propagation, we skip calling the event listeners on that responder for that event component.Furthermore, there was some confusion over the phases on the event object, and so now
onHandlehas been split into three methods:onEvent,onEventCaptureandonRootEvent.Ref #15257