Skip to content

Commit eb746ad

Browse files
benjamincanaczernoniaclaude
authored
feat(Dialog): add unmountOnHide prop (#2662)
* feat(Dialog): support unmountOnHide When set to false, Dialog content and overlay stay in the DOM when closed (hidden via v-show) instead of being unmounted. Useful for SEO and performance by avoiding remounts on every open. Closes #1727 * test(Dialog): add tests for unmountOnHide Also fixes focus restoration when unmountOnHide=false — since FocusScope never unmounts, close-auto-focus doesn't fire. Added a watcher on present to manually restore focus to trigger when the dialog hides. * fix(Dialog): replace deprecated SpyInstance with MockInstance * fix(Dialog): restore focus for non-modal when unmountOnHide=false The focus-restoration watcher only lived in DialogContentModal, so a non-modal dialog with unmountOnHide=false never returned focus to the trigger on close (close-auto-focus doesn't fire while mounted). Mirror the watcher in DialogContentNonModal, respecting interact-outside, and document why the watcher exists in both. * test(Dialog): exercise open→close cycle for aria-hidden check Assert the rest of the page stays accessible after a full open/close cycle (content remains mounted) rather than just checking initial state. * fix(DismissableLayer): restore body pointer-events when toggled off while mounted When `disableOutsidePointerEvents` toggles true→false without unmounting (the unmountOnHide=false case), the watchEffect cleanup read the prop reactively and saw the new false value, skipping the body pointer-events restore and leaving the page frozen. Capture the value at run time for cleanup, and read the layer set size via toRaw to avoid self-retriggering. * fix(FocusScope): re-run mount auto-focus when re-trapped while mounted With unmountOnHide=false the FocusScope stays mounted across open/close, so the mount auto-focus (keyed off physical mount) never re-fires on reopen and focus never enters the content. Watch the trapped false->true transition and re-run the mount auto-focus. The default unmount-on-close path mounts already trapped, so this transition never happens there and behavior is unchanged. * fix(FocusScope): retry auto-focus across v-show visibility frame When a force-mounted scope (`unmountOnHide: false`) becomes trapped on reopen, the `v-show` visibility can apply a frame after `trapped` flips, so the first focus attempt runs while the container is still `display: none` and no-ops. Retry the focus move on the next frame without re-dispatching the auto-focus event. * fix(FocusScope): drive auto-focus off visibility for force-mounted scopes A force-mounted scope (Dialog `unmountOnHide: false`) physically mounts once while hidden via `v-show`, so keying auto-focus off physical mount fired `mountAutoFocus`/`openAutoFocus` and stole focus into a closed dialog, and never re-focused non-modal content on open (it isn't trapped). Gate the mount-time auto-focus on visibility and re-run it on the hidden -> visible transition via a MutationObserver, covering first open and reopen for both modal and non-modal. Replaces the trapped watcher + rAF retry, which only handled the modal case. * fix(Dialog): drive FocusScope and DismissableLayer off presence for unmountOnHide When `unmountOnHide` is `false` the dialog content stays mounted while closed, but FocusScope and DismissableLayer keyed their global stack membership off physical mount. With more than one keep-mounted dialog on a page, the later-mounted hidden layer was treated as the topmost layer (swallowing Escape and outside interactions meant for the open dialog) and its hidden scope permanently paused the open dialog's focus trap. - FocusScope: replace `display: none` sniffing with an explicit `present` prop; stack membership and mount auto-focus now follow `present` transitions instead of mount. Drop the per-consumer MutationObserver. - DismissableLayer: add an internal `present` prop (kept out of the public `DismissableLayerProps`); layer-stack membership, body pointer-events locking and outside-interaction events now follow presence, so hidden layers no longer swallow dismissal or emit `interactOutside` while closed. - Dialog: forward `present` through DialogContentImpl to both primitives. - Tests: regression coverage for the two-dialog scenario; replace `setTimeout` magic waits with `vi.waitFor`. * fix(DismissableLayer): gate Escape handling on present A layer kept mounted while hidden (e.g. a Dialog with `unmountOnHide: false`) is out of the layer stack, so its `index` is `-1`. When no layer is visible (`size === 0`), `-1 === size - 1` made the hidden layer look like the highest layer and emit `escapeKeyDown` / `dismiss` on Escape for an already-closed dialog. Gate the Escape handler on `props.present`, consistent with the existing `pointerDownOutside` / `focusOutside` guards. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: zernonia <zernonia@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 01bdb66 commit eb746ad

12 files changed

Lines changed: 556 additions & 42 deletions

‎docs/content/meta/DialogRoot.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@
2121
'description': '<p>The controlled open state of the dialog. Can be binded as <code>v-model:open</code>.</p>\n',
2222
'type': 'boolean',
2323
'required': false
24+
},
25+
{
26+
'name': 'unmountOnHide',
27+
'description': '<p>When set to <code>false</code>, the dialog content will not be unmounted when closed, but instead hidden with CSS. Useful for SEO or when you want to improve performance by not remounting the component on every open.</p>\n',
28+
'type': 'boolean',
29+
'required': false,
30+
'default': 'true'
2431
}
2532
]" />
2633

