mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 08:37:49 +00:00
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 <neil@stably.ai> Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
This commit is contained in:
@@ -179,6 +179,8 @@ When a control has multiple plausible primitives, use this fork:
|
||||
|
||||
If you find yourself styling around a primitive (`<Popover>` to act like a `<Dialog>`, 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.
|
||||
|
||||
@@ -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. */
|
||||
|
||||
@@ -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')}
|
||||
|
||||
@@ -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 (
|
||||
<Sonner
|
||||
theme={theme as ToasterProps['theme']}
|
||||
position="bottom-right"
|
||||
// Why: Orca has persistent bottom chrome, so bottom-right toasts need
|
||||
// breathing room above the status bar instead of sitting on its edge.
|
||||
// mobileOffset keeps that clearance below Sonner's 600px breakpoint
|
||||
// (narrow/resized windows and the web client), which otherwise reverts
|
||||
// to Sonner's default 16px and lets toasts crowd the status bar again.
|
||||
offset={{ bottom: 'calc(2.5rem + env(safe-area-inset-bottom, 0px))' }}
|
||||
mobileOffset={{ bottom: 'calc(2.5rem + env(safe-area-inset-bottom, 0px))' }}
|
||||
className="toaster group"
|
||||
icons={{
|
||||
success: <CircleCheckIcon className="size-4" />,
|
||||
info: <InfoIcon className="size-4" />,
|
||||
warning: <TriangleAlertIcon className="size-4" />,
|
||||
error: <OctagonXIcon className="size-4" />,
|
||||
loading: <Loader2Icon className="size-4 animate-spin" />
|
||||
<div
|
||||
onPointerDownCapture={(event) => {
|
||||
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}
|
||||
>
|
||||
<Sonner
|
||||
theme={theme as ToasterProps['theme']}
|
||||
position="bottom-right"
|
||||
// Why: Orca has persistent bottom chrome, so bottom-right toasts need
|
||||
// breathing room above the status bar instead of sitting on its edge.
|
||||
// mobileOffset keeps that clearance below Sonner's 600px breakpoint
|
||||
// (narrow/resized windows and the web client), which otherwise reverts
|
||||
// to Sonner's default 16px and lets toasts crowd the status bar again.
|
||||
offset={{ bottom: 'calc(2.5rem + env(safe-area-inset-bottom, 0px))' }}
|
||||
mobileOffset={{ bottom: 'calc(2.5rem + env(safe-area-inset-bottom, 0px))' }}
|
||||
className="toaster group"
|
||||
icons={{
|
||||
success: <CircleCheckIcon className="size-4" />,
|
||||
info: <InfoIcon className="size-4" />,
|
||||
warning: <TriangleAlertIcon className="size-4" />,
|
||||
error: <OctagonXIcon className="size-4" />,
|
||||
loading: <Loader2Icon className="size-4 animate-spin" />
|
||||
}}
|
||||
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}
|
||||
/>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -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<void> => {
|
||||
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()
|
||||
|
||||
Reference in New Issue
Block a user