Skip to content
Closed
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/quiet-dialogs-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@youversion/platform-react-ui': patch
---

Restore the original Shadow DOM opener after a dialog closes and reopens during its exit animation, while ignoring disconnected or cross-tree focus targets.
12 changes: 6 additions & 6 deletions docs/adr/0007-prototype-shadow-dom-style-isolation.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,14 @@ dismissal and restore focus into the remaining parent overlay. Either
nested-overlay teardown order also works with the current architecture. Separate
component shadow roots own distinct portal containers, but do not isolate Radix
outside interaction: opening a peer popover dismisses the existing peer across
the same or separate roots. A verse action popover does not restore final focus
after its nested dialog and then the popover close, and rapid dialog close/reopen
also loses final focus restoration.
the same or separate roots. Nested dialog-to-popover dismissal restores focus in
order, and final dismissal returns to the original outside control. A dialog
that closes and reopens during retained exit presence also preserves and restores
its original opener; disconnected or cross-tree restore targets are ignored.

These observations do not select or design production overlay coordination.
YPE-5356 owns deciding whether and how to support concurrent peers and exact
final focus restoration. The detailed Chromium evidence and remaining
validation live in the rollout plan.
YPE-5356 owns deciding whether and how to support concurrent peers. The detailed
Chromium evidence and remaining validation live in the rollout plan.

Radix's development-only relationship checks can also emit warnings for valid
IDs inside a shadow root because those checks query the document rather than
Expand Down
17 changes: 8 additions & 9 deletions docs/shadow-dom-isolation-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,17 @@ This is a working plan, not approval for package-wide rollout.
- Nested and concurrent overlays within and across component shadow roots were
exercised through the production `ShadowRootHost` seam (YPE-5355). The Shadow
DOM ADR records the architectural boundary; the results below record the
supported contract and peer-dismissal and final focus-restoration gaps. Runtime
changes remain YPE-5356.
supported contract and the remaining peer-dismissal gap. Runtime changes remain
YPE-5356.

## Nested and concurrent overlay evidence

| Scenario | Result | Evidence and limitation |
| --- | --- | --- |
| Verse action popover opens the highlights permission dialog | Partially supported | Both render in the shadow-local top layer. The dialog receives focus and the wrapper is inert. A first Escape closes only the dialog and restores focus inside the popover; a second closes the popover. Chromium spike observation found no final focus restoration. Automated evidence covers the stable dismissal, containment, and teardown contract without requiring that known gap to remain. |
| Verse action popover opens the highlights permission dialog | Supported | Both render in the shadow-local top layer. The dialog receives focus and the wrapper is inert. A first Escape closes only the dialog and restores focus inside the popover; a second closes the popover and restores the original outside control. |
| Dialog contains a popover | Supported | The popover is interactive and receives focus. Escape closes it first and restores its trigger while the dialog remains modal. A second Escape closes the dialog and restores its opener. |
| Two independent popovers in the same or separate roots | Unsupported as concurrent peers | Chromium spike observation found that opening a popover dismisses an existing peer through Radix outside interaction, including when the peers use separate component shadow roots. Automated evidence covers the stable contract that the new peer receives focus, remains non-modal, tears down cleanly, and that separate roots remain usable after primary teardown. |
| Rapid dialog close/reopen during exit | Unsupported | The first dialog remains connected in its closed state when it is reopened. The reopened dialog receives focus and the wrapper remains inert. Chromium spike observation found that final dismissal loses the opener; automated evidence requires safe modal release without locking in that focus loss. |
| Rapid dialog close/reopen during exit | Supported | The first dialog remains connected in its closed state when it is reopened. The reopened dialog receives focus and the wrapper remains inert. Final dismissal releases the modal state and restores the original opener. Disconnected or cross-tree restore targets are ignored. |