@@ -55,6 +62,7 @@
5562
| `defaultOpen` | The open state of the dialog when it is initially rendered. Use when you do not need to control its open state. | `boolean` | No | `false` |
5663
| `modal` | The modality of the dialog When set to true, <br> interaction with outside elements will be disabled and only dialog content will be visible to screen readers. | `boolean` | No | `true` |
5764
| `open` | The controlled open state of the dialog. Can be binded as v-model:open. | `boolean` | No | - |
65+
| `unmountOnHide` | When set to `false`, the dialog content will not be unmounted when closed, but instead hidden with CSS. Useful for SEO or when you want to improve performance by not remounting the component on every open. | `boolean` | No | `true` |
5866

5967
**Events**
6068

‎packages/core/src/Dialog/Dialog.test.ts‎

Lines changed: 262 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import type { DOMWrapper, VueWrapper } from '@vue/test-utils'
2-
import type { Mock, SpyInstance } from 'vitest'
2+
import type { Mock, MockInstance } from 'vitest'
33
import { createEvent, findByText, fireEvent, render } from '@testing-library/vue'
44
import { mount } from '@vue/test-utils'
55
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
66
import { axe } from 'vitest-axe'
77
import { defineComponent, nextTick } from 'vue'
8+
import { sleep } from '@/test'
89
import { DialogClose, DialogContent, DialogOverlay, DialogRoot, DialogTitle, DialogTrigger } from '.'
910

1011
const OPEN_TEXT = 'Open'
@@ -50,6 +51,29 @@ const DialogTest = defineComponent({
5051
</DialogRoot>`,
5152
})
5253

54+
const UnmountOnHideDialogTest = defineComponent({
55+
components: { DialogRoot, DialogTrigger, DialogOverlay, DialogContent, DialogClose, DialogTitle },
56+
template: `<DialogRoot :unmount-on-hide="false">
57+
<DialogTrigger>${OPEN_TEXT}</DialogTrigger>
58+
<DialogOverlay />
59+
<DialogContent>
60+
<DialogTitle>${TITLE_TEXT}</DialogTitle>
61+
<DialogClose>${CLOSE_TEXT}</DialogClose>
62+
</DialogContent>
63+
</DialogRoot>`,
64+
})
65+
66+
const NonModalUnmountOnHideDialogTest = defineComponent({
67+
components: { DialogRoot, DialogTrigger, DialogOverlay, DialogContent, DialogClose, DialogTitle },
68+
template: `<DialogRoot :modal="false" :unmount-on-hide="false">
69+
<DialogTrigger>${OPEN_TEXT}</DialogTrigger>
70+
<DialogContent>
71+
<DialogTitle>${TITLE_TEXT}</DialogTitle>
72+
<DialogClose>${CLOSE_TEXT}</DialogClose>
73+
</DialogContent>
74+
</DialogRoot>`,
75+
})
76+
5377
// Reproduces https://github.com/unovue/reka-ui/issues/2660 — the content is
5478
// nested *inside* the overlay (a common centering pattern), so pointerdown
5579
// events from controls in the content bubble up to the overlay.
@@ -67,6 +91,240 @@ const NestedContentDialogTest = defineComponent({
6791
</DialogRoot>`,
6892
})
6993

