diff --git a/frontend/src/components/Drawer/AppsDirectory.css b/frontend/src/components/Drawer/AppsDirectory.css index fbd0bbd04..cdbea8cb8 100644 --- a/frontend/src/components/Drawer/AppsDirectory.css +++ b/frontend/src/components/Drawer/AppsDirectory.css @@ -214,17 +214,6 @@ white-space: nowrap; } -.apps-directory__card-menu-anchor.drawer__more { - position: absolute; - top: 50%; - left: 50%; - width: 1px; - height: 1px; - padding: 0; - opacity: 0; - pointer-events: none; -} - .apps-directory__card--editing { align-items: center; min-height: 112px; diff --git a/frontend/src/components/Drawer/Drawer.css b/frontend/src/components/Drawer/Drawer.css index d2b37e5d4..f14342d02 100644 --- a/frontend/src/components/Drawer/Drawer.css +++ b/frontend/src/components/Drawer/Drawer.css @@ -345,9 +345,6 @@ the composer pill's focus halo so the platform reads as a single design system. */ .drawer__item:focus-visible, -.drawer__more:focus-visible, -.drawer__menu-item:focus-visible, -.drawer__menu-confirm-btn:focus-visible, .drawer__rename-input:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; @@ -405,12 +402,8 @@ visibility: hidden; } -/* ── Row + three-dots menu ─────────────────────────── */ +/* ── Context-menu row ──────────────────────────────── */ -/* The row stacks two siblings (button + menu trigger) horizontally so - * the trigger isn't nested inside the row button (nested + ) @@ -1541,193 +1607,40 @@ function DrawerItemMenu({ surface, pinned, menuOpen, + menuPlacement, + restoreFocusRef, actions, - triggerClassName = 'drawer__more', - triggerHidden = false, }) { const id = item.id const label = kind === 'chat' ? item.title : item.name - const slug = item.slug - const [confirmingDelete, setConfirmingDelete] = useState(false) - const [confirmingDeleteData, setConfirmingDeleteData] = useState(false) - - useEffect(() => { - if (!menuOpen) { - setConfirmingDelete(false) - setConfirmingDeleteData(false) - } - }, [menuOpen]) return ( - actions.toggleMenu(kind, id, true, surface)} - onClose={() => actions.toggleMenu(kind, id, false, surface)} - > - - {/* No Tooltip wrap here. Both Menu.Trigger and Tooltip - are Radix asChild wrappers that merge their props - onto the first child element. Nesting them breaks - the click-prop chain — Menu's onClick lands on the - Tooltip wrapper instead of the button, so tapping - ⋮ did nothing. The aria-label below covers screen - readers; visible tooltip discovery is a nice-to-have - we can revisit later with a different composition. */} - - - - {!confirmingDelete && !confirmingDeleteData ? ( - <> - actions.pin(kind, id, !pinned)} - className="drawer__menu-item--icon" - > - {pinned - ? - actions.startRename(kind, id, surface)}>Rename - {kind === 'app' && slug && ( - // Opens the in-PWA InstallSheet to set the home-screen - // name + icon first; the sheet saves, then navigates - // same-tab to `/apps//?install=1`. Same-tab keeps - // the user in the installed Möbius PWA context — no - // jarring browser-tab pop-out — and lets engagement - // from the parent shell count toward the per-origin - // Site Engagement score that gates beforeinstallprompt. - actions.install(item)}> - Install to home screen - - )} - {kind === 'app' && isDrawerAppShareEligible(item) && ( - actions.share(item)}> - Share app - - )} - {kind === 'chat' ? ( - // Chats soft-delete with 7-day recovery, so no - // confirm step — one tap deletes, the note below - // tells the user how to undo via the agent. - // Close the parent's menu state BEFORE onDelete fires: - // the row unmounts as soon as the refetch lands, but - // the parent's openMenu still references this row's - // id, leaving a Radix trigger looking "pressed" on - // whichever row slides up into the slot. - { - actions.toggleMenu(kind, id, false, surface) - actions.remove(kind, id) - }} - className="drawer__menu-item--danger" - > - Delete - - ) : ( - // Deleting an app is a reversible soft-delete (the agent - // can recover it for 7 days, like a chat), but we still - // want a confirm step. `preventDefault` on onSelect stops - // Radix from auto-closing the menu when the item is - // selected — we want the menu to stay open and swap - // to the confirm-chip below. - { e.preventDefault(); setConfirmingDelete(true) }} - className="drawer__menu-item--danger" - > - Delete - - )} - {kind === 'app' && ( - // Wipes the app's stored data but keeps it installed — a - // separate action from Delete (which removes the whole app). - // Same preventDefault-to-hold-open + confirm-chip pattern as - // the app Delete above; the wording stays exactly "Delete - // data" (no "keeps your data" phrasing). - { e.preventDefault(); setConfirmingDeleteData(true) }} - className="drawer__menu-item--danger" - > - Delete data - - )} - - ) : confirmingDeleteData ? ( -
- Delete data? -
- - -
-
- ) : ( -
- Confirm delete? -
- - -
-
- )} - {/* The 7-day recovery note applies to Delete (soft-delete), not - to the immediate, non-recoverable "Delete data" wipe — hide it - while that confirm chip is showing. */} - {!confirmingDeleteData && ( -

- The agent can recover deleted {kind === 'chat' ? 'chats' : 'apps'} for 7 days. -

- )} - - + + ) + : null} + pinned={pinned} + canInstall={kind === 'app' && Boolean(item.slug)} + canShare={kind === 'app' && isDrawerAppShareEligible(item)} + placement={menuPlacement} + restoreFocusRef={restoreFocusRef} + onClose={() => actions.toggleMenu(kind, id, false, surface)} + onPin={() => actions.pin(kind, id, !pinned)} + onRename={() => actions.startRename(kind, id, surface)} + onInstall={() => actions.install(item)} + onShare={() => actions.share(item)} + onDelete={() => actions.remove(kind, id)} + onDeleteData={() => actions.removeData(id)} + /> ) } diff --git a/frontend/src/components/Drawer/DrawerItemActionMenu.jsx b/frontend/src/components/Drawer/DrawerItemActionMenu.jsx new file mode 100644 index 000000000..b813ae5e9 --- /dev/null +++ b/frontend/src/components/Drawer/DrawerItemActionMenu.jsx @@ -0,0 +1,310 @@ +/* DrawerItemActionMenu gives app launcher cards and drawer rows one + pointer-accurate desktop menu and one named, thumb-friendly compact-screen + action sheet. */ + +import { useEffect, useLayoutEffect, useRef, useState } from 'react' +import { createPortal } from 'react-dom' +import { Chat, Pin, PinFilled, X } from '@openai/apps-sdk-ui/components/Icon' +import { placeContextMenu } from '../../lib/contextMenuGeometry.js' + +function focusableMenuItems(menu) { + return [...(menu?.querySelectorAll('[role="menuitem"]:not([disabled])') || [])] +} + +export default function DrawerItemActionMenu({ + open, + itemKind, + itemName, + icon, + pinned, + canInstall, + canShare, + placement, + restoreFocusRef, + onClose, + onPin, + onRename, + onInstall, + onShare, + onDelete, + onDeleteData, +}) { + const menuRef = useRef(null) + const wasOpenRef = useRef(false) + const restoreOnCloseRef = useRef(true) + const [confirmation, setConfirmation] = useState(null) + const [position, setPosition] = useState(null) + + function close({ restoreFocus = true } = {}) { + restoreOnCloseRef.current = restoreFocus + onClose() + } + + useEffect(() => { + if (open) { + wasOpenRef.current = true + return + } + setConfirmation(null) + setPosition(null) + if (!wasOpenRef.current) return + wasOpenRef.current = false + if (!restoreOnCloseRef.current) return + const frame = requestAnimationFrame(() => restoreFocusRef?.current?.focus()) + return () => cancelAnimationFrame(frame) + }, [open, restoreFocusRef]) + + useEffect(() => { + if (!open) return + function closeFromKeyboard(event) { + if (event.key !== 'Escape' && event.key !== 'Tab') return + event.preventDefault() + event.stopPropagation() + event.stopImmediatePropagation() + restoreOnCloseRef.current = true + onClose() + } + document.addEventListener('keydown', closeFromKeyboard, true) + return () => document.removeEventListener('keydown', closeFromKeyboard, true) + }, [open, onClose]) + + useLayoutEffect(() => { + if (!open || !menuRef.current) return + const root = document.documentElement + const rootRect = root.getBoundingClientRect() + const menuRect = menuRef.current + const placementX = Number(placement?.clientX) + const placementY = Number(placement?.clientY) + setPosition(placeContextMenu({ + clientPoint: { + x: Number.isFinite(placementX) + ? placementX + : rootRect.left + rootRect.width / 2, + y: Number.isFinite(placementY) + ? placementY + : rootRect.top + rootRect.height / 2, + }, + clientViewport: rootRect, + layoutViewport: { + width: root.offsetWidth || root.clientWidth || rootRect.width, + height: root.offsetHeight || root.clientHeight || rootRect.height, + }, + menuSize: { + width: menuRect.offsetWidth, + height: menuRect.offsetHeight, + }, + })) + }, [open, placement, confirmation]) + + useLayoutEffect(() => { + if (!open || !position || !menuRef.current) return + const frame = requestAnimationFrame(() => { + focusableMenuItems(menuRef.current)[0]?.focus() + }) + return () => cancelAnimationFrame(frame) + }, [open, position, confirmation]) + + if (!open) return null + + function run(action, { restoreFocus = true } = {}) { + close({ restoreFocus }) + action() + } + + function handleDeleteAction() { + if (itemKind === 'chat') { + run(onDelete, { restoreFocus: false }) + return + } + setConfirmation('delete') + } + + function onMenuKeyDown(event) { + if (!['ArrowDown', 'ArrowUp', 'Home', 'End'].includes(event.key)) return + const items = focusableMenuItems(menuRef.current) + if (!items.length) return + event.preventDefault() + const current = items.indexOf(document.activeElement) + const next = event.key === 'Home' + ? 0 + : event.key === 'End' + ? items.length - 1 + : event.key === 'ArrowDown' + ? (current + 1 + items.length) % items.length + : (current - 1 + items.length) % items.length + items[next].focus() + } + + const kindLabel = itemKind === 'chat' ? 'Chat' : 'App' + const recoveryLabel = itemKind === 'chat' ? 'chats' : 'apps' + const identityIcon = itemKind === 'chat' + ? ( + + ) + : icon + + const layer = ( +
{ + if (event.target === event.currentTarget) close() + }} + onContextMenu={event => event.preventDefault()} + onWheel={event => { + if (event.target === event.currentTarget) close() + }} + > +
event.stopPropagation()} + onKeyDown={onMenuKeyDown} + > + + ) + + return createPortal(layer, document.body) +} diff --git a/frontend/src/components/Shell/__tests__/workspaceUi.test.js b/frontend/src/components/Shell/__tests__/workspaceUi.test.js index 39cb4520f..2ad714dc8 100644 --- a/frontend/src/components/Shell/__tests__/workspaceUi.test.js +++ b/frontend/src/components/Shell/__tests__/workspaceUi.test.js @@ -11,6 +11,10 @@ const shellBrand = readFileSync(new URL('../ShellBrand.jsx', import.meta.url), ' const newChatLanding = readFileSync(new URL('../NewChatLanding.jsx', import.meta.url), 'utf8') const workspaceViewSrc = readFileSync(new URL('../workspaceView.js', import.meta.url), 'utf8') const drawer = readFileSync(new URL('../../Drawer/Drawer.jsx', import.meta.url), 'utf8') +const drawerItemActionMenu = readFileSync( + new URL('../../Drawer/DrawerItemActionMenu.jsx', import.meta.url), + 'utf8', +) const paneModelSrc = readFileSync(new URL('../paneModel.js', import.meta.url), 'utf8') const chrome = readFileSync(new URL('../WorkspaceChrome.jsx', import.meta.url), 'utf8') const dragBinding = readFileSync(new URL('../useWorkspaceDrag.js', import.meta.url), 'utf8') @@ -837,9 +841,38 @@ test('large drawer lists memoize ordering and row actions without changing row o assert.doesNotMatch(drawer, /onSelect=\{\(\) => on(?:Chat|App)/) }) +test('row-owned context menus keep a visible trigger without hidden anchors', () => { + assert.match(drawer, / { + assert.match( + drawerItemActionMenu, + /function handleDeleteAction\(\)[\s\S]*?itemKind === 'chat'[\s\S]*?run\(onDelete, \{ restoreFocus: false \}\)[\s\S]*?return[\s\S]*?setConfirmation\('delete'\)/, + ) + assert.match( + drawerItemActionMenu, + /className="drawer__item-action-item drawer__item-action-item--danger"\s*\n\s*onClick=\{handleDeleteAction\}/, + ) + assert.match( + drawerItemActionMenu, + /confirmation === 'delete-data'[\s\S]*?confirmation === 'delete'/, + 'app and app-data deletion must retain their confirmation paths', + ) +}) + test('drawer row menus use one semantic context-menu path across pointer types', () => { - assert.match(drawer, /function openRowMenu\(event\)[\s\S]*?actions\.toggleMenu\(kind, id, true, surface\)/) - assert.match(drawer, /onContextMenu=\{openRowMenu\}/) + assert.match(drawer, /function openItemMenu\(event\)[\s\S]*?actions\.toggleMenu\(kind, id, true, surface,/) + assert.equal((drawer.match(/onContextMenu=\{openItemMenu\}/g) || []).length, 2, + 'app cards, app rows, and chat rows must share one opening function') assert.match(dragBinding, /srcEl\.dispatchEvent\(new window\.MouseEvent\('contextmenu'/) assert.doesNotMatch( dragBinding, @@ -853,12 +886,19 @@ test('a secondary-button release cannot immediately select a flipped drawer menu assert.match(drawer, /event\.pointerType !== 'mouse' \|\| event\.button !== 2/) assert.match(drawer, /window\.addEventListener\('pointerup', onSecondaryPointerUp, true\)/) assert.match(drawer, /upEvent\.pointerId !== pointerId \|\| upEvent\.button !== 2/) - assert.match(drawer, /cleanup\(\)[\s\S]*?actions\.toggleMenu\(kind, id, true, surface\)/) + assert.match(drawer, /cleanup\(\)[\s\S]*?actions\.toggleMenu\(kind, id, true, surface, placement\)/) assert.match(drawer, /timer = setTimeout\(cleanup, 1500\)/) }) +test('launcher cards and drawer rows share the same long-press threshold and movement slop', () => { + assert.match(drawer, /import \{ DRAWER_HOLD_MS, PRE_HOLD_MOVE_PX \} from '\.\.\/Shell\/dragController\.js'/) + assert.match(drawer, /\}, DRAWER_HOLD_MS\)/) + assert.match(drawer, /> PRE_HOLD_MOVE_PX/) + assert.doesNotMatch(drawer, /520/) +}) + test('double-click edits a drawer row name instead of duplicating its context menu', () => { - assert.match(drawer, /onDoubleClick=\{event => \{[\s\S]*?actions\.startRename\(kind, id\)/) + assert.match(drawer, /onDoubleClick=\{event => \{[\s\S]*?actions\.startRename\(kind, id, surface\)/) }) test('the Settings surface responds to PANE width via a query container', () => { diff --git a/frontend/src/lib/__tests__/appsDirectoryContract.test.js b/frontend/src/lib/__tests__/appsDirectoryContract.test.js index 31afa0f42..7a259e964 100644 --- a/frontend/src/lib/__tests__/appsDirectoryContract.test.js +++ b/frontend/src/lib/__tests__/appsDirectoryContract.test.js @@ -9,7 +9,13 @@ const src = resolve(here, '../..') const drawer = readFileSync(resolve(src, 'components/Drawer/Drawer.jsx'), 'utf8') const directory = readFileSync(resolve(src, 'components/Drawer/AppsDirectory.jsx'), 'utf8') const css = readFileSync(resolve(src, 'components/Drawer/AppsDirectory.css'), 'utf8') +const drawerCss = readFileSync(resolve(src, 'components/Drawer/Drawer.css'), 'utf8') +const itemActionMenu = readFileSync( + resolve(src, 'components/Drawer/DrawerItemActionMenu.jsx'), + 'utf8', +) const shell = readFileSync(resolve(src, 'components/Shell/Shell.jsx'), 'utf8') +const shellCss = readFileSync(resolve(src, 'components/Shell/Shell.css'), 'utf8') const tabModel = readFileSync(resolve(src, 'components/Shell/tabModel.js'), 'utf8') const navigationIcons = readFileSync(resolve(src, 'components/navigationIcons.js'), 'utf8') @@ -23,8 +29,8 @@ test('Apps is a single drawer destination and the old full app list is gone', () test('the directory preserves app management on every card', () => { assert.match(drawer, /variant="card"/) assert.match(drawer, / { assert.match(tabModel, /APPS_TAB_KEY = 'apps:apps'/) assert.match(shell, /const APPS_KEY = tabModel\.APPS_TAB_KEY/) assert.match(css, /@media \(max-width: 720px\)[\s\S]*?grid-template-columns: repeat\(4/) - assert.match(drawer, /onContextMenu=\{openCardMenu\}/) - assert.match(drawer, /setTimeout\(\(\) => \{[\s\S]*?toggleMenu[\s\S]*?520\)/) - assert.match( - drawer, - /triggerClassName="drawer__more apps-directory__card-menu-anchor"\s+triggerHidden/, - 'the invisible menu anchor must not add a ghost keyboard focus stop', - ) + assert.equal((drawer.match(/onContextMenu=\{openItemMenu\}/g) || []).length, 2, + 'launcher cards and drawer rows must enter the same context-menu path') + assert.match(drawer, /DRAWER_HOLD_MS, PRE_HOLD_MOVE_PX/) + assert.match(drawer, /setTimeout\(\(\) => \{[\s\S]*?toggleMenu[\s\S]*?DRAWER_HOLD_MS\)/) + assert.doesNotMatch(drawer, /520/) + assert.match(drawer, /menuPlacement=\{openMenu/) + assert.match(itemActionMenu, /placeContextMenu/) + assert.match(itemActionMenu, /stopImmediatePropagation/) + assert.match(drawerCss, /@media \(max-width: 720px\)[\s\S]*?drawer__item-action-menu/) + assert.match(drawerCss, /bottom: max\(12px, env\(safe-area-inset-bottom\)\)/) assert.match(shell, /const navigationSurfaceOpen = modalDrawerOpen/) }) +test('chat and app rows share one placed action menu contract', () => { + assert.match(drawer, / { + const desktop = shellCss.match(/@media \(min-width: 1024px\) \{[\s\S]*$/)?.[0] || '' + assert.doesNotMatch(desktop, /(?:^|[;{])\s*zoom\s*:/, + 'desktop density must not scale the viewport away from pointer coordinates') +}) + test('the app directory distinguishes loading, errors, and confirmed emptiness', () => { assert.match(drawer, /status=\{appsStatus\}/) assert.match(directory, /status === 'loading'/) diff --git a/frontend/src/lib/__tests__/contextMenuGeometry.test.js b/frontend/src/lib/__tests__/contextMenuGeometry.test.js new file mode 100644 index 000000000..05f563152 --- /dev/null +++ b/frontend/src/lib/__tests__/contextMenuGeometry.test.js @@ -0,0 +1,37 @@ +import test from 'node:test' +import assert from 'node:assert/strict' +import { placeContextMenu } from '../contextMenuGeometry.js' + +test('a desktop context menu stays beside the painted pointer through root zoom', () => { + const position = placeContextMenu({ + clientPoint: { x: 380, y: 215 }, + clientViewport: { left: 0, top: 0, width: 1512, height: 861 }, + layoutViewport: { width: 1680, height: 956.6667 }, + menuSize: { width: 220, height: 190 }, + }) + + assert.ok(Math.abs(position.x * 0.9 - 388) < 0.1) + assert.ok(Math.abs(position.y * 0.9 - 223) < 0.1) +}) + +test('a context menu flips before the right and bottom viewport edges', () => { + const position = placeContextMenu({ + clientPoint: { x: 790, y: 590 }, + clientViewport: { left: 0, top: 0, width: 800, height: 600 }, + layoutViewport: { width: 800, height: 600 }, + menuSize: { width: 220, height: 180 }, + }) + + assert.deepEqual(position, { x: 562, y: 402 }) +}) + +test('an oversized context menu clamps to the viewport padding', () => { + const position = placeContextMenu({ + clientPoint: { x: 10, y: 10 }, + clientViewport: { left: 0, top: 0, width: 200, height: 150 }, + layoutViewport: { width: 200, height: 150 }, + menuSize: { width: 240, height: 180 }, + }) + + assert.deepEqual(position, { x: 12, y: 12 }) +}) diff --git a/frontend/src/lib/contextMenuGeometry.js b/frontend/src/lib/contextMenuGeometry.js new file mode 100644 index 000000000..49d71842f --- /dev/null +++ b/frontend/src/lib/contextMenuGeometry.js @@ -0,0 +1,50 @@ +/* Context-menu geometry bridges painted pointer coordinates to the root + layout space, then keeps the menu beside the pointer and inside the viewport. */ + +function positiveNumber(value, fallback) { + const number = Number(value) + return Number.isFinite(number) && number > 0 ? number : fallback +} + +function clamp(value, minimum, maximum) { + return Math.min(Math.max(value, minimum), Math.max(minimum, maximum)) +} + +export function placeContextMenu({ + clientPoint, + clientViewport, + layoutViewport, + menuSize, + gap = 8, + padding = 12, +}) { + const clientWidth = positiveNumber(clientViewport?.width, 1) + const clientHeight = positiveNumber(clientViewport?.height, 1) + const layoutWidth = positiveNumber(layoutViewport?.width, clientWidth) + const layoutHeight = positiveNumber(layoutViewport?.height, clientHeight) + const scaleX = layoutWidth / clientWidth + const scaleY = layoutHeight / clientHeight + const pointX = (Number(clientPoint?.x) - (Number(clientViewport?.left) || 0)) * scaleX + const pointY = (Number(clientPoint?.y) - (Number(clientViewport?.top) || 0)) * scaleY + const menuWidth = positiveNumber(menuSize?.width, 0) + const menuHeight = positiveNumber(menuSize?.height, 0) + const gapX = Math.max(0, Number(gap) || 0) * scaleX + const gapY = Math.max(0, Number(gap) || 0) * scaleY + const paddingX = Math.max(0, Number(padding) || 0) * scaleX + const paddingY = Math.max(0, Number(padding) || 0) * scaleY + + let x = pointX + gapX + if (x + menuWidth > layoutWidth - paddingX) { + x = pointX - menuWidth - gapX + } + + let y = pointY + gapY + if (y + menuHeight > layoutHeight - paddingY) { + y = pointY - menuHeight - gapY + } + + return { + x: clamp(x, paddingX, layoutWidth - menuWidth - paddingX), + y: clamp(y, paddingY, layoutHeight - menuHeight - paddingY), + } +}