Add support for command invokers API #32480
Conversation
|
Awesome! Can you provide a fixture that illustrates their usage? You can use https://react.new/ and then specify builds from the ci/codesandbox check on this PR |
|
Comparing: aac177c...7ac0be1 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
|
https://codesandbox.io/p/sandbox/happy-wilbur-crtsz9 - So this will show the in-built commands work aswell as showing that custom ones work, but for some reason only onCommandCapture works, not onCommand? I'm assuming I'm missing something in this PR to make onCommand work as expected. Though command events don't bubble so perhaps this is just due to React's synthetic events? Btw this will only work in Chrome Canary with the experimental flag enabled right now. (It's shipping to stable 135) |
eps1lon
force-pushed
the
command-invokers
branch
from
144b87e to
7e636fd
Compare
| @@ -3077,6 +3093,10 @@ export function hydrateProperties( | |||
| } | |||
| } | |||
|
|
|||
| if (props.onCommand != null) { | |||
There was a problem hiding this comment.
The target of these events won't be the element with the onCommand listener but the element that is the designated commandFor element. Not sure how to model that with our current event infrastructure. That doesn't explain why it doesn't work for commandFor="app" though.
There was a problem hiding this comment.
That makes sense right? You do <div id="foo" onCommand={handle}><button commandFor="foo" command="--foobar">...</button> and clicking the button causes the browser to fire a (non-bubbling) command event on the foo div. That shouldn't need special handling in react right?
There was a problem hiding this comment.
Though command events don't bubble, so I'm guessing I'm missing something somewhere to make the non capturing listener work with React's delegated event system?
There was a problem hiding this comment.
That's why you have to attach the listener to the dom element referenced in commandFor not the one where we want to listen for command events.
|
I'm going to go ahead and close this. I frankly don't have the energy to deal with React's legacy event system. If anyone else wishes to pick this up feel free to, I'm happy to explain how the API works. |
Summary
Command Invokers are shipping in chrome soon and are now in the HTML spec, this adds support to React.
Fixes #32478
How did you test this change?
I've not yet the docs are out of date and refer to a UMD build that no longer exists.