Skip to content

fix: add dev warning when dialog has no accessible title#9819

Open
mvanhorn wants to merge 2 commits intoadobe:mainfrom
mvanhorn:fix/dialog-title-dev-warning
Open

fix: add dev warning when dialog has no accessible title#9819
mvanhorn wants to merge 2 commits intoadobe:mainfrom
mvanhorn:fix/dialog-title-dev-warning

Conversation

@mvanhorn
Copy link

Summary

  • Adds a development-mode warning when a dialog is rendered without an accessible title (no aria-label, aria-labelledby, or visible title element with titleProps)
  • Warns once per dialog instance to avoid console spam on re-renders
  • Warning is gated behind process.env.NODE_ENV !== 'production' and tree-shaken in production builds

Closes #5402

Context

When useDialog and useOverlayTriggerState are used in the same component, useSlotId may return undefined because the title element is not in the DOM when the slot query runs. This silently produces an inaccessible dialog with no aria-labelledby. The new warning catches this common mistake and tells the developer exactly how to fix it.

Test plan

  • New test: warning fires when dialog has no accessible title
  • New test: no warning when aria-label is provided
  • New test: no warning when aria-labelledby is provided
  • New test: no warning when a title element is rendered with titleProps
  • Existing tests updated to provide aria-label (they were previously untitled)
  • yarn jest packages/react-aria/test/dialog/useDialog.test.js - 8/8 passing

This contribution was developed with AI assistance (Claude Code).

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>
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.
Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

thanks for the pr

const WARNING_PATTERNS_WE_SHOULD_FIX_BUT_ALLOW = [
'Browserslist: caniuse-lite is outdated'
'Browserslist: caniuse-lite is outdated',
'A dialog must have a visible title for accessibility'
Copy link
Member

Choose a reason for hiding this comment

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

we should not have this turned on. do you need it? I looks like you're already catching the warning with a jest spy/mock

let hasAriaLabelledby = el.hasAttribute('aria-labelledby');
if (!hasAriaLabel && !hasAriaLabelledby) {
console.warn(
'A dialog must have a visible title for accessibility. ' +
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'A dialog must have a visible title for accessibility. ' +
'A dialog must have a title for accessibility. ' +

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No title id is returned when useDialog and useOverlayTriggerState are used in the same component

2 participants