94+
describe('given a Dialog with unmountOnHide=false', () => {
95+
let wrapper: VueWrapper<InstanceType<typeof UnmountOnHideDialogTest>>
96+
let trigger: DOMWrapper<HTMLElement>
97+
98+
beforeEach(() => {
99+
document.body.innerHTML = ''
100+
wrapper = mount(UnmountOnHideDialogTest, { attachTo: document.body })
101+
trigger = wrapper.find('button')
102+
})
103+
104+
// The content is force-mounted, so unmount explicitly to avoid leaking the
105+
// layer into `DismissableLayer`'s module-level tracking set across tests.
106+
afterEach(() => {
107+
wrapper?.unmount()
108+
})
109+
110+
it('should keep content in DOM when closed after being opened', async () => {
111+
await fireEvent.click(trigger.element)
112+
await nextTick()
113+
114+
await fireEvent.keyDown(document.activeElement!, { key: 'Escape' })
115+
await nextTick()
116+
117+
const contentEl = document.querySelector('[role="dialog"]')
118+
expect(contentEl).not.toBeNull()
119+
expect((contentEl as HTMLElement).style.display).toBe('none')
120+
})
121+
122+
it('should not pull focus into the content while closed on mount', async () => {
123+
// Content is force-mounted but hidden; auto-focus must not fire yet.
124+
expect(document.querySelector('[role="dialog"]')).not.toBeNull()
125+
expect(document.activeElement).toBe(document.body)
126+
})
127+
128+
it('should focus the close button on open', async () => {
129+
await fireEvent.click(trigger.element)
130+
const closeButton = await findByText(document.body, CLOSE_TEXT)
131+
await vi.waitFor(() => expect(closeButton).toBe(document.activeElement))
132+
})
133+
134+
it('should re-focus the content when reopened', async () => {
135+
// The content stays mounted, so focus must be re-applied on each open via
136+
// the `present` false -> true transition (not just on physical mount).
137+
await fireEvent.click(trigger.element)
138+
await nextTick()
139+
140+
await fireEvent.keyDown(document.activeElement!, { key: 'Escape' })
141+
await vi.waitFor(() => expect(document.activeElement).toBe(trigger.element))
142+
143+
await fireEvent.click(trigger.element)
144+
const closeButton = await findByText(document.body, CLOSE_TEXT)
145+
await vi.waitFor(() => expect(closeButton).toBe(document.activeElement))
146+
})
147+
148+
it('should restore focus to trigger on close', async () => {
149+
await fireEvent.click(trigger.element)
150+
await nextTick()
151+
152+
await fireEvent.keyDown(document.activeElement!, { key: 'Escape' })
153+
await vi.waitFor(() => expect(document.activeElement).toBe(trigger.element))
154+
})
155+
156+
it('should not apply aria-hidden to body after open then close', async () => {
157+
await fireEvent.click(trigger.element)
158+
await nextTick()
159+
160+
await fireEvent.keyDown(document.activeElement!, { key: 'Escape' })
161+
await nextTick()
162+
163+
// Content stays mounted, but the rest of the page must stay accessible.
164+
expect(document.querySelector('[role="dialog"]')).not.toBeNull()
165+
expect(document.body.getAttribute('aria-hidden')).toBeNull()
166+
})
167+
168+
it('should pass axe accessibility tests when open', async () => {
169+
await fireEvent.click(trigger.element)
170+
await nextTick()
171+
expect(await axe(document.body)).toHaveNoViolations()
172+
})
173+
})
174+
175+
describe('given a non-modal Dialog with unmountOnHide=false', () => {
176+
let wrapper: VueWrapper<InstanceType<typeof NonModalUnmountOnHideDialogTest>>
177+
let trigger: DOMWrapper<HTMLElement>
178+
179+
beforeEach(() => {
180+
document.body.innerHTML = ''
181+
wrapper = mount(NonModalUnmountOnHideDialogTest, { attachTo: document.body })
182+
trigger = wrapper.find('button')
183+
})
184+
185+
afterEach(() => {
186+
wrapper?.unmount()
187+
})
188+
189+
it('should keep content in DOM when closed after being opened', async () => {
190+
await fireEvent.click(trigger.element)
191+
await nextTick()
192+
193+
await fireEvent.keyDown(document.activeElement!, { key: 'Escape' })
194+
await nextTick()
195+
196+
const contentEl = document.querySelector('[role="dialog"]')
197+
expect(contentEl).not.toBeNull()
198+
expect((contentEl as HTMLElement).style.display).toBe('none')
199+
})
200+
201+
it('should focus the close button on open', async () => {
202+
expect(document.activeElement).toBe(document.body)
203+
204+
await fireEvent.click(trigger.element)
205+
const closeButton = await findByText(document.body, CLOSE_TEXT)
206+
await vi.waitFor(() => expect(closeButton).toBe(document.activeElement))
207+
})
208+
209+
it('should restore focus to trigger on close', async () => {
210+
await fireEvent.click(trigger.element)
211+
await nextTick()
212+
213+
await fireEvent.keyDown(document.activeElement!, { key: 'Escape' })
214+
await vi.waitFor(() => expect(document.activeElement).toBe(trigger.element))
215+
})
216+
})
217+
218+
describe('given a Dialog with unmountOnHide=false, openAutoFocus', () => {
219+
const OpenAutoFocusDialog = defineComponent({
220+
components: { DialogRoot, DialogTrigger, DialogContent, DialogClose, DialogTitle },
221+
props: ['onOpenAutoFocus'],
222+
template: `<DialogRoot :unmount-on-hide="false">
223+
<DialogTrigger>${OPEN_TEXT}</DialogTrigger>
224+
<DialogContent @open-auto-focus="onOpenAutoFocus">
225+
<DialogTitle>${TITLE_TEXT}</DialogTitle>
226+
<DialogClose>${CLOSE_TEXT}</DialogClose>
227+
</DialogContent>
228+
</DialogRoot>`,
229+
})
230+
231+
it('should not emit openAutoFocus while closed and emit once per open', async () => {
232+
document.body.innerHTML = ''
233+
const onOpenAutoFocus = vi.fn()
234+
const wrapper = mount(OpenAutoFocusDialog, { attachTo: document.body, props: { onOpenAutoFocus } })
235+
const trigger = wrapper.find('button')
236+
237+
// Force-mounted but hidden: the auto-focus must not fire on mount.
238+
await nextTick()
239+
expect(onOpenAutoFocus).toHaveBeenCalledTimes(0)
240+
241+
await fireEvent.click(trigger.element)
242+
await vi.waitFor(() => expect(onOpenAutoFocus).toHaveBeenCalledTimes(1))
243+
244+
wrapper.unmount()
245+
})
246+
})
247+
248+
// Two dialogs with `unmountOnHide: false` coexist on the page (e.g. a menu
249+
// drawer and a cart slideover), so both contents are force-mounted from the
250+
// start. Hidden layers/scopes must not participate in the global
251+
// `DismissableLayer` and `FocusScope` stacks: the later-mounted hidden one
252+
// would otherwise be treated as the topmost layer (swallowing Escape meant for
253+
// the open dialog) and would pause the open dialog's focus trap.
254+
describe('given two Dialogs with unmountOnHide=false', () => {
255+
const TwoDialogsTest = defineComponent({
256+
components: { DialogRoot, DialogTrigger, DialogOverlay, DialogContent, DialogClose, DialogTitle },
257+
props: ['onInteractOutside'],
258+
template: `<div>
259+
<DialogRoot :unmount-on-hide="false">
260+
<DialogTrigger data-testid="first-trigger">open first</DialogTrigger>
261+
<DialogOverlay />
262+
<DialogContent data-testid="first-content">
263+
<DialogTitle>first</DialogTitle>
264+
<DialogClose data-testid="first-close">close first</DialogClose>
265+
</DialogContent>
266+
</DialogRoot>
267+
<DialogRoot :modal="false" :unmount-on-hide="false">
268+
<DialogTrigger data-testid="second-trigger">open second</DialogTrigger>
269+
<DialogContent data-testid="second-content" @interact-outside="onInteractOutside">
270+
<DialogTitle>second</DialogTitle>
271+
<DialogClose data-testid="second-close">close second</DialogClose>
272+
</DialogContent>
273+
</DialogRoot>
274+
</div>`,
275+
})
276+
277+
let wrapper: VueWrapper<InstanceType<typeof TwoDialogsTest>>
278+
let onInteractOutside: Mock
279+
280+
beforeEach(() => {
281+
document.body.innerHTML = ''
282+
onInteractOutside = vi.fn()
283+
wrapper = mount(TwoDialogsTest, { attachTo: document.body, props: { onInteractOutside } })
284+
})
285+
286+
afterEach(() => {
287+
wrapper?.unmount()
288+
})
289+
290+
it('should close the open dialog on Escape even though a hidden one mounted after it', async () => {
291+
const trigger = wrapper.find('[data-testid="first-trigger"]')
292+
await fireEvent.click(trigger.element)
293+
await nextTick()
294+
295+
const content = document.querySelector('[data-testid="first-content"]') as HTMLElement
296+
expect(content.style.display).not.toBe('none')
297+
298+
await fireEvent.keyDown(document.activeElement!, { key: 'Escape' })
299+
await nextTick()
300+
301+
expect(content.style.display).toBe('none')
302+
await vi.waitFor(() => expect(document.activeElement).toBe(trigger.element))
303+
})
304+
305+
it('should keep the modal focus trap active despite the hidden later-mounted scope', async () => {
306+
await fireEvent.click(wrapper.find('[data-testid="first-trigger"]').element)
307+
await nextTick()
308+
await vi.waitFor(() => expect(document.activeElement?.getAttribute('data-testid')).toBe('first-close'))
309+
310+
// Move focus outside the open modal dialog: the trap must pull it back.
311+
const outside = document.querySelector('[data-testid="second-trigger"]') as HTMLElement
312+
outside.focus()
313+
await nextTick()
314+
315+
const content = document.querySelector('[data-testid="first-content"]') as HTMLElement
316+
expect(content.contains(document.activeElement)).toBe(true)
317+
})
318+
319+
it('should not emit interactOutside on a closed keep-mounted dialog', async () => {
320+
// Wait out the `setTimeout(0)` before outside-pointerdown listeners attach.
321+
await sleep(1)
322+
await fireEvent.pointerDown(document.body)
323+
await nextTick()
324+
expect(onInteractOutside).not.toHaveBeenCalled()
325+
})
326+
})
327+
70328
// Reproduces https://github.com/unovue/reka-ui/issues/2677 — a modal Dialog
71329
// hardcoded `disableOutsidePointerEvents` to `true`, so the prop passed to
72330
// `DialogContent` was ignored. These are tested without a `DialogOverlay`,
@@ -86,7 +344,7 @@ function makeModalDialog(contentBinding: string) {
86344
}
87345

