Skip to content

Add support for command invokers API #32480

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.

Sign up for GitHub

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 1 commit into from
Closed

Add support for command invokers API #32480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions fixtures/attribute-behavior/src/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,21 @@ const attributes = [
},
{name: 'cols', tagName: 'textarea'},
{name: 'colSpan', containerTagName: 'tr', tagName: 'td'},
{name: 'command', tagName: 'button', overrideStringValue: 'show-popover'},
{
name: 'commandFor',
read: element => {
document.body.appendChild(element);
try {
// trigger and target need to be connected for `commandForElement` to read the actual value.
return element.commandForElement;
} finally {
document.body.removeChild(element);
}
},
overrideStringValue: 'popover-target',
tagName: 'button',
},
{name: 'content', containerTagName: 'head', tagName: 'meta'},
{name: 'contentEditable'},
{
Expand Down
20 changes: 20 additions & 0 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ let didWarnFormActionTarget = false;
let didWarnFormActionMethod = false;
let didWarnForNewBooleanPropsWithEmptyValue: {[string]: boolean};
let didWarnPopoverTargetObject = false;
let didWarnCommandForObject = false;
if (__DEV__) {
didWarnForNewBooleanPropsWithEmptyValue = {};
}
Expand Down Expand Up @@ -875,6 +876,21 @@ function setProp(
);
}
}
break;
case 'commandFor':
if (__DEV__) {
if (
!didWarnCommandForObject &&
value != null &&
typeof value === 'object'
) {
didWarnCommandForObject = true;
console.error(
'The `commandFor` prop expects the ID of an Element as a string. Received %s instead.',
value,
);
}
}
// Fall through
default: {
if (
Expand Down Expand Up @@ -3077,6 +3093,10 @@ export function hydrateProperties(
}
}

if (props.onCommand != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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?

Copy link
Author

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

listenToNonDelegatedEvent('command', domElement);
}

if (props.onClick != null) {
// TODO: This cast may not be sound for SVG, MathML or custom elements.
trapClickOnNonInteractiveElement(((domElement: any): HTMLElement));
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom-bindings/src/events/DOMEventNames.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export type DOMEventName =
| 'change'
| 'click'
| 'close'
| 'command'
| 'compositionend'
| 'compositionstart'
| 'compositionupdate'
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom-bindings/src/events/DOMEventProperties.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const simpleEventPluginEvents = [
'canPlayThrough',
'click',
'close',
'command',
'contextMenu',
'copy',
'cut',
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom-bindings/src/events/DOMPluginEventSystem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ export const nonDelegatedEvents: Set<DOMEventName> = new Set([
'beforetoggle',
'cancel',
'close',
'command',
'invalid',
'load',
'scroll',
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom-bindings/src/events/ReactDOMEventListener.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ export function getEventPriority(domEventName: DOMEventName): EventPriority {
case 'cancel':
case 'click':
case 'close':
case 'command':
case 'contextmenu':
case 'copy':
case 'cut':
Expand Down
9 changes: 9 additions & 0 deletions packages/react-dom-bindings/src/events/SyntheticEvent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,3 +600,12 @@ const ToggleEventInterface = {
};
export const SyntheticToggleEvent: $FlowFixMe =
createSyntheticEvent(ToggleEventInterface);

const CommandEventInterface = {
...EventInterface,
source: 0,
command: 0,
};
export const SyntheticCommandEvent: $FlowFixMe = createSyntheticEvent(
CommandEventInterface,
);
4 changes: 4 additions & 0 deletions packages/react-dom-bindings/src/events/plugins/SimpleEventPlugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
SyntheticClipboardEvent,
SyntheticPointerEvent,
SyntheticToggleEvent,
SyntheticCommandEvent,
} from '../../events/SyntheticEvent';

import {
Expand Down Expand Up @@ -167,6 +168,9 @@ function extractEvents(
// MDN claims <details> should not receive ToggleEvent contradicting the spec: https://html.spec.whatwg.org/multipage/indices.html#event-toggle
SyntheticEventCtor = SyntheticToggleEvent;
break;
case 'command':
SyntheticEventCtor = SyntheticCommandEvent;
break;
default:
// Unknown event. This is used by createEventHandle.
break;
Expand Down
2 changes: 2 additions & 0 deletions packages/react-dom-bindings/src/shared/possibleStandardNames.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ const possibleStandardNames = {
classname: 'className',
cols: 'cols',
colspan: 'colSpan',
command: 'command',
commandfor: 'commandFor',
content: 'content',
contenteditable: 'contentEditable',
contextmenu: 'contextMenu',
Expand Down
28 changes: 28 additions & 0 deletions packages/react-dom/src/__tests__/DOMPropertyOperations-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1346,6 +1346,34 @@ describe('DOMPropertyOperations', () => {
);
});
});

it('warns when using commandFor={HTMLElement}', async () => {
const popoverTarget = document.createElement('div');
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);

await act(() => {
root.render(
<button key="one" commandFor={popoverTarget} command="toggle-popover">
Toggle popover
</button>,
);
});

assertConsoleErrorDev([
'The `commandFor` prop expects the ID of an Element as a string. Received HTMLDivElement {} instead.\n' +
' in button (at **)',
]);

// Dedupe warning
await act(() => {
root.render(
<button key="two" commandFor={popoverTarget} command="toggle-popover">
Toggle popover
</button>,
);
});
});
});

describe('deleteValueForProperty', () => {
Expand Down
16 changes: 16 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMEventPropagation-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1334,6 +1334,22 @@ describe('ReactDOMEventListener', () => {
});
});

it('onCommand', async () => {
await testEmulatedBubblingEvent({
type: 'div',
reactEvent: 'onCommand',
reactEventType: 'command',
nativeEvent: 'command',
dispatch(node) {
const e = new Event('command', {
bubbles: false,
cancelable: true,
});
node.dispatchEvent(e);
},
});
});

it('onVolumeChange', async () => {
await testEmulatedBubblingEvent({
type: 'video',
Expand Down
Loading