Separate `ShadowRootHost` instances use different portal containers and tear
those containers down independently. This provides lifecycle isolation, not
Expand All @@ -55,7 +55,7 @@ open.
| Portal lifecycle | Unit and browser coverage exercise lazy creation, exit-animation retention, cleanup, immediate reopen behavior, and the direct-Radix `VerseActionPopover` consumer. | Validated for shared primitives and the known bypass | Repeat the consumer audit when adding another direct overlay primitive. |
| Dialog relationships | Chromium resolves title and description relationships inside the component tree. | Validated in Chromium | Verify announcements with real assistive technology. |
| Dialog keyboard containment | Browser coverage exercises initial focus, programmatic escape redirection, forward and reverse traversal, radio-group collapsing, negative `tabindex`, and wraparound. | Validated in Chromium | Expand the browser and assistive-technology matrix. |
| Dialog modal lifetime | Coverage verifies inert background content while open and through staggered Content and Overlay exit animations. YPE-5355 also exercises both unmount orders for overlapping popover and dialog exits. | Validated for order-independent teardown | YPE-5356 owns peer concurrency across component roots and final focus-restoration gaps. Expand browser and assistive-technology coverage. |
| Dialog modal lifetime | Coverage verifies inert background content while open and through staggered Content and Overlay exit animations. YPE-5355 also exercises both unmount orders for overlapping popover and dialog exits. | Validated for order-independent teardown | YPE-5356 owns peer concurrency across component roots. Expand browser and assistive-technology coverage. |
| Dialog dismissal and restoration | Coverage exercises Escape, backdrop click, full-viewport hit testing, overlay-only focus, and restoration after both modal nodes unmount. | Validated in Chromium | Verify real screen-reader and cross-browser behavior. |
| Consumer form participation | Chromium coverage verifies that a light-DOM form does not own or serialize a native control inside an SDK shadow root. | Unsupported across tree scopes | Use an explicit component contract if a rollout target requires outer-form participation. |
| Consumer labels and ARIA ID references | Chromium coverage verifies that external native labels, `aria-labelledby`, and `aria-describedby` relationships do not resolve to controls inside the root. | Unsupported across tree scopes | Keep relationships in one tree scope; verify real assistive technology separately. |
Expand Down Expand Up @@ -84,10 +84,9 @@ decision. YPE-5356 owns whether and how to implement that coordination.
- Apply [ADR 0007's client-only SSR and hydration contract](adr/0007-prototype-shadow-dom-style-isolation.md#ssr-and-hydration-contract)
per rollout component. YPE-5356 decides whether its first-paint, layout, and
no-JavaScript limitations are acceptable for that component.
- Resolve the YPE-5355 peer-dismissal and final focus-restoration gaps before
shipping nested and concurrent overlays (YPE-5356). The decision must consider
trigger-time peer dismissal as well as overlay order and restore targets;
ADR 0007 records the gaps but does not select a coordination design.
- Resolve the YPE-5355 peer-dismissal gap before shipping concurrent peer
overlays (YPE-5356). The decision must consider trigger-time peer dismissal
and overlay order; ADR 0007 records the gap but does not select a coordination design.
Cross-browser and assistive-technology coverage still remain.
- Keep the YPE-5400 custom-property contract and compiled-stylesheet prevention
guard green as component styles change. The audit below closes the known
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Meta, StoryObj } from '@storybook/react-vite';
import { http, HttpResponse } from 'msw';
import { useCallback, useEffect, useRef, useState } from 'react';
import { expect, userEvent, waitFor } from 'storybook/test';
import { expect, spyOn, userEvent, waitFor } from 'storybook/test';
import { ShadowRootHost } from '../lib/shadow-root-host';
import { requireShadowRoot } from '../test/dom-stubs';
import { HighlightPermissionDialog } from './highlight-permission-dialog';
Expand Down Expand Up @@ -369,7 +369,7 @@ function manualWorkflow(

export const PopoverOpensDialog = manualWorkflow(
'Popover opens dialog — manual workflow',
'Focus Prior control, click First verse, choose a highlight color, then press Escape twice. Focus returns inside the popover after the first Escape; after the second, confirm it does not return to Prior control.',
'Focus Prior control, click First verse, choose a highlight color, then press Escape twice. Confirm focus returns inside the popover after the first Escape and to Prior control after the second.',
);
export const DialogContainsPopover = manualWorkflow(
'Dialog contains popover — manual workflow',
Expand All @@ -381,11 +381,11 @@ export const TwoIndependentOverlays = manualWorkflow(
);
export const RapidCloseReopenDuringExit = manualWorkflow(
'Rapid close/reopen during exit — manual workflow',
'Focus Run rapid close/reopen and press Enter once. The dialog opens, closes, and reopens during retained exit presence. Press Escape after it reopens; confirm the page unlocks but focus does not return to the Run button.',
'Focus Run rapid close/reopen and press Enter once. The dialog opens, closes, and reopens during retained exit presence. Press Escape after it reopens; confirm the page unlocks and focus returns to the Run button.',
{ enableRapidReopen: true },
);

/** YPE-5355 stable regression evidence; unsupported observations remain in the decision docs. */
/** YPE-5355 regression evidence with YPE-5889 final-focus coverage. */
export const PopoverOpensDialogEvidence: Story = {
tags: ['!dev'],
play: async ({ canvasElement, step }) => {
Expand Down Expand Up @@ -453,8 +453,7 @@ export const PopoverOpensDialogEvidence: Story = {
await waitFor(() => {
void expect(topLayer.querySelector('[data-slot="verse-action-popover"]')).toBeNull();
void expect(topLayer.matches(':popover-open')).toBe(false);
const focused = root.activeElement;
void expect(focused === null || !topLayer.contains(focused)).toBe(true);
void expect(root.activeElement).toBe(priorControl);
});
});

Expand Down Expand Up @@ -701,7 +700,7 @@ export const TwoIndependentOverlaysEvidence: Story = {
},
};

/** YPE-5355 stable regression evidence; final-focus observations remain in the docs. */
/** YPE-5355 lifecycle evidence with YPE-5889 final-focus coverage. */
export const RapidCloseReopenDuringExitEvidence: Story = {
tags: ['!dev'],
args: { enableRapidReopen: true },
Expand Down Expand Up @@ -765,9 +764,105 @@ export const RapidCloseReopenDuringExitEvidence: Story = {
void expect(getPermissionDialog(topLayer)).toBeNull();
void expect(contentWrapper.inert).toBe(false);
void expect(topLayer.matches(':popover-open')).toBe(false);
const focused = root.activeElement;
void expect(focused === null || !topLayer.contains(focused)).toBe(true);
void expect(root.activeElement).toBe(runRapidCloseReopen);
});
});
} finally {
exitAnimationStyle.remove();
}
},
};

