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

React DOM: Add support for Popover API #27981

Merged
merged 7 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
75 changes: 75 additions & 0 deletions fixtures/attribute-behavior/AttributeTableSnapshot.md
Original file line number Diff line number Diff line change
Expand Up @@ -8448,6 +8448,81 @@
| `pointsAtZ=(null)`| (initial)| `<number: 0>` |
| `pointsAtZ=(undefined)`| (initial)| `<number: 0>` |

## `popover` (on `<div>` inside `<div>`)
| Test Case | Flags | Result |
| --- | --- | --- |
| `popover=(string)`| (changed)| `"manual"` |
| `popover=(empty string)`| (changed)| `"auto"` |
| `popover=(array with string)`| (changed)| `"manual"` |
| `popover=(empty array)`| (changed)| `"auto"` |
| `popover=(object)`| (changed)| `"manual"` |
| `popover=(numeric string)`| (changed)| `"manual"` |
| `popover=(-1)`| (changed)| `"manual"` |
| `popover=(0)`| (changed)| `"manual"` |
| `popover=(integer)`| (changed)| `"manual"` |
| `popover=(NaN)`| (changed, warning)| `"manual"` |
| `popover=(float)`| (changed)| `"manual"` |
| `popover=(true)`| (initial, warning)| `<null>` |
| `popover=(false)`| (initial, warning)| `<null>` |
| `popover=(string 'true')`| (changed)| `"manual"` |
| `popover=(string 'false')`| (changed)| `"manual"` |
| `popover=(string 'on')`| (changed)| `"manual"` |
| `popover=(string 'off')`| (changed)| `"manual"` |
| `popover=(symbol)`| (initial, warning)| `<null>` |
| `popover=(function)`| (initial, warning)| `<null>` |
| `popover=(null)`| (initial)| `<null>` |
| `popover=(undefined)`| (initial)| `<null>` |

## `popoverTarget` (on `<button>` inside `<div>`)
| Test Case | Flags | Result |
| --- | --- | --- |
| `popoverTarget=(string)`| (changed)| `<HTMLDivElement>` |
| `popoverTarget=(empty string)`| (initial)| `<null>` |
| `popoverTarget=(array with string)`| (changed, warning, ssr warning)| `<HTMLDivElement>` |
| `popoverTarget=(empty array)`| (initial, warning, ssr warning)| `<null>` |
| `popoverTarget=(object)`| (initial, warning, ssr warning)| `<null>` |
| `popoverTarget=(numeric string)`| (initial)| `<null>` |
| `popoverTarget=(-1)`| (initial)| `<null>` |
| `popoverTarget=(0)`| (initial)| `<null>` |
| `popoverTarget=(integer)`| (initial)| `<null>` |
| `popoverTarget=(NaN)`| (initial, warning)| `<null>` |
| `popoverTarget=(float)`| (initial)| `<null>` |
| `popoverTarget=(true)`| (initial, warning)| `<null>` |
| `popoverTarget=(false)`| (initial, warning)| `<null>` |
| `popoverTarget=(string 'true')`| (initial)| `<null>` |
| `popoverTarget=(string 'false')`| (initial)| `<null>` |
| `popoverTarget=(string 'on')`| (initial)| `<null>` |
| `popoverTarget=(string 'off')`| (initial)| `<null>` |
| `popoverTarget=(symbol)`| (initial, warning)| `<null>` |
| `popoverTarget=(function)`| (initial, warning)| `<null>` |
| `popoverTarget=(null)`| (initial)| `<null>` |
| `popoverTarget=(undefined)`| (initial)| `<null>` |

## `popoverTargetAction` (on `<button>` inside `<div>`)
| Test Case | Flags | Result |
| --- | --- | --- |
| `popoverTargetAction=(string)`| (changed)| `"show"` |
| `popoverTargetAction=(empty string)`| (initial)| `"toggle"` |
| `popoverTargetAction=(array with string)`| (changed)| `"show"` |
| `popoverTargetAction=(empty array)`| (initial)| `"toggle"` |
| `popoverTargetAction=(object)`| (initial)| `"toggle"` |
| `popoverTargetAction=(numeric string)`| (initial)| `"toggle"` |
| `popoverTargetAction=(-1)`| (initial)| `"toggle"` |
| `popoverTargetAction=(0)`| (initial)| `"toggle"` |
| `popoverTargetAction=(integer)`| (initial)| `"toggle"` |
| `popoverTargetAction=(NaN)`| (initial, warning)| `"toggle"` |
| `popoverTargetAction=(float)`| (initial)| `"toggle"` |
| `popoverTargetAction=(true)`| (initial, warning)| `"toggle"` |
| `popoverTargetAction=(false)`| (initial, warning)| `"toggle"` |
| `popoverTargetAction=(string 'true')`| (initial)| `"toggle"` |
| `popoverTargetAction=(string 'false')`| (initial)| `"toggle"` |
| `popoverTargetAction=(string 'on')`| (initial)| `"toggle"` |
| `popoverTargetAction=(string 'off')`| (initial)| `"toggle"` |
| `popoverTargetAction=(symbol)`| (initial, warning)| `"toggle"` |
| `popoverTargetAction=(function)`| (initial, warning)| `"toggle"` |
| `popoverTargetAction=(null)`| (initial)| `"toggle"` |
| `popoverTargetAction=(undefined)`| (initial)| `"toggle"` |

