Skip to content

Commit ac718c6

Browse files
mvanhornclaudesnowystinger
authored
fix: add dev warning when dialog has no accessible title (adobe#9819)
* fix: add dev warning when dialog has no accessible title When a dialog is rendered without an aria-label, aria-labelledby, or a visible title element, emit a console.warn in development mode to help developers catch this common accessibility mistake early. The warning fires once per dialog instance and is tree-shaken in production builds. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(dialog): resolve test failures in dialog title dev warning Check the DOM element directly for aria-label/aria-labelledby attributes instead of relying on hook props, since wrapper components (e.g. RAC Dialog) may add aria-labelledby from context after useDialog runs. Add the warning pattern to the allowed warnings list in setupTests.js so existing tests that render dialogs without accessible labels are not broken. * fix: address review feedback - remove setupTests allowlist entry, drop 'visible' from warning Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add aria-label to test dialogs that lack accessible titles Adds aria-label="Test dialog" to Dialog test renders that don't have a heading or aria-labelledby, preventing the new dev warning from failing tests via failTestOnConsoleWarn(). 9 test files fixed. Remaining failures (ContextualHelp, DatePicker, DateRangePicker, Form, ListView) have dialogs rendered indirectly through components — those need source-level changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: check props and titleId in addition to DOM attributes for dialog warning The useEffect-based DOM check can race with useSlotId registration, causing false positives in components where a heading exists but hasn't registered yet. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add aria-label to ContextualHelp dialog and revert ineffective titleId check ContextualHelp renders <Header> (not <Heading>), so the Dialog's useSlotId resolves to undefined and the dialog has no accessible title. Pass the variant label ("Help"/"Information") as aria-label. Reverts the !!titleId check in useDialog which didn't work because useSlotId resolves to undefined via useLayoutEffect before the warning's useEffect runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix contextual help usage --------- Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Robert Snow <rsnow@adobe.com> Co-authored-by: Robert Snow <snowystinger@gmail.com>
1 parent 485c2da commit ac718c6

File tree

14 files changed

+119
-57
lines changed

14 files changed

+119
-57
lines changed

‎packages/@adobe/react-spectrum/test/actiongroup/ActionGroup.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ describe('ActionGroup', function () {
575575
<ActionGroup>
576576
<DialogTrigger>
577577
<Item>Hi</Item>
578-
<Dialog>
578+
<Dialog aria-label="Test dialog">
579579
I'm a dialog
580580
</Dialog>
581581
</DialogTrigger>

‎packages/@adobe/react-spectrum/test/contextualhelp/ContextualHelp.test.js‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {act, pointerMap, render, simulateDesktop} from '@react-spectrum/test-uti
1414
import {Content} from '../../src/view/Content';
1515
import {ContextualHelp} from '../../src/contextualhelp/ContextualHelp';
1616
import {Footer} from '../../src/view/Footer';
17-
import {Header} from '../../src/view/Header';
17+
import {Heading} from '../../src/text/Heading';
1818
import {Link} from '../../src/link/Link';
1919
import {Provider} from '../../src/provider/Provider';
2020
import React from 'react';
@@ -39,7 +39,7 @@ describe('ContextualHelp', function () {
3939
let {getByRole, queryByRole} = render(
4040
<Provider theme={theme}>
4141
<ContextualHelp>
42-
<Header>Test title</Header>
42+
<Heading>Test title</Heading>
4343
</ContextualHelp>
4444
</Provider>
4545
);
@@ -56,7 +56,7 @@ describe('ContextualHelp', function () {
5656
let {getByRole, queryByRole, getByTestId, getByText} = render(
5757
<Provider theme={theme}>
5858
<ContextualHelp>
59-
<Header>Test title</Header>
59+
<Heading>Test title</Heading>
6060
</ContextualHelp>
6161
</Provider>
6262
);
@@ -84,7 +84,7 @@ describe('ContextualHelp', function () {
8484
let {getByRole, getByText} = render(
8585
<Provider theme={theme}>
8686
<ContextualHelp>
87-
<Header>Test title</Header>
87+
<Heading>Test title</Heading>
8888
<Content>Help content</Content>
8989
</ContextualHelp>
9090
</Provider>
@@ -109,7 +109,7 @@ describe('ContextualHelp', function () {
109109
let {getByRole, getByText} = render(
110110
<Provider theme={theme}>
111111
<ContextualHelp>
112-
<Header>Test title</Header>
112+
<Heading>Test title</Heading>
113113
<Content>Help content</Content>
114114
<Footer>
115115
<Link>Test link</Link>
@@ -137,7 +137,7 @@ describe('ContextualHelp', function () {
137137
let {getByRole} = render(
138138
<Provider theme={theme}>
139139
<ContextualHelp>
140-
<Header>Test title</Header>
140+
<Heading>Test title</Heading>
141141
<Content>Help content</Content>
142142
</ContextualHelp>
143143
</Provider>
@@ -151,7 +151,7 @@ describe('ContextualHelp', function () {
151151
let {getByRole} = render(
152152
<Provider theme={theme}>
153153
<ContextualHelp variant="info">
154-
<Header>Test title</Header>
154+
<Heading>Test title</Heading>
155155
<Content>Help content</Content>
156156
</ContextualHelp>
157157
</Provider>
@@ -165,7 +165,7 @@ describe('ContextualHelp', function () {
165165
let {getByRole} = render(
166166
<Provider theme={theme}>
167167
<ContextualHelp aria-label="test">
168-
<Header>Test title</Header>
168+
<Heading>Test title</Heading>
169169
<Content>Help content</Content>
170170
</ContextualHelp>
171171
</Provider>
@@ -179,7 +179,7 @@ describe('ContextualHelp', function () {
179179
let {getByRole} = render(
180180
<Provider theme={theme}>
181181
<ContextualHelp aria-labelledby="test">
182-
<Header>Test title</Header>
182+
<Heading>Test title</Heading>
183183
<Content>Help content</Content>
184184
</ContextualHelp>
185185
</Provider>

‎packages/@adobe/react-spectrum/test/dialog/Dialog.ssr.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ describe('Dialog SSR', function () {
1717
await testSSR(__filename, `
1818
import {Dialog} from '../../exports/index.ts';
1919
20-
<Dialog>
20+
<Dialog aria-label="The label">
2121
contents
2222
</Dialog>
2323
`);

‎packages/@adobe/react-spectrum/test/dialog/Dialog.test.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {render} from '@react-spectrum/test-utils-internal';
2121
describe('Dialog', function () {
2222
it('does not auto focus anything inside', function () {
2323
let {getByRole} = render(
24-
<Dialog>
24+
<Dialog aria-label="Test dialog">
2525
<input data-testid="input1" />
2626
<input data-testid="input2" />
2727
</Dialog>
@@ -35,7 +35,7 @@ describe('Dialog', function () {
3535

3636
it('auto focuses the dialog itself if there is no focusable child', function () {
3737
let {getByRole} = render(
38-
<Dialog>
38+
<Dialog aria-label="Test dialog">
3939
contents
4040
</Dialog>
4141
);
@@ -46,7 +46,7 @@ describe('Dialog', function () {
4646

4747
it('autofocuses any element that has autofocus inside', function () {
4848
let {getByTestId} = render(
49-
<Dialog>
49+
<Dialog aria-label="Test dialog">
5050
<input data-testid="input1" />
5151
<input data-testid="input2" autoFocus />
5252
</Dialog>

‎packages/@adobe/react-spectrum/test/dialog/DialogTrigger.test.js‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ describe('DialogTrigger', function () {
6868
<Provider theme={theme}>
6969
<DialogTrigger>
7070
<ActionButton>Trigger</ActionButton>
71-
<Dialog>contents</Dialog>
71+
<Dialog aria-label="Test dialog">contents</Dialog>
7272
</DialogTrigger>
7373
</Provider>
7474
);
@@ -90,7 +90,7 @@ describe('DialogTrigger', function () {
9090
<Provider theme={theme}>
9191
<DialogTrigger type="tray">
9292
<ActionButton>Trigger</ActionButton>
93-
<Dialog>contents</Dialog>
93+
<Dialog aria-label="Test dialog">contents</Dialog>
9494
</DialogTrigger>
9595
</Provider>
9696
);
@@ -116,7 +116,7 @@ describe('DialogTrigger', function () {
116116
<Provider theme={theme}>
117117
<DialogTrigger type="popover">
118118
<ActionButton>Trigger</ActionButton>
119-
<Dialog>contents</Dialog>
119+
<Dialog aria-label="Test dialog">contents</Dialog>
120120
</DialogTrigger>
121121
</Provider>
122122
);
@@ -140,7 +140,7 @@ describe('DialogTrigger', function () {
140140
<DialogTrigger type="popover">
141141
<ActionButton>Trigger</ActionButton>
142142
{(close) => (
143-
<Dialog>
143+
<Dialog aria-label="Test dialog">
144144
contents
145145
<ButtonGroup>
146146
<Button variant="secondary" onPress={close}>
@@ -180,7 +180,7 @@ describe('DialogTrigger', function () {
180180
<Provider theme={theme}>
181181
<DialogTrigger type="popover">
182182
<ActionButton>Trigger</ActionButton>
183-
<Dialog>contents</Dialog>
183+
<Dialog aria-label="Test dialog">contents</Dialog>
184184
</DialogTrigger>
185185
</Provider>
186186
);
@@ -207,7 +207,7 @@ describe('DialogTrigger', function () {
207207
<Provider theme={theme}>
208208
<DialogTrigger type="popover" mobileType="tray">
209209
<ActionButton>Trigger</ActionButton>
210-
<Dialog>contents</Dialog>
210+
<Dialog aria-label="Test dialog">contents</Dialog>
211211
</DialogTrigger>
212212
</Provider>
213213
);
@@ -233,7 +233,7 @@ describe('DialogTrigger', function () {
233233
<Provider theme={theme}>
234234
<DialogTrigger type={type}>
235235
<ActionButton>Trigger</ActionButton>
236-
<Dialog>
236+
<Dialog aria-label="Test dialog">
237237
<input data-testid="input1" />
238238
<input data-testid="input2" />
239239
</Dialog>
@@ -268,7 +268,7 @@ describe('DialogTrigger', function () {
268268
<Provider theme={theme}>
269269
<DialogTrigger>
270270
<ActionButton>Trigger</ActionButton>
271-
<Dialog>contents</Dialog>
271+
<Dialog aria-label="Test dialog">contents</Dialog>
272272
</DialogTrigger>
273273
</Provider>
274274
);
@@ -291,7 +291,7 @@ describe('DialogTrigger', function () {
291291
<Provider theme={theme}>
292292
<DialogTrigger type="popover">
293293
<ActionButton>Trigger</ActionButton>
294-
<Dialog>contents</Dialog>
294+
<Dialog aria-label="Test dialog">contents</Dialog>
295295
</DialogTrigger>
296296
</Provider>
297297
);
@@ -336,7 +336,7 @@ describe('DialogTrigger', function () {
336336
<Provider theme={theme}>
337337
<DialogTrigger onOpenChange={onOpenChange} type="popover">
338338
<ActionButton>Trigger</ActionButton>
339-
<Dialog>contents</Dialog>
339+
<Dialog aria-label="Test dialog">contents</Dialog>
340340
</DialogTrigger>
341341
</Provider>
342342
);
@@ -382,7 +382,7 @@ describe('DialogTrigger', function () {
382382
<Provider theme={theme} ref={rootProviderRef}>
383383
<DialogTrigger>
384384
<ActionButton>Trigger</ActionButton>
385-
<Dialog>contents</Dialog>
385+
<Dialog aria-label="Test dialog">contents</Dialog>
386386
</DialogTrigger>
387387
</Provider>
388388
);
@@ -422,7 +422,7 @@ describe('DialogTrigger', function () {
422422
<Provider theme={theme}>
423423
<DialogTrigger isOpen={isOpen} onOpenChange={onOpenChange}>
424424
<ActionButton>Trigger</ActionButton>
425-
<Dialog>contents</Dialog>
425+
<Dialog aria-label="Test dialog">contents</Dialog>
426426
</DialogTrigger>
427427
</Provider>
428428
);
@@ -480,7 +480,7 @@ describe('DialogTrigger', function () {
480480
<Provider theme={theme}>
481481
<DialogTrigger defaultOpen={defaultOpen} onOpenChange={onOpenChange}>
482482
<ActionButton>Trigger</ActionButton>
483-
<Dialog>contents</Dialog>
483+
<Dialog aria-label="Test dialog">contents</Dialog>
484484
</DialogTrigger>
485485
</Provider>
486486
);
@@ -518,7 +518,7 @@ describe('DialogTrigger', function () {
518518
<Provider theme={theme}>
519519
<DialogTrigger defaultOpen={defaultOpen} onOpenChange={onOpenChange}>
520520
<ActionButton>Trigger</ActionButton>
521-
{(close) => <Dialog>contents<Button variant="primary" data-testid="closebtn" onPress={close}>Close</Button></Dialog>}
521+
{(close) => <Dialog aria-label="Test dialog">contents<Button variant="primary" data-testid="closebtn" onPress={close}>Close</Button></Dialog>}
522522
</DialogTrigger>
523523
</Provider>
524524
);
@@ -557,7 +557,7 @@ describe('DialogTrigger', function () {
557557
<Provider theme={theme}>
558558
<DialogTrigger isDismissable defaultOpen={defaultOpen} onOpenChange={onOpenChange}>
559559
<ActionButton>Trigger</ActionButton>
560-
<Dialog>contents</Dialog>
560+
<Dialog aria-label="Test dialog">contents</Dialog>
561561
</DialogTrigger>
562562
</Provider>
563563
);
@@ -596,7 +596,7 @@ describe('DialogTrigger', function () {
596596
<Provider theme={theme}>
597597
<DialogTrigger type="modal" isDismissable defaultOpen={defaultOpen} onOpenChange={onOpenChange}>
598598
<ActionButton>Trigger</ActionButton>
599-
<Dialog>contents</Dialog>
599+
<Dialog aria-label="Test dialog">contents</Dialog>
600600
</DialogTrigger>
601601
</Provider>
602602
);
@@ -634,7 +634,7 @@ describe('DialogTrigger', function () {
634634
<Provider theme={theme}>
635635
<DialogTrigger type="modal" defaultOpen={defaultOpen} onOpenChange={onOpenChange}>
636636
<ActionButton>Trigger</ActionButton>
637-
<Dialog>contents</Dialog>
637+
<Dialog aria-label="Test dialog">contents</Dialog>
638638
</DialogTrigger>
639639
</Provider>
640640
);
@@ -667,7 +667,7 @@ describe('DialogTrigger', function () {
667667
<Provider theme={theme}>
668668
<DialogTrigger type="popover" mobileType="modal" defaultOpen={defaultOpen} onOpenChange={onOpenChange}>
669669
<ActionButton>Trigger</ActionButton>
670-
<Dialog>contents</Dialog>
670+
<Dialog aria-label="Test dialog">contents</Dialog>
671671
</DialogTrigger>
672672
</Provider>
673673
);
@@ -705,7 +705,7 @@ describe('DialogTrigger', function () {
705705
<Provider theme={theme}>
706706
<DialogTrigger type="popover" defaultOpen={defaultOpen} onOpenChange={onOpenChange} isDismissable={false}>
707707
<ActionButton>Trigger</ActionButton>
708-
<Dialog>contents</Dialog>
708+
<Dialog aria-label="Test dialog">contents</Dialog>
709709
</DialogTrigger>
710710
</Provider>
711711
);
@@ -742,7 +742,7 @@ describe('DialogTrigger', function () {
742742
<Provider theme={theme}>
743743
<DialogTrigger isKeyboardDismissDisabled>
744744
<ActionButton>Trigger</ActionButton>
745-
{close => <Dialog><ActionButton onPress={close}>Close</ActionButton></Dialog>}
745+
{close => <Dialog aria-label="Test dialog"><ActionButton onPress={close}>Close</ActionButton></Dialog>}
746746
</DialogTrigger>
747747
</Provider>
748748
);
@@ -794,7 +794,7 @@ describe('DialogTrigger', function () {
794794
<Menu>
795795
<DialogTrigger isKeyboardDismissDisabled>
796796
<Item>Open menu</Item>
797-
<Dialog>Content body</Dialog>
797+
<Dialog aria-label="Test dialog">Content body</Dialog>
798798
</DialogTrigger>
799799
</Menu>
800800
</MenuTrigger>
@@ -830,12 +830,12 @@ describe('DialogTrigger', function () {
830830
<TextField id="document-input" aria-label="document input" />
831831
<DialogTrigger>
832832
<ActionButton id="outer-trigger">Trigger</ActionButton>
833-
<Dialog id="outer-dialog">
833+
<Dialog aria-label="Test dialog" id="outer-dialog">
834834
<Content>
835835
<TextField id="outer-input" aria-label="outer input" autoFocus />
836836
<DialogTrigger>
837837
<ActionButton id="inner-trigger">Trigger</ActionButton>
838-
<Dialog id="inner-dialog">
838+
<Dialog aria-label="Test dialog" id="inner-dialog">
839839
<Content>
840840
<TextField id="inner-input" aria-label="outer input" autoFocus />
841841
</Content>
@@ -900,12 +900,12 @@ describe('DialogTrigger', function () {
900900
<TextField id="document-input" aria-label="document input" />
901901
<DialogTrigger type="popover">
902902
<ActionButton id="outer-trigger">Trigger1</ActionButton>
903-
<Dialog id="outer-dialog">
903+
<Dialog aria-label="Test dialog" id="outer-dialog">
904904
<Content>
905905
<TextField id="outer-input" aria-label="outer input" />
906906
<DialogTrigger type="popover">
907907
<ActionButton id="inner-trigger">Trigger2</ActionButton>
908-
<Dialog id="inner-dialog">
908+
<Dialog aria-label="Test dialog" id="inner-dialog">
909909
<Content>
910910
<TextField id="inner-input" label="inner input" />
911911
</Content>
@@ -958,7 +958,7 @@ describe('DialogTrigger', function () {
958958
<UNSAFE_PortalProvider getContainer={() => container.current}>
959959
<DialogTrigger type={props.type}>
960960
<ActionButton>Trigger</ActionButton>
961-
<Dialog>contents</Dialog>
961+
<Dialog aria-label="Test dialog">contents</Dialog>
962962
</DialogTrigger>
963963
</UNSAFE_PortalProvider>
964964
</Provider>

‎packages/@adobe/react-spectrum/test/form/Form.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {Button} from '../../src/button/Button';
1515
import {Content} from '../../src/view/Content';
1616
import {ContextualHelp} from '../../src/contextualhelp/ContextualHelp';
1717
import {Form} from '../../src/form/Form';
18-
import {Header} from '../../src/view/Header';
18+
import {Heading} from '../../src/text/Heading';
1919
import {Item} from 'react-stately/Item';
2020
import {Picker} from '../../src/picker/Picker';
2121
import {pointerMap, render, simulateMobile} from '@react-spectrum/test-utils-internal';
@@ -212,7 +212,7 @@ describe('Form', function () {
212212
label="Test Picker"
213213
contextualHelp={(
214214
<ContextualHelp>
215-
<Header>What is it good for?</Header>
215+
<Heading>What is it good for?</Heading>
216216
<Content>Absolutely nothing.</Content>
217217
</ContextualHelp>
218218
)}>

‎packages/@adobe/react-spectrum/test/menu/Menu.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ describe('Menu', function () {
584584
<Section title="Test">
585585
<DialogTrigger>
586586
<Item>Hi</Item>
587-
<Dialog>
587+
<Dialog aria-label="Test dialog">
588588
I'm a dialog
589589
</Dialog>
590590
</DialogTrigger>

‎packages/@adobe/react-spectrum/test/overlays/Popover.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ function PopoverWithDialog({children}) {
2424
let state = useOverlayTriggerState({isOpen: true});
2525
return (
2626
<Popover triggerRef={ref} state={state}>
27-
<Dialog>{children}</Dialog>
27+
<Dialog aria-label="Test dialog">{children}</Dialog>
2828
</Popover>
2929
);
3030
}

‎packages/@adobe/react-spectrum/test/overlays/Tray.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ describe('Tray', function () {
9999
let {getByRole} = render(
100100
<Provider theme={theme}>
101101
<TestTray isOpen onOpenChange={onOpenChange} shouldCloseOnBlur>
102-
<Dialog>contents</Dialog>
102+
<Dialog aria-label="Test dialog">contents</Dialog>
103103
</TestTray>
104104
</Provider>
105105
);

0 commit comments

Comments
 (0)