Skip to content

Commit ca443d3

Browse files
committed
Add warning when using popoverTarget={Element}
1 parent f74fb57 commit ca443d3

File tree

5 files changed

+47
-11
lines changed

5 files changed

+47
-11
lines changed

fixtures/attribute-behavior/AttributeTableSnapshot.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8462,25 +8462,25 @@
84628462
| `popover=(integer)`| (changed)| `"manual"` |
84638463
| `popover=(NaN)`| (changed, warning)| `"manual"` |
84648464
| `popover=(float)`| (changed)| `"manual"` |
8465-
| `popover=(true)`| (initial, warning)| `<null>` |
8466-
| `popover=(false)`| (initial, warning)| `<null>` |
8465+
| `popover=(true)`| (initial)| `<null>` |
8466+
| `popover=(false)`| (initial)| `<null>` |
84678467
| `popover=(string 'true')`| (changed)| `"manual"` |
84688468
| `popover=(string 'false')`| (changed)| `"manual"` |
84698469
| `popover=(string 'on')`| (changed)| `"manual"` |
84708470
| `popover=(string 'off')`| (changed)| `"manual"` |
8471-
| `popover=(symbol)`| (initial, warning)| `<null>` |
8472-
| `popover=(function)`| (initial, warning)| `<null>` |
8471+
| `popover=(symbol)`| (initial)| `<null>` |
8472+
| `popover=(function)`| (initial)| `<null>` |
84738473
| `popover=(null)`| (initial)| `<null>` |
84748474
| `popover=(undefined)`| (initial)| `<null>` |
84758475

84768476
## `popoverTarget` (on `<button>` inside `<div>`)
84778477
| Test Case | Flags | Result |
84788478
| --- | --- | --- |
8479-
| `popoverTarget=(string)`| (initial)| `<null>` |
8479+
| `popoverTarget=(string)`| (changed)| `<HTMLDivElement>` |
84808480
| `popoverTarget=(empty string)`| (initial)| `<null>` |
8481-
| `popoverTarget=(array with string)`| (initial)| `<null>` |
8482-
| `popoverTarget=(empty array)`| (initial)| `<null>` |
8483-
| `popoverTarget=(object)`| (initial)| `<null>` |
8481+
| `popoverTarget=(array with string)`| (changed, warning, ssr warning)| `<HTMLDivElement>` |
8482+
| `popoverTarget=(empty array)`| (initial, warning, ssr warning)| `<null>` |
8483+
| `popoverTarget=(object)`| (initial, warning, ssr warning)| `<null>` |
84848484
| `popoverTarget=(numeric string)`| (initial)| `<null>` |
84858485
| `popoverTarget=(-1)`| (initial)| `<null>` |
84868486
| `popoverTarget=(0)`| (initial)| `<null>` |
@@ -8493,8 +8493,8 @@
84938493
| `popoverTarget=(string 'false')`| (initial)| `<null>` |
84948494
| `popoverTarget=(string 'on')`| (initial)| `<null>` |
84958495
| `popoverTarget=(string 'off')`| (initial)| `<null>` |
8496-
| `popoverTarget=(symbol)`| (initial, warning)| `<null>` |
8497-
| `popoverTarget=(function)`| (initial, warning)| `<null>` |
8496+
| `popoverTarget=(symbol)`| (initial)| `<null>` |
8497+
| `popoverTarget=(function)`| (initial)| `<null>` |
84988498
| `popoverTarget=(null)`| (initial)| `<null>` |
84998499
| `popoverTarget=(undefined)`| (initial)| `<null>` |
85008500

fixtures/attribute-behavior/public/index.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
You need to enable JavaScript to run this app.
2727
</noscript>
2828
<div id="root"></div>
29+
<div id="popover-target" popover="auto"></div>
2930
<!--
3031
This HTML file is a template.
3132
If you open it directly in the browser, you will see an empty page.

fixtures/attribute-behavior/src/attributes.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1450,7 +1450,16 @@ const attributes = [
14501450
{name: 'popover', overrideStringValue: 'manual'},
14511451
{
14521452
name: 'popoverTarget',
1453-
read: element => element.popoverTargetElement,
1453+
read: element => {
1454+
document.body.appendChild(element);
1455+
try {
1456+
// trigger and target need to be connected for `popoverTargetElement` to read the actual value.
1457+
return element.popoverTargetElement;
1458+
} finally {
1459+
document.body.removeChild(element);
1460+
}
1461+
},
1462+
overrideStringValue: 'popover-target',
14541463
tagName: 'button',
14551464
},
14561465
{name: 'popoverTargetAction', overrideStringValue: 'show', tagName: 'button'},

packages/react-dom-bindings/src/client/ReactDOMComponent.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -866,6 +866,15 @@ function setProp(
866866
case 'innerText':
867867
case 'textContent':
868868
break;
869+
case 'popoverTarget':
870+
if (__DEV__) {
871+
if (value != null && typeof value === 'object') {
872+
console.error(
873+
'The `popoverTarget` prop expects the ID of an Element as a string. Received %s instead.',
874+
value,
875+
);
876+
}
877+
}
869878
// Fall through
870879
default: {
871880
if (

packages/react-dom/src/__tests__/DOMPropertyOperations-test.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
'use strict';
1111

12+
const {assertConsoleErrorDev} = require('internal-test-utils');
1213
// Set by `yarn test-fire`.
1314
const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');
1415

@@ -1317,6 +1318,22 @@ describe('DOMPropertyOperations', () => {
13171318
});
13181319
expect(customElement.foo).toBe(undefined);
13191320
});
1321+
1322+
it('warns when using popoverTarget={HTMLElement}', async () => {
1323+
const popoverTarget = document.createElement('div');
1324+
const container = document.createElement('div');
1325+
const root = ReactDOMClient.createRoot(container);
1326+
1327+
await act(() => {
1328+
root.render(
1329+
<button popoverTarget={popoverTarget}>Toggle popover</button>,
1330+
);
1331+
});
1332+
1333+
assertConsoleErrorDev([
1334+
'The `popoverTarget` prop expects the ID of an Element as a string. Received [object HTMLDivElement] instead.',
1335+
]);
1336+
});
13201337
});
13211338

13221339
describe('deleteValueForProperty', () => {

0 commit comments

Comments
 (0)