## `poster` (on `<video>` inside `<div>`)
| Test Case | Flags | Result |
| --- | --- | --- |
Expand Down
1 change: 1 addition & 0 deletions fixtures/attribute-behavior/public/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
You need to enable JavaScript to run this app.
</noscript>
<div id="root"></div>
<div id="popover-target" popover="auto"></div>
<!--
This HTML file is a template.
If you open it directly in the browser, you will see an empty page.
Expand Down
16 changes: 16 additions & 0 deletions fixtures/attribute-behavior/src/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -1447,6 +1447,22 @@ const attributes = [
containerTagName: 'svg',
tagName: 'feSpotLight',
},
{name: 'popover', overrideStringValue: 'manual'},
{
name: 'popoverTarget',
read: element => {
document.body.appendChild(element);
try {
// trigger and target need to be connected for `popoverTargetElement` to read the actual value.
return element.popoverTargetElement;
} finally {
document.body.removeChild(element);
}
},
overrideStringValue: 'popover-target',
tagName: 'button',
},
{name: 'popoverTargetAction', overrideStringValue: 'show', tagName: 'button'},
{
name: 'poster',
tagName: 'video',
Expand Down
27 changes: 27 additions & 0 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ let didWarnFormActionName = false;
let didWarnFormActionTarget = false;
let didWarnFormActionMethod = false;
let didWarnForNewBooleanPropsWithEmptyValue: {[string]: boolean};
let didWarnPopoverTargetObject = false;
let canDiffStyleForHydrationWarning;
if (__DEV__) {
didWarnForNewBooleanPropsWithEmptyValue = {};
Expand Down Expand Up @@ -770,6 +771,11 @@ function setProp(
}
break;
}
case 'popover':
listenToNonDelegatedEvent('beforetoggle', domElement);
listenToNonDelegatedEvent('toggle', domElement);
setValueForAttribute(domElement, 'popover', value);
break;
case 'xlinkActuate':
setValueForNamespacedAttribute(
domElement,
Expand Down Expand Up @@ -861,6 +867,20 @@ function setProp(
case 'innerText':
case 'textContent':
break;
case 'popoverTarget':
if (__DEV__) {
if (
!didWarnPopoverTargetObject &&
value != null &&
typeof value === 'object'
) {
didWarnPopoverTargetObject = true;
console.error(
'The `popoverTarget` prop expects the ID of an Element as a string. Received %s instead.',
value,
);
}
}
// Fall through
default: {
if (
Expand Down Expand Up @@ -2953,6 +2973,13 @@ export function hydrateProperties(
}
}

if (props.popover != null) {
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the toggle event.
listenToNonDelegatedEvent('beforetoggle', domElement);
listenToNonDelegatedEvent('toggle', domElement);
}

if (props.onScroll != null) {
listenToNonDelegatedEvent('scroll', domElement);
}
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom-bindings/src/events/DOMEventNames.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export type DOMEventName =
// 'animationstart' |
| 'beforeblur' // Not a real event. This is used by event experiments.
| 'beforeinput'
| 'beforetoggle'
| 'blur'
| 'canplay'
| 'canplaythrough'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const topLevelEventsToReactNames: Map<DOMEventName, string | null> =
const simpleEventPluginEvents = [
'abort',
'auxClick',
'beforeToggle',
'cancel',
'canPlay',
'canPlayThrough',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export const mediaEventTypes: Array<DOMEventName> = [
// set them on the actual target element itself. This is primarily
// because these events do not consistently bubble in the DOM.
export const nonDelegatedEvents: Set<DOMEventName> = new Set([
'beforetoggle',
'cancel',
'close',
'invalid',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ export function getEventPriority(domEventName: DOMEventName): EventPriority {
case 'select':
case 'selectstart':
return DiscreteEventPriority;
case 'beforetoggle':
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
case 'drag':
case 'dragenter':
case 'dragexit':
Expand Down
8 changes: 8 additions & 0 deletions packages/react-dom-bindings/src/events/SyntheticEvent.js
Original file line number Diff line number Diff line change
Expand Up @@ -592,3 +592,11 @@ const WheelEventInterface = {
};
export const SyntheticWheelEvent: $FlowFixMe =
createSyntheticEvent(WheelEventInterface);

const ToggleEventInterface = {
...EventInterface,
newState: 0,
oldState: 0,
};
export const SyntheticToggleEvent: $FlowFixMe =
createSyntheticEvent(ToggleEventInterface);
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
SyntheticWheelEvent,
SyntheticClipboardEvent,
SyntheticPointerEvent,
SyntheticToggleEvent,
} from '../../events/SyntheticEvent';

import {
Expand Down Expand Up @@ -161,6 +162,11 @@ function extractEvents(
case 'pointerup':
SyntheticEventCtor = SyntheticPointerEvent;
break;
case 'toggle':
case 'beforetoggle':
// MDN claims <details> should not receive ToggleEvent contradicting the spec: https://html.spec.whatwg.org/multipage/indices.html#event-toggle
SyntheticEventCtor = SyntheticToggleEvent;
break;
default:
// Unknown event. This is used by createEventHandle.
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,9 @@ const possibleStandardNames = {
pointsatx: 'pointsAtX',
pointsaty: 'pointsAtY',
pointsatz: 'pointsAtZ',
popover: 'popover',
popovertarget: 'popoverTarget',
popovertargetaction: 'popoverTargetAction',
prefix: 'prefix',
preservealpha: 'preserveAlpha',
preserveaspectratio: 'preserveAspectRatio',
Expand Down
30 changes: 29 additions & 1 deletion packages/react-dom/src/__tests__/DOMPropertyOperations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ describe('DOMPropertyOperations', () => {
let React;
let ReactDOMClient;
let act;
let assertConsoleErrorDev;

beforeEach(() => {
jest.resetModules();
React = require('react');
ReactDOMClient = require('react-dom/client');
({act} = require('internal-test-utils'));
({act, assertConsoleErrorDev} = require('internal-test-utils'));
});

// Sets a value in a way that React doesn't see,
Expand Down Expand Up @@ -1317,6 +1318,33 @@ describe('DOMPropertyOperations', () => {
});
expect(customElement.foo).toBe(undefined);
});

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

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

assertConsoleErrorDev([
'The `popoverTarget` prop expects the ID of an Element as a string. Received [object HTMLDivElement] instead.',
]);

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

describe('deleteValueForProperty', () => {
Expand Down
46 changes: 41 additions & 5 deletions packages/react-dom/src/__tests__/ReactDOMEventPropagation-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1268,6 +1268,40 @@ describe('ReactDOMEventListener', () => {
});
});

it('onBeforeToggle Popover API', async () => {
await testEmulatedBubblingEvent({
type: 'div',
targetProps: {popover: 'any'},
reactEvent: 'onBeforeToggle',
reactEventType: 'beforetoggle',
nativeEvent: 'beforetoggle',
dispatch(node) {
const e = new Event('beforetoggle', {
bubbles: false,
cancelable: true,
});
node.dispatchEvent(e);
},
});
});

it('onToggle Popover API', async () => {
await testEmulatedBubblingEvent({
type: 'div',
targetProps: {popover: 'any'},
reactEvent: 'onToggle',
reactEventType: 'toggle',
nativeEvent: 'toggle',
dispatch(node) {
const e = new Event('toggle', {
bubbles: false,
cancelable: true,
});
node.dispatchEvent(e);
},
});
});

it('onVolumeChange', async () => {
await testEmulatedBubblingEvent({
type: 'video',
Expand Down Expand Up @@ -1969,6 +2003,7 @@ describe('ReactDOMEventListener', () => {
type={eventConfig.type}
targetRef={targetRef}
targetProps={{
...eventConfig.targetProps,
[eventConfig.reactEvent]: e => {
log.push('---- inner');
},
Expand Down Expand Up @@ -2135,11 +2170,10 @@ describe('ReactDOMEventListener', () => {
<Fixture
type={eventConfig.type}
targetRef={targetRef}
targetProps={
{
// No listener on the target itself.
}
}
targetProps={{
...eventConfig.targetProps,
// No listener on the target itself.
}}
parentProps={{
[eventConfig.reactEvent]: e => {
log.push('--- inner parent');
Expand Down Expand Up @@ -2368,6 +2402,7 @@ describe('ReactDOMEventListener', () => {
type={eventConfig.type}
targetRef={targetRef}
targetProps={{
...eventConfig.targetProps,
[eventConfig.reactEvent]: e => {
e.stopPropagation(); // <---------
log.push('---- inner');
Expand Down Expand Up @@ -2705,6 +2740,7 @@ describe('ReactDOMEventListener', () => {
}
}}
targetProps={{
...eventConfig.targetProps,
[eventConfig.reactEvent]: e => {
log.push('---- inner');
},
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/src/__tests__/ReactTestUtils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe('ReactTestUtils', () => {
"animationStart",
"auxClick",
"beforeInput",
"beforeToggle",
"blur",
"canPlay",
"canPlayThrough",
Expand Down
Loading