Skip to content
Open
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
5 changes: 5 additions & 0 deletions .changeset/fix-dialog-shadow-dom-accessibility.md
Original file line number Diff line number Diff line change
@@ -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.
46 changes: 45 additions & 1 deletion packages/react/dialog/src/dialog.test.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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<typeof vi.spyOn>;
let consoleErrorMockFunction: ReturnType<typeof vi.fn>;

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(
<Dialog.Root open>
<Dialog.Content>
<Dialog.Title>Shadow Dialog Title</Dialog.Title>
<Dialog.Description>Shadow Dialog Description</Dialog.Description>
<Dialog.Close>Close</Dialog.Close>
</Dialog.Content>
</Dialog.Root>,
{ container }
);

// Wait for effects to run
await act(async () => {});

expect(consoleErrorMockFunction).not.toHaveBeenCalled();
});
});
75 changes: 69 additions & 6 deletions packages/react/dialog/src/dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<DialogOverlayImplElement, DialogOverlayImplProps>(
(props: ScopedProps<DialogOverlayImplProps>, 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
Expand Down Expand Up @@ -414,7 +456,7 @@ const DialogContentImpl = React.forwardRef<DialogContentImplElement, DialogConte
</FocusScope>
{process.env.NODE_ENV !== 'production' && (
<>
<TitleWarning titleId={context.titleId} />
<TitleWarning contentRef={contentRef} titleId={context.titleId} />
<DescriptionWarning contentRef={contentRef} descriptionId={context.descriptionId} />
</>
)}
Expand Down Expand Up @@ -503,9 +545,12 @@ const [WarningProvider, useWarningContext] = createContext(TITLE_WARNING_NAME, {
docsSlug: 'dialog',
});

type TitleWarningProps = { titleId?: string };
type TitleWarningProps = {
contentRef: React.RefObject<DialogContentElement | null>;
titleId?: string;
};

const TitleWarning: React.FC<TitleWarningProps> = ({ titleId }) => {
const TitleWarning: React.FC<TitleWarningProps> = ({ 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.
Expand All @@ -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);
Comment on lines +564 to 574
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

getRootNode() can return a Node that is neither Document nor ShadowRoot (e.g. a detached subtree root element or a DocumentFragment). In those cases, the cast to Document | ShadowRoot can cause a runtime TypeError when calling getElementById. Consider guarding with a runtime check (e.g. verify rootNode has getElementById) and otherwise fall back to contentRef.current?.ownerDocument ?? document.

Copilot uses AI. Check for mistakes.
}
}, [MESSAGE, titleId]);
}, [MESSAGE, contentRef, titleId]);

return null;
};
Expand All @@ -539,7 +593,16 @@ const DescriptionWarning: React.FC<DescriptionWarningProps> = ({ 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);
Comment on lines +596 to 606
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

Same concern as in TitleWarning: contentRef.current?.getRootNode() may return a Node without getElementById, which would throw at runtime. Add a runtime guard (or resolve via ownerDocument unless the root is a ShadowRoot) before calling getElementById.

Copilot uses AI. Check for mistakes.
}
}, [MESSAGE, contentRef, descriptionId]);
Expand Down