fix: keep menus and notifications usable above modals
Cross-platform packages / Validate source (push) Canceled after 0s
Cross-platform packages / Authorize desktop packaging (push) Canceled after 0s
Cross-platform packages / linux arm64 (push) Canceled after 0s
Cross-platform packages / macos arm64 (push) Canceled after 0s
Cross-platform packages / windows arm64 (push) Canceled after 0s
Cross-platform packages / linux x64 (push) Canceled after 0s
Cross-platform packages / macos x64 (push) Canceled after 0s
Cross-platform packages / windows x64 (push) Canceled after 0s
Cross-platform packages / Publish GitHub and OSS release (push) Canceled after 0s
GoodBuddy Agents / Validate Agent source (push) Canceled after 0s
GoodBuddy Agents / Build darwin arm64 Agent (push) Canceled after 0s
GoodBuddy Agents / Build linux arm64 Agent (push) Canceled after 0s
GoodBuddy Agents / Build linux x64 Agent (push) Canceled after 0s
Cross-platform packages / Validate source (push) Canceled after 0s
Cross-platform packages / Authorize desktop packaging (push) Canceled after 0s
Cross-platform packages / linux arm64 (push) Canceled after 0s
Cross-platform packages / macos arm64 (push) Canceled after 0s
Cross-platform packages / windows arm64 (push) Canceled after 0s
Cross-platform packages / linux x64 (push) Canceled after 0s
Cross-platform packages / macos x64 (push) Canceled after 0s
Cross-platform packages / windows x64 (push) Canceled after 0s
Cross-platform packages / Publish GitHub and OSS release (push) Canceled after 0s
GoodBuddy Agents / Validate Agent source (push) Canceled after 0s
GoodBuddy Agents / Build darwin arm64 Agent (push) Canceled after 0s
GoodBuddy Agents / Build linux arm64 Agent (push) Canceled after 0s
GoodBuddy Agents / Build linux x64 Agent (push) Canceled after 0s
Parsing test menus and application notifications could appear behind modal overlays or become unreachable while the background was inert. Share a stable top-layer portal that belongs to the owning or active modal, preserving notification timers, keyboard navigation and focus restoration. Add unit and Electron regression coverage for settings, nested parsing results, application center, inference monitoring and native dialogs, with matching UI guidance and validation evidence. Validation recorded in the preceding implementation: 4,793 tests passed and 67 skipped; typecheck, lint, isolated production build and Electron UI checks passed. Staged diff whitespace checks passed before this commit. Release note: Menus and notifications remain visible and usable while settings or other dialogs are open, including keyboard navigation and notification dismissal.
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
import { useEffect, useEffectEvent, useLayoutEffect, useRef, type ReactNode, type RefObject } from 'react'
|
||||
import { createPortal, flushSync } from 'react-dom'
|
||||
import { flushSync } from 'react-dom'
|
||||
import { FloatingPortal } from './FloatingPortal'
|
||||
import './anchored-menu.css'
|
||||
|
||||
export function AnchoredMenu({ anchorRef, id, label, onClose, children }: {
|
||||
@@ -87,6 +88,6 @@ export function AnchoredMenu({ anchorRef, id, label, onClose, children }: {
|
||||
if (document.activeElement === document.body || menu.contains(document.activeElement)) anchor?.focus()
|
||||
}
|
||||
}, [anchorRef])
|
||||
return createPortal(<div ref={menuRef} id={id} role="menu" aria-label={label}
|
||||
tabIndex={-1} className="anchored-menu">{children}</div>, document.body)
|
||||
return <FloatingPortal anchorRef={anchorRef}><div ref={menuRef} id={id} role="menu" aria-label={label}
|
||||
tabIndex={-1} className="anchored-menu">{children}</div></FloatingPortal>
|
||||
}
|
||||
|
||||
@@ -252,6 +252,7 @@ import {
|
||||
type KeepAliveCacheEntry,
|
||||
} from "./keep-alive-cache";
|
||||
import { activateModalFocus, trapTabFocus } from "./dialog-focus";
|
||||
import { FloatingPortal } from "./FloatingPortal";
|
||||
import {
|
||||
displayErrorMessage,
|
||||
displayNetworkAwareErrorMessage,
|
||||
@@ -486,7 +487,7 @@ function AppNotificationItem({
|
||||
);
|
||||
}
|
||||
|
||||
function AppNotificationViewport({
|
||||
export function AppNotificationViewport({
|
||||
notifications,
|
||||
dispatch,
|
||||
}: {
|
||||
@@ -499,7 +500,7 @@ function AppNotificationViewport({
|
||||
return null;
|
||||
}
|
||||
return (
|
||||
<section
|
||||
<FloatingPortal><section
|
||||
aria-label={t("notifications.viewport")}
|
||||
className="app-notification-viewport"
|
||||
>
|
||||
@@ -510,7 +511,7 @@ function AppNotificationViewport({
|
||||
notification={notification}
|
||||
/>
|
||||
))}
|
||||
</section>
|
||||
</section></FloatingPortal>
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
import { useEffect, useRef, useState } from 'react'
|
||||
import { createPortal } from 'react-dom'
|
||||
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
|
||||
import { afterEach, expect, it } from 'vitest'
|
||||
import { FloatingPortal } from './FloatingPortal'
|
||||
import { activateModalFocus, trapTabFocus } from './dialog-focus'
|
||||
|
||||
afterEach(cleanup)
|
||||
|
||||
function Modal({ label, onClose }: { label: string; onClose: () => void }): React.JSX.Element {
|
||||
const close = useRef<HTMLButtonElement>(null)
|
||||
useEffect(() => activateModalFocus(() => close.current), [])
|
||||
return createPortal(<section role="dialog" aria-modal="true" aria-label={label}
|
||||
onKeyDown={event => trapTabFocus(event, event.currentTarget)}>
|
||||
<button ref={close} onClick={onClose}>Close {label}</button>
|
||||
</section>, document.body)
|
||||
}
|
||||
|
||||
it('moves one live notification host into successive modals without remounting its content', async () => {
|
||||
let mounts = 0
|
||||
function Notice(): React.JSX.Element {
|
||||
useEffect(() => { mounts++ }, [])
|
||||
return <div role="status">Saved<button>Dismiss</button></div>
|
||||
}
|
||||
function Fixture(): React.JSX.Element {
|
||||
const [modal, setModal] = useState(false)
|
||||
return <div className="app-shell"><button onClick={() => setModal(true)}>Open</button>
|
||||
<FloatingPortal><Notice /></FloatingPortal>
|
||||
{modal && <Modal label="Settings" onClose={() => setModal(false)} />}
|
||||
</div>
|
||||
}
|
||||
render(<Fixture />)
|
||||
const host = screen.getByRole('status').parentElement!
|
||||
expect(host.parentElement).toBe(document.body)
|
||||
fireEvent.click(screen.getByText('Open'))
|
||||
await waitFor(() => expect(host.parentElement).toBe(screen.getByRole('dialog')))
|
||||
expect(host).not.toHaveAttribute('inert')
|
||||
expect(document.querySelector<HTMLElement>('.app-shell')!.inert).toBe(true)
|
||||
expect(mounts).toBe(1)
|
||||
screen.getByText('Dismiss').focus()
|
||||
fireEvent.keyDown(screen.getByText('Dismiss'), { key: 'Tab' })
|
||||
expect(screen.getByText('Close Settings')).toHaveFocus()
|
||||
fireEvent.click(screen.getByText('Close Settings'))
|
||||
await waitFor(() => expect(host.parentElement).toBe(document.body))
|
||||
expect(mounts).toBe(1)
|
||||
expect(document.querySelector<HTMLElement>('.app-shell')!.inert).toBe(false)
|
||||
})
|
||||
|
||||
it('keeps an anchored floating menu inside its owning modal', async () => {
|
||||
function Fixture(): React.JSX.Element {
|
||||
const anchor = useRef<HTMLButtonElement>(null)
|
||||
const [open, setOpen] = useState(false)
|
||||
return <section role="dialog" aria-modal="true" aria-label="Settings">
|
||||
<button ref={anchor} onClick={() => setOpen(true)}>Test</button>
|
||||
{open && <FloatingPortal anchorRef={anchor}><div role="menu">Scenarios</div></FloatingPortal>}
|
||||
</section>
|
||||
}
|
||||
render(<Fixture />)
|
||||
fireEvent.click(screen.getByText('Test'))
|
||||
await waitFor(() => expect(screen.getByRole('dialog')).toContainElement(screen.getByRole('menu')))
|
||||
expect(screen.getByRole('menu').parentElement).toHaveProperty('popover', 'manual')
|
||||
})
|
||||
@@ -0,0 +1,77 @@
|
||||
import { useLayoutEffect, useState, type ReactNode, type RefObject } from 'react'
|
||||
import { createPortal } from 'react-dom'
|
||||
import { trapTabFocus } from './dialog-focus'
|
||||
import './floating-portal.css'
|
||||
|
||||
// Top-layer painting does not escape modal inertness: the host must also belong
|
||||
// to the active dialog. Keep the host stable so moving it preserves toast timers.
|
||||
export function FloatingPortal({ children, anchorRef }: {
|
||||
children: ReactNode
|
||||
anchorRef?: RefObject<HTMLElement | null>
|
||||
}): React.JSX.Element {
|
||||
const [host] = useState(() => {
|
||||
const element = document.createElement('div')
|
||||
element.className = 'floating-portal'
|
||||
element.popover = 'manual'
|
||||
return element
|
||||
})
|
||||
useLayoutEffect(() => {
|
||||
let focused: HTMLElement | null = null
|
||||
let returnFocus: HTMLElement | null = null
|
||||
const sync = (): void => {
|
||||
const modals = Array.from(document.querySelectorAll<HTMLElement>('[aria-modal="true"], dialog:modal'))
|
||||
const owner = anchorRef?.current?.closest<HTMLElement>('[aria-modal="true"], dialog:modal') ??
|
||||
(anchorRef ? null : modals.reverse().find(element =>
|
||||
!element.closest('[inert], [hidden], [aria-hidden="true"]') && getComputedStyle(element).display !== 'none'))
|
||||
const target = owner ?? document.body
|
||||
const active = host.contains(document.activeElement) ? document.activeElement as HTMLElement : null
|
||||
if (host.parentElement !== target) {
|
||||
target.append(host)
|
||||
host.removeAttribute('inert')
|
||||
}
|
||||
if (!host.matches(':popover-open')) host.showPopover()
|
||||
if (active) active.focus({ preventScroll: true })
|
||||
if (focused && !focused.isConnected && document.activeElement === document.body) {
|
||||
restoreFocus()
|
||||
}
|
||||
}
|
||||
const restoreFocus = (): void => {
|
||||
const modal = host.closest<HTMLElement>('[aria-modal="true"], dialog:modal')
|
||||
const target = returnFocus?.isConnected && !returnFocus.closest('[inert], [hidden], :disabled') &&
|
||||
(!modal || modal.contains(returnFocus)) ? returnFocus : modal?.querySelector<HTMLElement>('button:not(:disabled), [tabindex="0"]')
|
||||
target?.focus({ preventScroll: true })
|
||||
focused = null
|
||||
}
|
||||
const onFocus = (event: FocusEvent): void => {
|
||||
focused = event.target instanceof HTMLElement ? event.target : null
|
||||
if (event.relatedTarget instanceof HTMLElement && !host.contains(event.relatedTarget)) {
|
||||
returnFocus = event.relatedTarget
|
||||
}
|
||||
}
|
||||
const onKeyDown = (event: KeyboardEvent): void => {
|
||||
if (event.defaultPrevented || anchorRef) return
|
||||
const modal = host.closest<HTMLElement>('[aria-modal="true"], dialog:modal')
|
||||
if (event.key === 'Escape') {
|
||||
event.preventDefault()
|
||||
event.stopPropagation()
|
||||
restoreFocus()
|
||||
} else {
|
||||
trapTabFocus(event, modal)
|
||||
}
|
||||
}
|
||||
sync()
|
||||
const observer = new MutationObserver(sync)
|
||||
observer.observe(document.body, { childList: true, subtree: true, attributes: true,
|
||||
attributeFilter: ['inert', 'hidden', 'aria-hidden', 'aria-modal', 'open'] })
|
||||
host.addEventListener('focusin', onFocus)
|
||||
host.addEventListener('keydown', onKeyDown)
|
||||
return () => {
|
||||
observer.disconnect()
|
||||
host.removeEventListener('focusin', onFocus)
|
||||
host.removeEventListener('keydown', onKeyDown)
|
||||
if (host.contains(document.activeElement)) restoreFocus()
|
||||
host.remove()
|
||||
}
|
||||
}, [anchorRef, host])
|
||||
return createPortal(children, host)
|
||||
}
|
||||
@@ -1,6 +1,5 @@
|
||||
.anchored-menu {
|
||||
position: fixed;
|
||||
z-index: calc(var(--z-dialog) - 1);
|
||||
box-sizing: border-box;
|
||||
overflow-y: auto;
|
||||
padding: var(--space-2);
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
.floating-portal {
|
||||
position: fixed;
|
||||
inset: 0;
|
||||
width: auto;
|
||||
height: auto;
|
||||
margin: 0;
|
||||
padding: 0;
|
||||
border: 0;
|
||||
overflow: visible;
|
||||
background: transparent;
|
||||
color: inherit;
|
||||
pointer-events: none;
|
||||
}
|
||||
|
||||
.floating-portal::backdrop {
|
||||
background: transparent;
|
||||
pointer-events: none;
|
||||
}
|
||||
|
||||
.floating-portal > * {
|
||||
pointer-events: auto;
|
||||
-webkit-app-region: no-drag;
|
||||
}
|
||||
@@ -12398,7 +12398,6 @@ details.settings-section > :not(summary) + :not(summary) {
|
||||
|
||||
.app-notification-viewport {
|
||||
position: fixed;
|
||||
z-index: 75;
|
||||
top: 54px;
|
||||
right: var(--space-4);
|
||||
display: grid;
|
||||
|
||||
@@ -7,6 +7,11 @@ if (typeof Element !== 'undefined') {
|
||||
Element.prototype.scrollTo = vi.fn()
|
||||
}
|
||||
|
||||
// jsdom does not implement top-layer rendering; Electron tests cover it.
|
||||
if (typeof HTMLElement !== 'undefined') {
|
||||
HTMLElement.prototype.showPopover = vi.fn()
|
||||
}
|
||||
|
||||
if (typeof ResizeObserver === 'undefined') {
|
||||
globalThis.ResizeObserver = class {
|
||||
observe = vi.fn()
|
||||
|
||||
Reference in New Issue
Block a user