From c8db507932f70350ca5f07486f052be5f0f412eb Mon Sep 17 00:00:00 2001 From: Daniel G Date: Sat, 24 Oct 2026 09:25:04 +0200 Subject: [PATCH] fix(ui): dismiss notifications without closing modal editors (#32065) * fix(ui): allow toast dismissal over modal panels * test(ui): retain toast stacking and credential boundaries * Preserve modal state when dismissing stacked toasts * fix(ui): limit modal toast gate to dialog and sheet overlays Radix menus and selects also set body pointer-events:none; gating on that blocked toast clicks from closing an open dropdown. --------- Co-authored-by: Neil Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> --- docs/STYLEGUIDE.md | 2 + src/renderer/src/assets/main.css | 10 ++ .../components/star-nag/StarNagToastHost.tsx | 1 + src/renderer/src/components/ui/sonner.tsx | 98 +++++++++++++------ .../add-project-error-toast-stacking.spec.ts | 82 +++++++++++++--- 5 files changed, 152 insertions(+), 41 deletions(-) diff --git a/docs/STYLEGUIDE.md b/docs/STYLEGUIDE.md index 9b9a6f5a62f..151ce7ea202 100644 --- a/docs/STYLEGUIDE.md +++ b/docs/STYLEGUIDE.md @@ -179,6 +179,8 @@ When a control has multiple plausible primitives, use this fork: If you find yourself styling around a primitive (`` to act like a ``, or vice versa), stop and reconsider — the focus-management semantics differ and a future contributor will be misled by the mismatch. +Above a modal, only toast close buttons remain clickable. Dismissing a toast must leave the modal open, with focus and unsaved input intact. + ### Tooltips Tooltips exist to _name_ a control whose meaning isn't obvious from its appearance. They are not the place to teach, persuade, or warn — anything users need to read while acting belongs in the visible UI. diff --git a/src/renderer/src/assets/main.css b/src/renderer/src/assets/main.css index 6dc469142f7..3e6e4531483 100644 --- a/src/renderer/src/assets/main.css +++ b/src/renderer/src/assets/main.css @@ -558,6 +558,16 @@ body:has(> [data-slot='dialog-overlay'], > [data-slot='sheet-overlay']) { line-height: 1.45; } +/* Visible toasts absorb backdrop clicks; Toaster gates actions while a modal owns the pointer lock. */ +body:has(> [data-slot='dialog-overlay'], > [data-slot='sheet-overlay']) + [data-sonner-toaster] + [data-sonner-toast][data-visible='true'][data-removed='false']:is( + [data-front='true'], + [data-expanded='true'] + ) { + pointer-events: auto; +} + [data-sonner-toaster] [data-sonner-toast][data-styled='true'] [data-close-button] { /* Why: Sonner's dark-theme defaults can resolve the close icon and button background to the same color; keep the X readable on both Orca themes. */ diff --git a/src/renderer/src/components/star-nag/StarNagToastHost.tsx b/src/renderer/src/components/star-nag/StarNagToastHost.tsx index d61a35482b2..6ad3292f58e 100644 --- a/src/renderer/src/components/star-nag/StarNagToastHost.tsx +++ b/src/renderer/src/components/star-nag/StarNagToastHost.tsx @@ -130,6 +130,7 @@ function StarNagToast({ variant="ghost" size="icon" className="size-7 shrink-0" + data-close-button onClick={close} disabled={busy} aria-label={translate('auto.components.star.nag.StarNagToastHost.dismiss', 'Dismiss')} diff --git a/src/renderer/src/components/ui/sonner.tsx b/src/renderer/src/components/ui/sonner.tsx index 3096b4882cc..1be3c8166a6 100644 --- a/src/renderer/src/components/ui/sonner.tsx +++ b/src/renderer/src/components/ui/sonner.tsx @@ -5,42 +5,84 @@ import { OctagonXIcon, TriangleAlertIcon } from 'lucide-react' +import type { SyntheticEvent } from 'react' import { Toaster as Sonner, type ToasterProps } from 'sonner' import { useAppStore } from '@/store' +// Why: scope to real modals; Radix menus/selects also set body pointer-events:none. +function isModalOverlayOpen(element: Element): boolean { + return ( + element.ownerDocument.body.querySelector( + ":scope > [data-slot='dialog-overlay'], :scope > [data-slot='sheet-overlay']" + ) !== null + ) +} + +function blockModalToastInteraction(event: SyntheticEvent): void { + const target = event.target + if ( + target instanceof Element && + target.closest('[data-sonner-toast]') !== null && + target.closest('[data-close-button]') === null && + isModalOverlayOpen(target) + ) { + event.preventDefault() + event.stopPropagation() + } +} + const Toaster = ({ ...props }: ToasterProps) => { const theme = useAppStore((s) => s.settings?.theme) || 'system' return ( - , - info: , - warning: , - error: , - loading: +
{ + if ( + event.target instanceof Element && + event.target.closest('[data-close-button]') && + isModalOverlayOpen(event.target) + ) { + // Toast interactions must not reach the backdrop or move modal focus. + event.preventDefault() + event.stopPropagation() + } else { + blockModalToastInteraction(event) + } }} - style={ - { - '--normal-bg': 'var(--popover)', - '--normal-text': 'var(--popover-foreground)', - '--normal-border': 'var(--border)', - '--border-radius': 'var(--radius)', - '--width': 'min(26rem, calc(100vw - 2rem))' - } as React.CSSProperties - } - {...props} - /> + onClickCapture={blockModalToastInteraction} + onAuxClickCapture={blockModalToastInteraction} + onContextMenuCapture={blockModalToastInteraction} + > + , + info: , + warning: , + error: , + loading: + }} + style={ + { + '--normal-bg': 'var(--popover)', + '--normal-text': 'var(--popover-foreground)', + '--normal-border': 'var(--border)', + '--border-radius': 'var(--radius)', + '--width': 'min(26rem, calc(100vw - 2rem))' + } as React.CSSProperties + } + {...props} + /> +
) } diff --git a/tests/e2e/add-project-error-toast-stacking.spec.ts b/tests/e2e/add-project-error-toast-stacking.spec.ts index e482404367a..3f46ee7d59c 100644 --- a/tests/e2e/add-project-error-toast-stacking.spec.ts +++ b/tests/e2e/add-project-error-toast-stacking.spec.ts @@ -1,5 +1,5 @@ import path from 'node:path' -import type { ElectronApplication, TestInfo } from '@stablyai/playwright-test' +import type { ElectronApplication, Locator, TestInfo } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' import { openSidebarProjectDialog } from './helpers/sidebar-project-dialog' import { @@ -92,15 +92,65 @@ for (const theme of ['dark', 'light'] as const) { expect(layers.toast, 'dialog overlay covers the project error').toBeGreaterThan( layers.overlay ) - expect(layers.pointerEvents).toBe('none') + expect(layers.pointerEvents).toBe('auto') await pathField.click() - // Radix still owns pointer/focus containment; a visible toast must not steal the form. - await expect( - toast.locator('[data-close-button]').click({ trial: true, timeout: 300 }) - ).rejects.toThrow() + await toast.locator('[data-close-button]').click() + await expect(toast).toBeHidden() + await expect(page.getByRole('dialog', { name: /Open host project/i })).toBeVisible() await expect(pathField).toHaveValue(missingPath) expect(await pathField.evaluate((input) => input === document.activeElement)).toBe(true) + for (const pointerType of ['mouse', 'touch'] as const) { + await client.app.evaluate(({ BrowserWindow }) => { + BrowserWindow.getAllWindows()[0].webContents.send('star-nag:show', { + mode: 'web', + surface: 'toast' + }) + }) + const customToast = page + .locator('[data-sonner-toast]') + .filter({ hasText: 'Onboarding completed!' }) + await expect(customToast).toBeVisible() + const clickToastControl = async (control: Locator): Promise => { + if (pointerType === 'mouse') { + await control.click() + return + } + const cdp = await page.context().newCDPSession(page) + try { + await cdp.send('Emulation.setTouchEmulationEnabled', { enabled: true }) + await control.click({ trial: true }) + const bounds = await control.boundingBox() + if (!bounds) { + throw new Error('Missing toast control target') + } + await cdp.send('Input.dispatchTouchEvent', { + type: 'touchStart', + touchPoints: [{ x: bounds.x + bounds.width / 2, y: bounds.y + bounds.height / 2 }] + }) + await cdp.send('Input.dispatchTouchEvent', { type: 'touchEnd', touchPoints: [] }) + } finally { + await cdp.send('Emulation.setTouchEmulationEnabled', { enabled: false }) + await cdp.detach() + } + } + await pathField.click() + await clickToastControl(customToast.getByRole('button', { name: 'Later', exact: true })) + await expect(customToast).toBeVisible() + await expect(page.getByRole('dialog', { name: /Open host project/i })).toBeVisible() + await expect(pathField).toHaveValue(missingPath) + expect(await pathField.evaluate((input) => input === document.activeElement)).toBe(true) + await clickToastControl(customToast.getByRole('button', { name: 'Dismiss', exact: true })) + await expect(customToast).toBeHidden() + await expect(page.getByRole('dialog', { name: /Open host project/i })).toBeVisible() + await expect(pathField).toHaveValue(missingPath) + expect(await pathField.evaluate((input) => input === document.activeElement)).toBe(true) + } + await page.screenshot({ + path: testInfo.outputPath(`toast-dismissed-modal-preserved-${theme}.png`), + animations: 'disabled' + }) + // A real action-toast renderer, fed a synthetic notification in this disposable client only. await client.app.evaluate(({ ipcMain, BrowserWindow }) => { ipcMain.removeHandler('mobile:consumePendingUnpairedDeviceAuthFailure') @@ -112,7 +162,11 @@ for (const theme of ['dark', 'light'] as const) { .filter({ hasText: 'A device tried to connect but is not paired' }) const action = actionToast.getByRole('button', { name: 'Open Mobile Settings' }) await expect(action).toBeVisible() - await expect(action.click({ trial: true, timeout: 300 })).rejects.toThrow() + await action.click() + await actionToast.locator('[data-title]').click() + await expect(actionToast).toBeVisible() + await expect(page.getByRole('dialog', { name: /Open host project/i })).toBeVisible() + await expect(page.getByPlaceholder('Search settings')).toBeHidden() await expect(pathField).toHaveValue(missingPath) expect(await pathField.evaluate((input) => input === document.activeElement)).toBe(true) @@ -137,6 +191,10 @@ for (const theme of ['dark', 'light'] as const) { Math.max(...overlays.map((overlay) => Number(getComputedStyle(overlay).zIndex))) ) ).toBe(140) + await expect( + actionToast.locator('[data-close-button]').click({ trial: true, timeout: 300 }) + ).rejects.toThrow() + await expect(page.getByRole('dialog', { name: 'SSH Password' })).toBeVisible() await page.evaluate(() => window.__store?.getState().removeSshCredentialRequest('toast-layer-fixture') ) @@ -147,9 +205,7 @@ for (const theme of ['dark', 'light'] as const) { .click() await expect(page.locator('[data-slot="dialog-overlay"]')).toHaveCount(0) const toaster = page.locator('[data-sonner-toaster]') - await expect(toaster).toHaveCSS('z-index', '40') - await toast.locator('[data-close-button]').click() - await expect(toast).toBeHidden() + await expect(toaster).toHaveCSS('z-index', '47') await page.evaluate(async () => { await window.__store?.getState().updateSettings({ experimentalAgentDashboardPopout: true }) @@ -157,15 +213,15 @@ for (const theme of ['dark', 'light'] as const) { await page.evaluate(() => window.__store?.getState().setAgentDashboardDrawerOpen(true)) await expect(page.locator('[data-agent-dashboard-sheet]')).toBeVisible() await expect(page.locator('[data-slot="sheet-overlay"]')).toHaveCount(0) - await expect(toaster).toHaveCSS('z-index', '40') + await expect(toaster).toHaveCSS('z-index', '47') await page.screenshot({ path: testInfo.outputPath(`nonmodal-drawer-${theme}.png`), animations: 'disabled' }) await page.evaluate(() => window.__store?.getState().setAgentDashboardDrawerOpen(false)) - // Scroll-lock is also used by selects and menus and must not by itself lift notifications. + // Scroll-lock alone must not lift notifications over nonmodal surfaces. await page.evaluate(() => document.body.setAttribute('data-scroll-locked', '1')) - await expect(toaster).toHaveCSS('z-index', '40') + await expect(toaster).toHaveCSS('z-index', '47') await page.evaluate(() => document.body.removeAttribute('data-scroll-locked')) await action.click() await expect(page.getByPlaceholder('Search settings')).toBeVisible()