export const RapidCloseReopenIgnoresInvalidOpenerEvidence: Story = {
tags: ['!dev'],
args: { enableRapidReopen: true },
play: async ({ canvasElement, step }) => {
const { contentWrapper, root } = await getPrimaryHarness(canvasElement);
const runRapidCloseReopen = await waitForElement<HTMLButtonElement>(
root,
'[data-testid="run-rapid-close-reopen"]',
'rapid close/reopen control not rendered',
);
const secondaryIsland = await waitForElement<HTMLElement>(
canvasElement,
'[data-testid="secondary-island"]',
'secondary island not rendered',
);
const secondaryRoot = await waitFor(() => requireShadowRoot(secondaryIsland));
const exitAnimationStyle = installUnequalExitDurations(root, {
dialog: 400,
overlay: 800,
popover: 180,
});
let topLayer!: HTMLElement;

try {
await step('Reopen the dialog before its retained exit completes', async () => {
await userEvent.click(runRapidCloseReopen);
topLayer = await waitForElement<HTMLElement>(
root,
'[data-yv-shadow-local-overlay]',
'shadow-local top layer not created',
);
const firstDialog = await waitFor(() => {
const dialog = getPermissionDialog(topLayer);
if (!dialog) throw new Error('permission dialog did not open');
return dialog;
});
await waitFor(() => void expect(firstDialog).toHaveAttribute('data-state', 'closed'));
await waitFor(() => {
const dialog = getPermissionDialog(topLayer);
if (!dialog || dialog.getAttribute('data-state') === 'closed') {
throw new Error('permission dialog did not reopen during exit');
}
void expect(contentWrapper.inert).toBe(true);
});
});

await step('Ignore the opener after it moves to another shadow root', async () => {
secondaryRoot.append(runRapidCloseReopen);
void expect(runRapidCloseReopen.getRootNode()).toBe(secondaryRoot);

await userEvent.keyboard('{Escape}');
await waitFor(() => {
void expect(getPermissionDialog(topLayer)).toBeNull();
void expect(contentWrapper.inert).toBe(false);
void expect(topLayer.matches(':popover-open')).toBe(false);
});
void expect(secondaryRoot.activeElement).not.toBe(runRapidCloseReopen);
});

await step('Ignore the opener after it disconnects', async () => {
contentWrapper.append(runRapidCloseReopen);
await userEvent.click(runRapidCloseReopen);
const firstDialog = await waitFor(() => {
const dialog = getPermissionDialog(topLayer);
if (!dialog) throw new Error('permission dialog did not reopen for disconnected target');
return dialog;
});
await waitFor(() => void expect(firstDialog).toHaveAttribute('data-state', 'closed'));
await waitFor(() => {
const dialog = getPermissionDialog(topLayer);
if (!dialog || dialog.getAttribute('data-state') === 'closed') {
throw new Error('permission dialog did not rapidly reopen for disconnected target');
}
});

const focus = spyOn(runRapidCloseReopen, 'focus');
try {
runRapidCloseReopen.remove();
void expect(runRapidCloseReopen.isConnected).toBe(false);
await userEvent.keyboard('{Escape}');
await waitFor(() => {
void expect(getPermissionDialog(topLayer)).toBeNull();
void expect(contentWrapper.inert).toBe(false);
void expect(topLayer.matches(':popover-open')).toBe(false);
});
void expect(focus).not.toHaveBeenCalled();
void expect(root.activeElement).not.toBe(runRapidCloseReopen);
} finally {
focus.mockRestore();
}
});
} finally {
exitAnimationStyle.remove();
Expand Down
59 changes: 59 additions & 0 deletions packages/ui/src/components/ui/dialog.test.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,50 @@
import { render, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { useState, type ReactNode } from 'react';
import { describe, expect, it, vi } from 'vitest';
import { ShadowRootHost } from '@/lib/shadow-root-host';
import { requireShadowRoot } from '@/test/dom-stubs';
import { Dialog, DialogContent, DialogTitle } from './dialog';

function PreventedCloseAutoFocusHarness(): ReactNode {
const [open, setOpen] = useState(false);
const [preventCloseAutoFocus, setPreventCloseAutoFocus] = useState(false);

return (
<ShadowRootHost portalStrategy="local-inline">
<button
type="button"
onClick={() => {
setPreventCloseAutoFocus(true);
setOpen(true);
}}
>
First opener
</button>
<button
type="button"
onClick={() => {
setPreventCloseAutoFocus(false);
setOpen(true);
}}
>
Second opener
</button>
<Dialog open={open} onOpenChange={setOpen}>
<DialogContent
aria-describedby={undefined}
onCloseAutoFocus={(event) => {
if (preventCloseAutoFocus) event.preventDefault();
}}
>
<DialogTitle>Title</DialogTitle>
<button type="button">Inside</button>
</DialogContent>
</Dialog>
</ShadowRootHost>
);
}

describe('Dialog shadow portal coordination', () => {
it('preserves light-DOM state and ref behavior without changing overlay focusability', async () => {
const cleanup = vi.fn();
Expand Down Expand Up @@ -109,4 +149,23 @@ describe('Dialog shadow portal coordination', () => {
});
expect(wrapper?.inert).toBe(false);
});

it('captures a new opener after a consumer prevents close autofocus', async () => {
const user = userEvent.setup();
const { container } = render(<PreventedCloseAutoFocusHarness />);
const shadowRoot = requireShadowRoot(container);
const [firstOpener, secondOpener] = Array.from(shadowRoot.querySelectorAll('button'));

await user.click(firstOpener!);
await waitFor(() => expect(shadowRoot.querySelector('[role="dialog"]')).not.toBeNull());
await user.keyboard('{Escape}');
await waitFor(() => expect(shadowRoot.querySelector('[role="dialog"]')).toBeNull());

await user.click(secondOpener!);
await waitFor(() => expect(shadowRoot.querySelector('[role="dialog"]')).not.toBeNull());
await user.keyboard('{Escape}');
await waitFor(() => expect(shadowRoot.querySelector('[role="dialog"]')).toBeNull());

expect(shadowRoot.activeElement).toBe(secondOpener);
});
});
8 changes: 6 additions & 2 deletions packages/ui/src/components/ui/use-shadow-dialog-focus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export function useShadowDialogFocus({
capturedRestoreFocusRef.current = false;
return;
}
if (!container || capturedRestoreFocusRef.current) return;
if (!container || capturedRestoreFocusRef.current || restoreFocusRef.current) return;

const shadowRoot = getOwnShadowRoot(container);
const activeElement = shadowRoot
Expand All @@ -134,7 +134,11 @@ export function useShadowDialogFocus({

const onCloseAutoFocus = React.useCallback(
(event: Event): void => {
if (event.defaultPrevented || container === undefined) return;
if (container === undefined) return;
if (event.defaultPrevented) {
restoreFocusRef.current = null;
return;
}

event.preventDefault();
const restoreFocusTo = restoreFocusRef.current;
Expand Down
Loading
Loading