Skip to content

Commit 17321da

Browse files
committed
fix(devtools): keep openAsModal on top of later dialogs and release removed ones
Two cases left the devtools or the page stuck: - The app opened another modal dialog while the panel was on top. That dialog became the topmost modal and the devtools were inert behind it. The host is now shown again when an app dialog opens after it. - The app removed an open dialog without close(), for example on unmount. That is a child list change, which the observer did not watch, so the host stayed modal and the page stayed inert. The observer now watches child list changes too.
1 parent 82bd84f commit 17321da

2 files changed

Lines changed: 65 additions & 6 deletions

File tree

‎e2e/apps/react-vite/tests/open-as-modal.spec.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,46 @@ test.describe('openAsModal', () => {
7373
).toHaveCount(0)
7474
})
7575

76+
test('stays on top when the app opens another modal dialog', async ({
77+
page,
78+
}) => {
79+
const dt = new DevtoolsPage(page)
80+
await dt.goto('/?open-as-modal')
81+
await expect(dt.trigger()).toBeVisible()
82+
await page.getByTestId('open-app-dialog').click()
83+
await pressOpenHotkey(page)
84+
await dt.expectOpen()
85+
86+
await page.evaluate(() => {
87+
const second = document.createElement('dialog')
88+
second.textContent = 'second app dialog'
89+
document.body.append(second)
90+
second.showModal()
91+
})
92+
93+
await expect.poll(() => closeButtonTakesClicks(page)).toBe(true)
94+
})
95+
96+
test('releases the page when an open app dialog is removed', async ({
97+
page,
98+
}) => {
99+
const dt = new DevtoolsPage(page)
100+
await dt.goto('/?open-as-modal')
101+
await expect(dt.trigger()).toBeVisible()
102+
await page.getByTestId('open-app-dialog').click()
103+
await pressOpenHotkey(page)
104+
await dt.expectOpen()
105+
106+
// Removed without close(), as when a component unmounts.
107+
await page.evaluate(() => document.querySelector('#app-dialog')!.remove())
108+
109+
await expect(
110+
page.locator('dialog [data-testid="tanstack_devtools"]'),
111+
).toHaveCount(0)
112+
await expect(page.getByTestId('text-input')).toBeEditable()
113+
await page.getByTestId('text-input').fill('works')
114+
})
115+
76116
test('without the option, a modal app dialog blocks the panel', async ({
77117
page,
78118
}) => {

‎packages/devtools/src/hooks/use-modal-host.ts‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,12 @@ export function createModalHost(
3939
let home: Node | null = null
4040

4141
const sync = () => {
42-
const appModalOpen = Array.from(doc.querySelectorAll('dialog')).some(
43-
(dialog) => dialog !== host && dialog.matches(':modal'),
44-
)
45-
if (isOpen() && appModalOpen) {
42+
const wanted =
43+
isOpen() &&
44+
Array.from(doc.querySelectorAll('dialog')).some(
45+
(dialog) => dialog !== host && dialog.matches(':modal'),
46+
)
47+
if (wanted) {
4648
if (host.open) return
4749
home = element.parentNode
4850
host.append(element)
@@ -55,10 +57,27 @@ export function createModalHost(
5557
}
5658
}
5759

58-
// `showModal()` and `close()` toggle the `open` attribute.
59-
const observer = new MutationObserver(sync)
60+
// `showModal()` and `close()` toggle the `open` attribute. Removing an open
61+
// dialog from the page is a child list change instead.
62+
const observer = new MutationObserver((records) => {
63+
// An app dialog shown after the host goes on top of it. Showing the host
64+
// again puts the host back on top.
65+
const appModalShown = records.some(
66+
(record) =>
67+
record.type === 'attributes' &&
68+
record.target !== host &&
69+
(record.target as Element).matches('dialog:modal'),
70+
)
71+
if (host.open && appModalShown) {
72+
host.close()
73+
host.showModal()
74+
return
75+
}
76+
sync()
77+
})
6078
observer.observe(doc.documentElement, {
6179
subtree: true,
80+
childList: true,
6281
attributeFilter: ['open'],
6382
})
6483
createEffect(() => {

0 commit comments

Comments
 (0)