88346
describe('given a modal Dialog (#2677)', () => {
89-
let consoleWarnMock: SpyInstance
347+
let consoleWarnMock: MockInstance
90348

91349
beforeEach(() => {
92350
document.body.innerHTML = ''
@@ -133,7 +391,7 @@ describe('given a default Dialog', () => {
133391
let wrapper: VueWrapper<InstanceType<typeof DialogTest>>
134392
let trigger: DOMWrapper<HTMLElement>
135393
let closeButton: HTMLElement
136-
let consoleWarnMock: SpyInstance
394+
let consoleWarnMock: MockInstance
137395
let consoleWarnMockFunction: Mock
138396

139397
beforeEach(() => {
@@ -250,7 +508,7 @@ describe('given a default Dialog', () => {
250508
// native `<select>`/`<input>` interactions break.
251509
describe('given a Dialog with content nested inside the overlay', () => {
252510
let wrapper: VueWrapper<InstanceType<typeof NestedContentDialogTest>>
253-
let consoleWarnMock: SpyInstance
511+
let consoleWarnMock: MockInstance
254512

255513
beforeEach(async () => {
256514
document.body.innerHTML = ''

‎packages/core/src/Dialog/DialogContent.vue‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,25 @@ const { forwardRef } = useForwardExpose()
3838
</script>
3939

4040
<template>
41-
<Presence :present="forceMount || rootContext.open.value">
41+
<Presence
42+
v-slot="{ present }"
43+
:present="forceMount || rootContext.open.value"
44+
:force-mount="forceMount || !rootContext.unmountOnHide.value"
45+
>
4246
<DialogContentModal
4347
v-if="rootContext.modal.value"
48+
v-show="rootContext.unmountOnHide.value || present"
4449
:ref="forwardRef"
50+
:present="rootContext.unmountOnHide.value || present"
4551
v-bind="{ ...props, ...emitsAsProps, ...$attrs }"
4652
>
4753
<slot />
4854
</DialogContentModal>
4955
<DialogContentNonModal
5056
v-else
57+
v-show="rootContext.unmountOnHide.value || present"
5158
:ref="forwardRef"
59+
:present="rootContext.unmountOnHide.value || present"
5260
v-bind="{ ...props, ...emitsAsProps, ...$attrs }"
5361
>
5462
<slot />

‎packages/core/src/Dialog/DialogContentImpl.vue‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export type DialogContentImplEmits = DismissableLayerEmits & {
2121
export interface DialogContentImplProps extends DismissableLayerProps {
2222
/**
2323
* Used to force mounting when more control is needed. Useful when
24-
* controlling transntion with Vue native transition or other animation libraries.
24+
* controlling transition with Vue native transition or other animation libraries.
2525
*/
2626
forceMount?: boolean
2727
/**
@@ -41,7 +41,12 @@ import { getOpenState } from '@/Menu/utils'
4141
import { injectDialogRootContext } from './DialogRoot.vue'
4242
import { useWarning } from './utils'
4343
44-
const props = defineProps<DialogContentImplProps>()
44+
const props = defineProps<DialogContentImplProps & {
45+
/** Whether the content is currently visible. Forwarded to `FocusScope` and
46+
* `DismissableLayer` so a force-mounted dialog (`unmountOnHide: false`)
47+
* auto-focuses on show and leaves the layer/scope stacks while hidden. */
48+
present?: boolean
49+
}>()
4550
const emits = defineEmits<DialogContentImplEmits>()
4651
4752
const rootContext = injectDialogRootContext()
@@ -75,6 +80,7 @@ if (process.env.NODE_ENV !== 'production') {
7580
as-child
7681
loop
7782
:trapped="props.trapFocus"
83+
:present="props.present"
7884
@mount-auto-focus="emits('openAutoFocus', $event)"
7985
@unmount-auto-focus="emits('closeAutoFocus', $event)"
8086
>
@@ -83,6 +89,7 @@ if (process.env.NODE_ENV !== 'production') {
8389
:ref="forwardRef"
8490
:as="as"
8591
:as-child="asChild"
92+
:present="props.present"
8693
:disable-outside-pointer-events="disableOutsidePointerEvents"
8794
role="dialog"
8895
:aria-describedby="rootContext.descriptionId"

0 commit comments

Comments
 (0)