diff --git a/.changeset/fix-dialog-shadow-dom-accessibility.md b/.changeset/fix-dialog-shadow-dom-accessibility.md new file mode 100644 index 000000000..01a97a8ca --- /dev/null +++ b/.changeset/fix-dialog-shadow-dom-accessibility.md @@ -0,0 +1,5 @@ +--- +'@radix-ui/react-dialog': patch +--- + +Fix accessibility check for `DialogTitle` and `DialogDescription` when rendered inside Shadow DOM. Previously used `document.getElementById` which fails in Shadow DOM contexts; now uses `element.getRootNode()` to search within the correct document scope. diff --git a/packages/react/dialog/src/dialog.test.tsx b/packages/react/dialog/src/dialog.test.tsx index 7e2fe0b9c..40112d459 100644 --- a/packages/react/dialog/src/dialog.test.tsx +++ b/packages/react/dialog/src/dialog.test.tsx @@ -1,7 +1,7 @@ import React from 'react'; import { axe } from 'vitest-axe'; import type { RenderResult } from '@testing-library/react'; -import { render, fireEvent, cleanup } from '@testing-library/react'; +import { render, fireEvent, cleanup, act } from '@testing-library/react'; import * as Dialog from './dialog'; import type { Mock, MockInstance } from 'vitest'; import { describe, it, afterEach, beforeEach, vi, expect } from 'vitest'; @@ -139,3 +139,47 @@ describe('given a default Dialog', () => { }); }); }); + +describe('given a Dialog rendered inside a Shadow DOM', () => { + let shadowHost: HTMLElement; + let shadowRoot: ShadowRoot; + let consoleErrorMock: ReturnType; + let consoleErrorMockFunction: ReturnType; + + beforeEach(() => { + consoleErrorMockFunction = vi.fn(); + consoleErrorMock = vi.spyOn(console, 'error').mockImplementation(consoleErrorMockFunction); + + // Create a shadow host and attach a shadow root + shadowHost = document.createElement('div'); + document.body.appendChild(shadowHost); + shadowRoot = shadowHost.attachShadow({ mode: 'open' }); + }); + + afterEach(() => { + consoleErrorMock.mockRestore(); + consoleErrorMockFunction.mockClear(); + document.body.removeChild(shadowHost); + }); + + it('should not fire accessibility warning when DialogTitle is present inside Shadow DOM', async () => { + const container = document.createElement('div'); + shadowRoot.appendChild(container); + + render( + + + Shadow Dialog Title + Shadow Dialog Description + Close + + , + { container } + ); + + // Wait for effects to run + await act(async () => {}); + + expect(consoleErrorMockFunction).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/react/dialog/src/dialog.tsx b/packages/react/dialog/src/dialog.tsx index dbbaa4126..6b248a743 100644 --- a/packages/react/dialog/src/dialog.tsx +++ b/packages/react/dialog/src/dialog.tsx @@ -196,10 +196,52 @@ interface DialogOverlayImplProps extends PrimitiveDivProps {} const Slot = createSlot('DialogOverlay.RemoveScroll'); +/** + * The attribute set on `document.body` by `react-remove-scroll-bar` to lock scroll. + * We reference it here to ensure synchronous cleanup on forced unmount. + */ +const SCROLL_LOCK_ATTRIBUTE = 'data-scroll-locked'; + const DialogOverlayImpl = React.forwardRef( (props: ScopedProps, forwardedRef) => { const { __scopeDialog, ...overlayProps } = props; const context = useDialogContext(OVERLAY_NAME, __scopeDialog); + + /** + * Ensure the scroll lock is released synchronously when the overlay is forcefully + * unmounted (e.g. when a router Link inside the Dialog triggers navigation while + * the Dialog is still open). + * + * `react-remove-scroll` manages `data-scroll-locked` via `useEffect`, which is + * asynchronous. In certain SPA router scenarios the old route's async effect + * cleanups may be deferred or skipped, leaving `overflow: hidden` on the body + * and preventing scrolling on the destination or return page. + * + * By using `useLayoutEffect` (synchronous, fires during the commit phase) we + * guarantee the attribute is decremented and removed as soon as the overlay + * leaves the React tree, regardless of routing strategy or animation timing. + * + * Counter coordination: `react-remove-scroll-bar` stores the number of active + * locks as an integer in the attribute value. We read and write that same + * counter so nested / stacked dialogs continue to work correctly. Because our + * `useLayoutEffect` cleanup runs *before* `react-remove-scroll`'s `useEffect` + * cleanup, the subsequent async decrement will read 0 and call + * `removeAttribute`, which is a safe no-op on an already-absent attribute. + */ + React.useLayoutEffect(() => { + return () => { + const raw = document.body.getAttribute(SCROLL_LOCK_ATTRIBUTE); + if (raw === null) return; // already cleaned up + const current = parseInt(raw, 10); + const next = isFinite(current) ? current - 1 : 0; + if (next <= 0) { + document.body.removeAttribute(SCROLL_LOCK_ATTRIBUTE); + } else { + document.body.setAttribute(SCROLL_LOCK_ATTRIBUTE, String(next)); + } + }; + }, []); + return ( // Make sure `Content` is scrollable even when it doesn't live inside `RemoveScroll` // ie. when `Overlay` and `Content` are siblings @@ -414,7 +456,7 @@ const DialogContentImpl = React.forwardRef {process.env.NODE_ENV !== 'production' && ( <> - + )} @@ -503,9 +545,12 @@ const [WarningProvider, useWarningContext] = createContext(TITLE_WARNING_NAME, { docsSlug: 'dialog', }); -type TitleWarningProps = { titleId?: string }; +type TitleWarningProps = { + contentRef: React.RefObject; + titleId?: string; +}; -const TitleWarning: React.FC = ({ titleId }) => { +const TitleWarning: React.FC = ({ contentRef, titleId }) => { const titleWarningContext = useWarningContext(TITLE_WARNING_NAME); const MESSAGE = `\`${titleWarningContext.contentName}\` requires a \`${titleWarningContext.titleName}\` for the component to be accessible for screen reader users. @@ -516,10 +561,19 @@ For more information, see https://radix-ui.com/primitives/docs/components/${titl React.useEffect(() => { if (titleId) { - const hasTitle = document.getElementById(titleId); + // Use getRootNode() to support Shadow DOM contexts where document.getElementById + // would fail to find elements rendered inside a shadow root. + // Guard: getRootNode() may return a Node without getElementById (e.g. detached + // subtree or DocumentFragment), so fall back to ownerDocument ?? document. + const rootNode = contentRef.current?.getRootNode() ?? document; + const searchRoot = + rootNode instanceof Document || rootNode instanceof ShadowRoot + ? rootNode + : (contentRef.current?.ownerDocument ?? document); + const hasTitle = searchRoot.getElementById(titleId); if (!hasTitle) console.error(MESSAGE); } - }, [MESSAGE, titleId]); + }, [MESSAGE, contentRef, titleId]); return null; }; @@ -539,7 +593,16 @@ const DescriptionWarning: React.FC = ({ contentRef, des const describedById = contentRef.current?.getAttribute('aria-describedby'); // if we have an id and the user hasn't set aria-describedby={undefined} if (descriptionId && describedById) { - const hasDescription = document.getElementById(descriptionId); + // Use getRootNode() to support Shadow DOM contexts where document.getElementById + // would fail to find elements rendered inside a shadow root. + // Guard: getRootNode() may return a Node without getElementById (e.g. detached + // subtree or DocumentFragment), so fall back to ownerDocument ?? document. + const rootNode = contentRef.current?.getRootNode() ?? document; + const searchRoot = + rootNode instanceof Document || rootNode instanceof ShadowRoot + ? rootNode + : (contentRef.current?.ownerDocument ?? document); + const hasDescription = searchRoot.getElementById(descriptionId); if (!hasDescription) console.warn(MESSAGE); } }, [MESSAGE, contentRef, descriptionId]);