From 7954a4b62af9009b23c0d6da245db6d455ee0f0b Mon Sep 17 00:00:00 2001 From: Carl Date: Wed, 23 Sep 2026 23:08:56 -0700 Subject: [PATCH 1/4] feat(channels): move session creation into the context menu Signed-off-by: Carl --- docs/channels.md | 16 ++ docs/sessions/README.md | 6 +- .../channels/ChannelSidebarItem.test.tsx | 61 ++++++- src/bundled/channels/ChannelSidebarItem.tsx | 55 ++++-- .../channels/ChannelSidebarRow.module.css | 18 -- .../channels/ChannelSidebarRow.test.tsx | 60 +------ src/bundled/channels/ChannelSidebarRow.tsx | 66 +------ src/bundled/channels/ChannelsPage.tsx | 90 +++++++++- .../channels/useChannelRowMenu.test.tsx | 122 +++++++++++++ src/bundled/channels/useChannelRowMenu.ts | 37 ++++ src/shared/design-system/DESIGN.md | 10 ++ .../design-system/styles/components.css | 29 +++ .../browser/navigation-session-menu.spec.mjs | 166 ++++++++++++++++++ tests/browser/navigation-sidebar.spec.mjs | 39 ++-- tests/browser/thread-unread.spec.mjs | 15 +- 15 files changed, 605 insertions(+), 185 deletions(-) create mode 100644 src/bundled/channels/useChannelRowMenu.test.tsx create mode 100644 src/bundled/channels/useChannelRowMenu.ts create mode 100644 tests/browser/navigation-session-menu.spec.mjs diff --git a/docs/channels.md b/docs/channels.md index 5d535738c..04516c167 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -80,6 +80,22 @@ palette; legacy sidebar filters are ignored. The saved-groups browser regression records every visible return frame and holds the redundant decode path, so eventual restoration cannot conceal a fallback-group/scroll jump. +Channel row actions share one page-owned `ContextMenuRoot` / `MenuPopup`, labelled +`Actions for `. **New session** comes first; additional sidebar actions +should extend that popup, with a separator only when another action group follows. +`ChannelSidebarItem` owns the context trigger inside its memo boundary, using stable +`onOpenMenu` props. It wraps the activity select surface rather than merging popup +props onto the activity button; session disclosure and child rows stay outside. +The popup and trigger are enabled only when `rowActions` supplies actual items; +each action owns its eligibility, so Sessions availability never gates sibling +actions. `useChannelRowMenu` owns channel id, the full rendered section key +(`starred`, `channels`, `group:`, etc.), and the keyboard anchor. It clears +that state if the row leaves that section or loses its last action; moving back +or restoring eligibility does not reopen the menu. For future group commands, +derive the saved group id separately from `group:` rather than conflating it +with rendered placement. Right-clicking the separate session disclosure remains +outside the parent menu trigger, as do child-session rows. + ## Starting a direct message The **+** action in the DMs sidebar header opens **New message**, a routed empty diff --git a/docs/sessions/README.md b/docs/sessions/README.md index d3a91708e..a7a2cefd4 100644 --- a/docs/sessions/README.md +++ b/docs/sessions/README.md @@ -20,7 +20,11 @@ Sessions are focused work conversations built on ordinary private channels. through normal channel invitations. Sending waits for both real rosters. Failed invitations preserve the draft and retry the saved operation. - New sessions can be created without a parent or beneath a Messages channel. - A channel's hover menu starts a child; its hover chevron collapses the children. + Right-click a channel and choose **New session** to start a child, or focus the + channel and press Shift+F10 / the Menu key. The hover chevron collapses children. + The action is unavailable for DMs, archived channels, sessions, or when Sessions + is disabled. Dismissing the menu restores row focus; starting a session focuses + the draft composer. Changing a saved session's parent remains future app metadata work. - Both entry points share the ordinary composer, centered at the bottom, and channel-style titles. The avatar-and-name picker sits before @ and opens upward. diff --git a/src/bundled/channels/ChannelSidebarItem.test.tsx b/src/bundled/channels/ChannelSidebarItem.test.tsx index c587e2d68..228975ffe 100644 --- a/src/bundled/channels/ChannelSidebarItem.test.tsx +++ b/src/bundled/channels/ChannelSidebarItem.test.tsx @@ -12,6 +12,7 @@ import { afterEach, expect, it, vi } from "vitest"; import type { RelaySession } from "../../features/relay/session"; import type { UnreadSnapshot } from "../../features/relay/unread"; import { ChannelSidebarItem } from "./ChannelSidebarItem"; +import { ContextMenuRoot } from "../../shared/design-system/ui/Menu"; afterEach(cleanup); @@ -63,7 +64,6 @@ it("keeps live unread updates and uses replacement session callbacks across row channel: { id: "alpha", name: "Alpha", channelType: "stream" as const }, session: first.session, working: false, - sessionsEnabled: true, selected: undefined, collapsed: false, onToggle: vi.fn(), @@ -139,7 +139,6 @@ it("renders one-to-one DM avatars from supplied profiles only, through the media }, session, working: false, - sessionsEnabled: false, selected: undefined, collapsed: false, onToggle: vi.fn(), @@ -211,3 +210,61 @@ it("renders one-to-one DM avatars from supplied profiles only, through the media } else expect(row.querySelector("svg")).toBeInTheDocument(); } }); + +it.each(["ContextMenu", "F10"])( + "opens the parent menu with %s without involving disclosure or child sessions", + async (key) => { + const onSelect = vi.fn(); + const onToggle = vi.fn(); + const onOpenMenu = vi.fn(); + const channel = { + id: "alpha", + name: "Alpha", + channelType: "stream" as const, + }; + render( + + + , + ); + const parent = screen.getByRole("button", { name: "Alpha" }); + fireEvent.keyDown(parent, { key, shiftKey: key === "F10" }); + expect(onOpenMenu).toHaveBeenCalledWith( + channel, + "group:work", + parent.parentElement, + ); + expect(onSelect).not.toHaveBeenCalled(); + const disclosure = screen.getByRole("button", { + name: "Collapse sessions in Alpha", + }); + const child = screen.getByRole("button", { + name: "Plan, session in Alpha", + }); + fireEvent.keyDown(disclosure, { key, shiftKey: key === "F10" }); + fireEvent.keyDown(child, { key, shiftKey: key === "F10" }); + expect(onOpenMenu).toHaveBeenCalledTimes(1); + const user = userEvent.setup(); + await user.click(disclosure); + expect(onToggle).toHaveBeenCalledWith("session-children:alpha", false); + expect(onSelect).not.toHaveBeenCalled(); + await user.click(child); + expect(onSelect).toHaveBeenCalledWith("child"); + }, +); diff --git a/src/bundled/channels/ChannelSidebarItem.tsx b/src/bundled/channels/ChannelSidebarItem.tsx index 633c79821..f918d4ac9 100644 --- a/src/bundled/channels/ChannelSidebarItem.tsx +++ b/src/bundled/channels/ChannelSidebarItem.tsx @@ -1,5 +1,6 @@ import { memo } from "react"; import { Avatar } from "../../shared/design-system/ui/Avatar"; +import { ContextMenuTrigger } from "../../shared/design-system/ui/Menu"; import type { ChannelSummary, Profile } from "../../features/relay/contracts"; import type { RelaySession } from "../../features/relay/session"; import { ChatCircleIcon } from "../../shared/design-system/icons/index"; @@ -18,7 +19,6 @@ export const ChannelSidebarItem = memo(function ChannelSidebarItem({ profile, session, working, - sessionsEnabled, selected, collapsed, onToggle, @@ -29,12 +29,14 @@ export const ChannelSidebarItem = memo(function ChannelSidebarItem({ onNewSession, onOpenThread, onHideDm, + menuEnabled, + sectionKey, + onOpenMenu, }: { channel: ChannelSummary; profile?: Profile | undefined; session: RelaySession; working: boolean; - sessionsEnabled: boolean; selected: string | undefined; collapsed: boolean; onToggle: (key: string, open: boolean) => void; @@ -45,6 +47,13 @@ export const ChannelSidebarItem = memo(function ChannelSidebarItem({ onNewSession: (id: string) => void; onOpenThread: (channelId: string, rootId: string) => void; onHideDm?: (id: string) => void; + menuEnabled?: boolean; + sectionKey?: string | undefined; + onOpenMenu?: ( + channel: ChannelSummary, + sectionKey: string, + anchor?: HTMLElement, + ) => void; }) { const Icon = channel.channelType === "dm" ? ChatCircleIcon : channelIcon(channel); @@ -94,17 +103,39 @@ export const ChannelSidebarItem = memo(function ChannelSidebarItem({ /> } - wrapSelect={(trigger) => ( - onOpenThread(item.channelId, item.rootId)} - trigger={trigger} - /> - )} + wrapSelect={(trigger) => { + const activity = ( + onOpenThread(item.channelId, item.rootId)} + trigger={trigger} + /> + ); + // Keep popup semantics on separate DOM nodes: activity owns the button, + // the context menu wraps only its select surface, not the child sessions. + return menuEnabled ? ( + } + onKeyDown={(event) => { + if ( + event.key === "ContextMenu" || + (event.shiftKey && event.key === "F10") + ) { + event.preventDefault(); + if (sectionKey) + onOpenMenu?.(channel, sectionKey, event.currentTarget); + } + }} + > + {activity} + + ) : ( + activity + ); + }} selected={selected} - sessionsEnabled={sessionsEnabled} collapsed={collapsed} onToggle={(open) => onToggle(`session-children:${channel.id}`, open)} draft={draft} diff --git a/src/bundled/channels/ChannelSidebarRow.module.css b/src/bundled/channels/ChannelSidebarRow.module.css index 007660ffd..46b9a16e5 100644 --- a/src/bundled/channels/ChannelSidebarRow.module.css +++ b/src/bundled/channels/ChannelSidebarRow.module.css @@ -66,24 +66,6 @@ color: var(--text-standard); font-weight: var(--type-weight-medium); } -.positioner { - z-index: var(--layer-popover); -} -.menu.menu { - position: relative; - width: max-content; - max-width: var(--available-width); - font-size: var(--text-body-sm); - outline: none; -} -.menuItem { - white-space: nowrap; - outline: none; -} -.menuItem[data-highlighted] { - background: var(--completion-highlight); -} - .row .disclosure { position: absolute; left: calc(var(--space-control-inset) - (var(--size-row) - 17px) / 2); diff --git a/src/bundled/channels/ChannelSidebarRow.test.tsx b/src/bundled/channels/ChannelSidebarRow.test.tsx index ad63a91b1..b5947b6eb 100644 --- a/src/bundled/channels/ChannelSidebarRow.test.tsx +++ b/src/bundled/channels/ChannelSidebarRow.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment jsdom import "@testing-library/jest-dom/vitest"; import { afterEach, expect, it, vi } from "vitest"; -import { cleanup, render, screen, waitFor } from "@testing-library/react"; +import { cleanup, render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { useState } from "react"; import type { RelaySession } from "../../features/relay/session"; @@ -73,7 +73,6 @@ function mount(selected = "child") { draft={true} draftSelected={false} selected={selected} - sessionsEnabled onSelect={onSelect} onPrepare={() => {}} onNewSession={onNewSession} @@ -83,61 +82,6 @@ function mount(selected = "child") { render(); return { onSelect, onNewSession }; } -it("opens a compact action menu independently of selecting its channel", async () => { - const user = userEvent.setup(); - const callbacks = mount(); - const trigger = screen.getByRole("button", { - name: "More options for Engineering", - }); - expect(trigger).toHaveAttribute("data-icon-shape", "round"); - await user.click(trigger); - await user.click( - await screen.findByRole("menuitem", { name: "New session" }), - ); - expect(callbacks.onNewSession).toHaveBeenCalledWith("parent"); - expect(callbacks.onSelect).not.toHaveBeenCalled(); - // The first close must finish before reopening, and keyboard dismissal must - // start after the popup has taken focus rather than racing its focus effect. - await waitFor(() => - expect( - screen.queryByRole("menu", { name: "More options for Engineering" }), - ).not.toBeInTheDocument(), - ); - await user.click(trigger); - await waitFor(() => - expect( - screen.getByRole("menu", { name: "More options for Engineering" }), - ).toHaveFocus(), - ); - await user.keyboard("{Escape}"); - await waitFor(() => { - expect( - screen.queryByRole("menu", { name: "More options for Engineering" }), - ).not.toBeInTheDocument(); - expect(trigger).toHaveFocus(); - }); -}); -it("hides session actions when the Sessions plugin is unavailable", () => { - const onNewSession = vi.fn(); - render( - {}} - icon={} - sessions={[]} - sessionsEnabled={false} - draft={false} - draftSelected={false} - onSelect={() => {}} - onPrepare={() => {}} - onNewSession={onNewSession} - />, - ); - expect( - screen.queryByRole("button", { name: "More options for Engineering" }), - ).not.toBeInTheDocument(); -}); it("offers a separate hide action only for DM rows", async () => { const onHideDm = vi.fn(); const onSelect = vi.fn(); @@ -148,7 +92,6 @@ it("offers a separate hide action only for DM rows", async () => { onToggle={() => {}} icon={} sessions={[]} - sessionsEnabled={false} draft={false} draftSelected={false} onSelect={onSelect} @@ -182,7 +125,6 @@ it("keeps focus in the sidebar when removing its last visible DM", async () => { onToggle={() => {}} icon={} sessions={[]} - sessionsEnabled={false} draft={false} draftSelected={false} onSelect={() => {}} diff --git a/src/bundled/channels/ChannelSidebarRow.tsx b/src/bundled/channels/ChannelSidebarRow.tsx index 0c90b58b2..8f4c2e275 100644 --- a/src/bundled/channels/ChannelSidebarRow.tsx +++ b/src/bundled/channels/ChannelSidebarRow.tsx @@ -3,13 +3,10 @@ import { IconButton } from "../../shared/design-system/ui/IconButton"; import { CaretDownIcon, CaretRightIcon, - DotsThreeVerticalIcon, XIcon, } from "../../shared/design-system/icons/index"; -import { useId, useRef, type ReactElement, type ReactNode } from "react"; -import { Menu } from "@base-ui/react/menu"; +import { useId, type ReactElement, type ReactNode } from "react"; import type { ChannelSummary } from "../../features/relay/contracts"; -import completion from "../../features/conversation/Completions.module.css"; import styles from "./ChannelSidebarRow.module.css"; export function ChannelSidebarRow({ @@ -19,7 +16,6 @@ export function ChannelSidebarRow({ childContent, wrapSelect, selected, - sessionsEnabled, sessions, draft, draftSelected, @@ -36,7 +32,6 @@ export function ChannelSidebarRow({ childContent?: ((channel: ChannelSummary) => ReactNode) | undefined; wrapSelect?: ((trigger: ReactElement) => ReactNode) | undefined; selected?: string | undefined; - sessionsEnabled: boolean; sessions: readonly ChannelSummary[]; draft: boolean; draftSelected: boolean; @@ -47,15 +42,9 @@ export function ChannelSidebarRow({ onNewSession: (id: string) => void; onHideDm?: (id: string) => void; }) { - const starting = useRef(false); const childrenId = useId(); const hasChildren = draft || sessions.length > 0; const Chevron = collapsed ? CaretRightIcon : CaretDownIcon; - const canParent = - sessionsEnabled && - channel.channelType !== "dm" && - channel.channelType !== "session" && - !channel.archived; const selectButton = ( {wrapSelect ? wrapSelect(selectButton) : selectButton} - {canParent && ( - { - if (open) starting.current = false; - }} - > - - } - /> - } - aria-label={`More options for ${channel.name}`} - /> - - - - - starting.current - ? (document - .getElementById("new-session-prompt") - ?.querySelector('[role="textbox"]') ?? - false) - : true - } - aria-label={`${channel.name} options`} - > - { - starting.current = true; - onNewSession(channel.id); - }} - > - New session - - - - - - )} {channel.channelType === "dm" && onHideDm && ( (null); + const startingSession = useRef(false); const pendingChannelCreation = useSyncExternalStore( queries.channelCreation.subscribe, queries.channelCreation.snapshot, @@ -921,6 +931,42 @@ function ChannelWorkspace({ : preferences.data, hiddenDms.hiddenIds, ); + // Compose actual items here; menu availability is their count, not the policy + // of any one action. Sibling actions keep their own eligibility checks. + const rowActions = (channel: ChannelSummary) => { + const actions: ReactNode[] = []; + if ( + sessionsEnabled && + channel.channelType !== "dm" && + channel.channelType !== "session" && + !channel.archived + ) { + actions.push( + { + startingSession.current = true; + startSession(channel.id); + }} + > + New session + , + ); + } + return actions; + }; + const { + rowMenu, + open: openMenu, + close: closeRowMenu, + } = useChannelRowMenu(sections, rowActions); + const openRowMenu = useCallback( + (channel: ChannelSummary, sectionKey: string, anchor?: HTMLElement) => { + startingSession.current = false; + openMenu(channel, sectionKey, anchor); + }, + [openMenu], + ); return (
child.id === current?.id) ? current?.id : undefined; - return ( + const actions = rowActions(channel); + const menuEnabled = actions.length > 0; + const menuOpen = + menuEnabled && + rowMenu?.channelId === channel.id && + rowMenu.sectionKey === section.key; + const channelItem = ( ); + if (!menuEnabled) return channelItem; + return ( + { + if (open) openRowMenu(channel, section.key); + else if (menuOpen) closeRowMenu(); + }} + > + {channelItem} + + startingSession.current + ? (document + .getElementById("new-session-prompt") + ?.querySelector( + '[role="textbox"]', + ) ?? false) + : (sidebar.list.current?.querySelector( + `[data-channel-id="${CSS.escape(channel.id)}"]`, + ) ?? false) + } + > + {actions} + + + ); })} ); diff --git a/src/bundled/channels/useChannelRowMenu.test.tsx b/src/bundled/channels/useChannelRowMenu.test.tsx new file mode 100644 index 000000000..a105acd3f --- /dev/null +++ b/src/bundled/channels/useChannelRowMenu.test.tsx @@ -0,0 +1,122 @@ +// @vitest-environment jsdom +import { StrictMode, type ReactNode } from "react"; +import { act, cleanup, renderHook } from "@testing-library/react"; +import { afterEach, expect, it } from "vitest"; +import type { ChannelSummary } from "../../features/relay/contracts"; +import { sidebarSections } from "./sidebar-sections"; +import { useChannelRowMenu } from "./useChannelRowMenu"; + +afterEach(cleanup); +const alpha: ChannelSummary = { + id: "alpha", + name: "Alpha", + channelType: "stream", +}; +const beta: ChannelSummary = { + id: "beta", + name: "Beta", + channelType: "stream", +}; +const newSession = ["New session"]; +const groups = [{ id: "work", name: "Work", order: 0 }]; +const sections = ( + placement: "channels" | "group:work" | "starred", + channels = [alpha, beta], +) => + sidebarSections(channels, { + sections: groups, + assignments: placement === "group:work" ? { alpha: "work" } : {}, + starred: placement === "starred" ? ["alpha"] : [], + }); +const initial = { + sections: sections("group:work"), + actionsFor: () => newSession as readonly ReactNode[], +}; +function mount() { + return renderHook( + ({ sections, actionsFor }) => useChannelRowMenu(sections, actionsFor), + { + initialProps: initial, + wrapper: ({ children }) => {children}, + }, + ); +} + +it.each(["channels", "group:work", "starred"] as const)( + "forgets a menu moved away from %s and never reopens it on return", + (placement) => { + const view = mount(); + const original = { ...initial, sections: sections(placement) }; + view.rerender(original); + act(() => view.result.current.open(alpha, placement)); + expect(view.result.current.rowMenu?.channelId).toBe("alpha"); + view.rerender({ + ...initial, + sections: sections(placement === "starred" ? "channels" : "starred"), + }); + expect(view.result.current.rowMenu).toBeUndefined(); + view.rerender(original); + expect(view.result.current.rowMenu).toBeUndefined(); + act(() => view.result.current.open(alpha, placement)); + expect(view.result.current.rowMenu?.sectionKey).toBe(placement); + }, +); + +it.each(["removed", "archived", "hidden", "no actions"])( + "forgets a menu when its row is %s, including restoration", + (reason) => { + const view = mount(); + act(() => view.result.current.open(alpha, "group:work")); + view.rerender({ + sections: sections( + "group:work", + reason === "removed" + ? [beta] + : [ + { + ...alpha, + ...(reason === "archived" ? { archived: true as const } : {}), + ...(reason === "hidden" ? { hidden: true as const } : {}), + }, + beta, + ], + ), + actionsFor: () => (reason === "no actions" ? [] : newSession), + }); + expect(view.result.current.rowMenu).toBeUndefined(); + view.rerender(initial); + expect(view.result.current.rowMenu).toBeUndefined(); + }, +); + +it("retains the same eligible placement and keyboard anchor across harmless updates", () => { + const view = mount(); + const { open, close } = view.result.current; + const anchor = document.createElement("button"); + act(() => open(alpha, "group:work", anchor)); + const menu = view.result.current.rowMenu; + view.rerender({ + ...initial, + sections: sections("group:work", [{ ...alpha, name: "Renamed" }, beta]), + }); + expect(view.result.current.rowMenu).toBe(menu); + expect(view.result.current.rowMenu?.anchor).toBe(anchor); + expect(view.result.current.open).toBe(open); + expect(view.result.current.close).toBe(close); + act(() => close()); + expect(view.result.current.rowMenu).toBeUndefined(); +}); + +it("keeps menus with independent actions when New session is unavailable, including DMs", () => { + const view = mount(); + const dm: ChannelSummary = { id: "dm", name: "DM", channelType: "dm" }; + // Test-only action: no new product action or writer is introduced by this seam. + const actionsFor = () => ["Independent action"]; + view.rerender({ sections: sidebarSections([dm]), actionsFor }); + act(() => view.result.current.open(dm, "dms")); + expect(view.result.current.rowMenu?.channelId).toBe("dm"); + view.rerender({ sections: sidebarSections([dm]), actionsFor: () => [] }); + expect(view.result.current.rowMenu).toBeUndefined(); + view.rerender({ sections: sidebarSections([dm]), actionsFor }); + expect(view.result.current.rowMenu).toBeUndefined(); +}); diff --git a/src/bundled/channels/useChannelRowMenu.ts b/src/bundled/channels/useChannelRowMenu.ts new file mode 100644 index 000000000..9f4167b50 --- /dev/null +++ b/src/bundled/channels/useChannelRowMenu.ts @@ -0,0 +1,37 @@ +import { useCallback, useState, type ReactNode } from "react"; +import type { ChannelSummary } from "../../features/relay/contracts"; + +type Section = { key: string; rows: readonly ChannelSummary[] }; +type RowMenu = { + channelId: string; + sectionKey: string; + anchor?: HTMLElement; +}; + +/** Page-owned menu identity follows rendered placement, not a saved group id. */ +export function useChannelRowMenu( + sections: readonly Section[], + actionsFor: (channel: ChannelSummary) => readonly ReactNode[], +) { + const [rowMenu, setRowMenu] = useState(); + // Clear during render so children cannot commit a stale portal after a move. + // Merely masking `open` leaves the old identity able to resurrect on return. + if (rowMenu) { + const channel = sections + .find((section) => section.key === rowMenu.sectionKey) + ?.rows.find((channel) => channel.id === rowMenu.channelId); + if (!channel || actionsFor(channel).length === 0) setRowMenu(undefined); + } + const open = useCallback( + (channel: ChannelSummary, sectionKey: string, anchor?: HTMLElement) => { + setRowMenu({ + channelId: channel.id, + sectionKey, + ...(anchor ? { anchor } : {}), + }); + }, + [], + ); + const close = useCallback(() => setRowMenu(undefined), []); + return { rowMenu, open, close }; +} diff --git a/src/shared/design-system/DESIGN.md b/src/shared/design-system/DESIGN.md index 6f890d5ee..546f57b88 100644 --- a/src/shared/design-system/DESIGN.md +++ b/src/shared/design-system/DESIGN.md @@ -261,6 +261,16 @@ Route navigation uses NavigationItem with aria-current instead. NavigationItem forwards normal button events, refs and data attributes so unread observation, preloading and product shortcuts remain with the caller. +## Menu inset corners + +Shared menu edge items follow the popup's inset curve: panel radius minus popup +padding and border (24px − 4px − 1px = 19px at the default scale). A single item +uses that radius on all four corners; multi-item menus use it only on the top +corners of the first item and bottom corners of the last. Interior corners keep +the row radius. The shared recipe handles direct items and edge radio groups, +ignoring Base UI focus/portal sentinels. Do not add feature-local radius overrides +or change the global row radius to correct a menu. + ## Align row content, not state backgrounds When composing NavigationItem lists inside dialogs or padded panels, align the diff --git a/src/shared/design-system/styles/components.css b/src/shared/design-system/styles/components.css index 80710142c..411b5e9b1 100644 --- a/src/shared/design-system/styles/components.css +++ b/src/shared/design-system/styles/components.css @@ -756,6 +756,35 @@ outline: none; user-select: none; } + /* Edge rows follow the surface inset; ignore Base UI portal/focus sentinels. */ + .buzz-menu-popup + > .buzz-menu-item:nth-child( + 1 of :not([data-base-ui-focus-guard], [aria-owns]) + ), + .buzz-menu-popup + > [role="group"]:nth-child( + 1 of :not([data-base-ui-focus-guard], [aria-owns]) + ) + > .buzz-menu-item:first-child { + border-top-left-radius: calc(var(--radius-panel) - var(--space-1) - 1px); + border-top-right-radius: calc(var(--radius-panel) - var(--space-1) - 1px); + } + .buzz-menu-popup + > .buzz-menu-item:nth-last-child( + 1 of :not([data-base-ui-focus-guard], [aria-owns]) + ), + .buzz-menu-popup + > [role="group"]:nth-last-child( + 1 of :not([data-base-ui-focus-guard], [aria-owns]) + ) + > .buzz-menu-item:last-child { + border-bottom-left-radius: calc(var(--radius-panel) - var(--space-1) - 1px); + border-bottom-right-radius: calc( + var(--radius-panel) - + var(--space-1) - + 1px + ); + } html[data-keyboard-navigation] .buzz-menu-item:focus-visible { outline: 2px solid var(--border-focus); /* Keep the ring inside the scrollable popup's clipping edge. */ diff --git a/tests/browser/navigation-session-menu.spec.mjs b/tests/browser/navigation-session-menu.spec.mjs new file mode 100644 index 000000000..63aa43e56 --- /dev/null +++ b/tests/browser/navigation-session-menu.spec.mjs @@ -0,0 +1,166 @@ +import { test, expect } from "./fixture.mjs"; +import { open } from "./timeline.mjs"; + +// Real app composition: context-menu dismissal must hand focus to the newly +// mounted composer, not restore it to the row. No preference/read-write host +// support is enabled: session entry must not depend on those capabilities. +test.use({ historyCounts: { alpha: 1, beta: 1 } }); + +test("channel context menu opens and resumes a session draft without a row menu button", async ({ + page, + app, +}) => { + await open(page, app); + // Base UI hides the background from accessibility while the menu is modal. + const sidebar = page.getByRole("navigation", { + name: "Subscribed channels", + includeHidden: true, + }); + const beta = sidebar.locator('[data-channel-id="beta"]'); + const alpha = sidebar.locator('[data-channel-id="alpha"]'); + const menu = page.getByRole("menu", { name: "Actions for Beta" }); + const start = menu.getByRole("menuitem", { + name: "New session", + exact: true, + }); + const composer = page.getByRole("textbox", { + name: "Message this session", + exact: true, + }); + await beta.hover(); + await expect( + sidebar.getByRole("button", { name: /More options/ }), + ).toHaveCount(0); + await beta.click({ button: "right" }); + await expect(start).toBeVisible(); + await expect(menu.getByRole("menuitem")).toHaveCount(1); + await expect(alpha).toHaveAttribute("aria-current", "page"); + // CSS inset geometry needs a real layout engine. A lone item follows all four + // popup corners in both themes, independent of available viewport width. + for (const mode of ["light", "dark"]) { + await page.evaluate((value) => { + document.documentElement.dataset.colorMode = value; + }, mode); + for (const width of [720, 1000, 1440]) { + await page.setViewportSize({ width, height: 950 }); + const inset = await menu.evaluate((popup) => { + const style = getComputedStyle(popup); + return ( + Number.parseFloat(style.borderTopLeftRadius) - + Number.parseFloat(style.paddingTop) - + Number.parseFloat(style.borderTopWidth) + ); + }); + for (const corner of [ + "top-left", + "top-right", + "bottom-left", + "bottom-right", + ]) { + await expect(start).toHaveCSS(`border-${corner}-radius`, `${inset}px`); + } + } + } + await start.click(); + await expect(menu).toHaveCount(0); + await expect(composer).toBeFocused(); + await expect( + sidebar.getByRole("button", { name: "New session draft in Beta" }), + ).toBeVisible(); + await composer.fill("Keep this draft"); + await alpha.click(); + await beta.focus(); + await page.keyboard.press("Shift+F10"); + await expect(menu).toBeFocused(); + await page.keyboard.press("Escape"); + await expect(menu).toHaveCount(0); + await expect(beta).toBeFocused(); + await page.keyboard.press("ContextMenu"); + await expect(menu).toBeFocused(); + await page.keyboard.press("ArrowDown"); + await expect(start).toBeFocused(); + await page.keyboard.press("Enter"); + await expect(menu).toHaveCount(0); + await expect(composer).toBeFocused(); + await expect(composer).toHaveText("Keep this draft"); +}); + +// Representative app-wiring regression: a decoded preference update unmounts an +// open Base UI portal while ChannelsPage stays mounted. Transition permutations +// belong in useChannelRowMenu.test.tsx; this checks the actual page/portal seam. +test.describe("menu placement lifetime", () => { + test.use({ productionBroker: true, savedSidebar: true }); + test("does not resurrect a menu after a preference refresh moves its row away and back", async ({ + page, + app, + }) => { + await open(page, app); + const sidebar = page.getByRole("navigation", { + name: "Subscribed channels", + includeHidden: true, + }); + const work = sidebar + .locator("details") + .filter({ has: page.locator("summary", { hasText: /^Work$/ }) }); + const starred = sidebar + .locator("details") + .filter({ has: page.locator("summary", { hasText: /Starred$/ }) }); + const beta = sidebar.locator('[data-channel-id="beta"]'); + const menu = page.getByRole("menu", { name: "Actions for Beta" }); + await expect(work.locator('[data-channel-id="beta"]')).toBeVisible(); + await page + .getByRole("button", { name: "Channel settings", exact: true }) + .click(); + await page.getByText("Diagnostics", { exact: true }).click(); + const refresh = page.getByRole("button", { + name: "Refresh groups and stars", + exact: true, + }); + let release; + const held = new Promise((resolve) => { + release = resolve; + }); + let requested; + const started = new Promise((resolve) => { + requested = resolve; + }); + let stars = ["alpha", "beta"]; + // Fake only the host decode response. No app hook or client state injection; + // no account writes. The existing refresh/store subscription drives the UI. + await page.route("**/sidebar-preferences", async (route) => { + requested(); + await held; + await route.fulfill({ + json: { + sections: [{ id: "work", name: "Work", order: 0 }], + assignments: { beta: "work" }, + starred: stars, + }, + }); + }); + try { + await refresh.click(); + await started; + await expect(refresh).toBeDisabled(); + await beta.click({ button: "right" }); + await expect(menu).toBeVisible(); + release(); + await expect(starred.locator('[data-channel-id="beta"]')).toBeVisible(); + await expect(menu).toHaveCount(0); + stars = ["alpha"]; + await expect(refresh).toBeEnabled(); + await refresh.click(); + await expect(work.locator('[data-channel-id="beta"]')).toBeVisible(); + await expect(menu).toHaveCount(0); + await beta.focus(); + await page.keyboard.press("Shift+F10"); + await expect(menu).toBeFocused(); + await page.keyboard.press("Escape"); + await expect(menu).toHaveCount(0); + await expect(beta).toBeFocused(); + } finally { + release(); + await page.unroute("**/sidebar-preferences"); + } + }); +}); diff --git a/tests/browser/navigation-sidebar.spec.mjs b/tests/browser/navigation-sidebar.spec.mjs index c068837bb..2b28d4b4d 100644 --- a/tests/browser/navigation-sidebar.spec.mjs +++ b/tests/browser/navigation-sidebar.spec.mjs @@ -213,22 +213,9 @@ sessionSidebar( const parentSurface = parent.locator( "xpath=ancestor::*[@data-channel-sidebar-row]", ); - const more = page.getByRole("button", { name: /More options for/ }).first(); - await expect(more).toHaveAttribute("data-icon-shape", "round"); - const [parentSurfaceBox, moreBox] = await Promise.all([ - parentSurface.boundingBox(), - more.boundingBox(), - ]); - expect(parentSurfaceBox.x + parentSurfaceBox.width).toBeCloseTo( - moreBox.x + moreBox.width, - 0, - ); - expect( - await more.evaluate( - (action, row) => row.contains(action), - await parentSurface.elementHandle(), - ), - ).toBe(true); + await expect( + page.getByRole("button", { name: /More options for/ }), + ).toHaveCount(0); expect( await parent.evaluate((row) => getComputedStyle(row).backgroundColor), ).toBe("rgba(0, 0, 0, 0)"); @@ -238,7 +225,7 @@ sessionSidebar( ), ).not.toBe("rgba(0, 0, 0, 0)"); - await more.click(); + await parent.click({ button: "right" }); await page.getByRole("menuitem", { name: "New session" }).click(); const draft = page.getByRole("button", { name: /New session draft in/ }); await expect(draft).toBeVisible(); @@ -355,15 +342,18 @@ sessionSidebar( }, ); -test("session actions follow the Sessions plugin availability", async ({ +test("disabling the only row action leaves no empty menu", async ({ page, app, }) => { await open(page, app); - await button(page, "Alpha").hover(); + await button(page, "Alpha").click({ button: "right" }); + const menu = page.getByRole("menu", { name: "Actions for Alpha" }); await expect( - page.getByRole("button", { name: "More options for Alpha" }), + menu.getByRole("menuitem", { name: "New session" }), ).toBeVisible(); + await page.keyboard.press("Escape"); + await expect(menu).toHaveCount(0); await button(page, "Your profile").click(); await page.getByRole("menuitem", { name: "Settings", exact: true }).click(); @@ -377,10 +367,11 @@ test("session actions follow the Sessions plugin availability", async ({ await expect(toggle).toHaveAttribute("aria-checked", "false"); await button(page, "Messages").first().click(); - await button(page, "Alpha").hover(); - await expect( - page.getByRole("button", { name: "More options for Alpha" }), - ).toHaveCount(0); + await button(page, "Alpha").click({ button: "right" }); + await expect(menu).toHaveCount(0); + await button(page, "Alpha").focus(); + await page.keyboard.press("Shift+F10"); + await expect(menu).toHaveCount(0); }); test("channel navigation preserves sidebar DOM, group state and scroll", async ({ diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index 8731a25af..06efa230b 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -110,8 +110,21 @@ test("thread buttons show observed unread independently, clear only after readin const queries = () => app.report.queries.filter(({ filter }) => filter.depth_limit); expect(queries()).toHaveLength(0); // Merely displaying buttons never fetches threads. + // The sibling context trigger must not steal the activity button's props or + // focus. Exercise the real portals while unread activity is still present. + await alpha.click({ button: "right" }); + const actions = page.getByRole("menu", { name: "Actions for Alpha" }); + await expect( + actions.getByRole("menuitem", { name: "New session" }), + ).toBeVisible(); await page.keyboard.press("Escape"); - await alpha.focus(); + await expect(actions).toHaveCount(0); + await expect(alpha).toBeFocused(); + await alpha.press("Shift+F10"); + await expect(actions).toBeFocused(); + await page.keyboard.press("Escape"); + await expect(actions).toHaveCount(0); + await expect(alpha).toBeFocused(); await alpha.press("Enter"); await expect(popover).toBeVisible(); const item = popover From 9c73c111fa08d77b90ce03d39186c2b940700b71 Mon Sep 17 00:00:00 2001 From: Carl Date: Wed, 23 Sep 2026 23:42:05 -0700 Subject: [PATCH 2/4] fix(design-system): fully round every menu row Signed-off-by: Carl --- src/shared/design-system/DESIGN.md | 16 +++++----- .../design-system/styles/components.css | 31 +------------------ src/shared/design-system/tokens/registry.ts | 2 +- .../browser/navigation-session-menu.spec.mjs | 20 ++++++------ tests/fixtures/design-system/viewer.spec.ts | 31 ++++++++++++++++++- 5 files changed, 50 insertions(+), 50 deletions(-) diff --git a/src/shared/design-system/DESIGN.md b/src/shared/design-system/DESIGN.md index 546f57b88..092ef2b9b 100644 --- a/src/shared/design-system/DESIGN.md +++ b/src/shared/design-system/DESIGN.md @@ -261,15 +261,13 @@ Route navigation uses NavigationItem with aria-current instead. NavigationItem forwards normal button events, refs and data attributes so unread observation, preloading and product shortcuts remain with the caller. -## Menu inset corners - -Shared menu edge items follow the popup's inset curve: panel radius minus popup -padding and border (24px − 4px − 1px = 19px at the default scale). A single item -uses that radius on all four corners; multi-item menus use it only on the top -corners of the first item and bottom corners of the last. Interior corners keep -the row radius. The shared recipe handles direct items and edge radio groups, -ignoring Base UI focus/portal sentinels. Do not add feature-local radius overrides -or change the global row radius to correct a menu. +## Menu row corners + +Every shared menu item uses `--radius-pill` on all four corners. First, middle and +last rows keep the same fully rounded highlight, so moving between them does not +change its shape. Direct items, grouped choices and submenu triggers share this +recipe. Do not add positional or feature-local radius overrides, derive a special +menu inset radius, or change the global row radius to correct a menu. ## Align row content, not state backgrounds diff --git a/src/shared/design-system/styles/components.css b/src/shared/design-system/styles/components.css index 411b5e9b1..9c2a9c540 100644 --- a/src/shared/design-system/styles/components.css +++ b/src/shared/design-system/styles/components.css @@ -749,42 +749,13 @@ min-height: var(--size-row); align-items: center; gap: var(--space-row-gap); - border-radius: var(--radius-row); + border-radius: var(--radius-pill); padding: var(--space-1) var(--space-2); color: var(--text-standard); cursor: default; outline: none; user-select: none; } - /* Edge rows follow the surface inset; ignore Base UI portal/focus sentinels. */ - .buzz-menu-popup - > .buzz-menu-item:nth-child( - 1 of :not([data-base-ui-focus-guard], [aria-owns]) - ), - .buzz-menu-popup - > [role="group"]:nth-child( - 1 of :not([data-base-ui-focus-guard], [aria-owns]) - ) - > .buzz-menu-item:first-child { - border-top-left-radius: calc(var(--radius-panel) - var(--space-1) - 1px); - border-top-right-radius: calc(var(--radius-panel) - var(--space-1) - 1px); - } - .buzz-menu-popup - > .buzz-menu-item:nth-last-child( - 1 of :not([data-base-ui-focus-guard], [aria-owns]) - ), - .buzz-menu-popup - > [role="group"]:nth-last-child( - 1 of :not([data-base-ui-focus-guard], [aria-owns]) - ) - > .buzz-menu-item:last-child { - border-bottom-left-radius: calc(var(--radius-panel) - var(--space-1) - 1px); - border-bottom-right-radius: calc( - var(--radius-panel) - - var(--space-1) - - 1px - ); - } html[data-keyboard-navigation] .buzz-menu-item:focus-visible { outline: 2px solid var(--border-focus); /* Keep the ring inside the scrollable popup's clipping edge. */ diff --git a/src/shared/design-system/tokens/registry.ts b/src/shared/design-system/tokens/registry.ts index 4e681a1d5..0c10d5d8b 100644 --- a/src/shared/design-system/tokens/registry.ts +++ b/src/shared/design-system/tokens/registry.ts @@ -1116,7 +1116,7 @@ export const RADII = [ token: "radius-pill", variable: "--radius-pill", value: "round", - use: "Pills, avatars, and fully circular controls.", + use: "Pills, menu rows, avatars, and fully circular controls.", }, ]; diff --git a/tests/browser/navigation-session-menu.spec.mjs b/tests/browser/navigation-session-menu.spec.mjs index 63aa43e56..4bb848d58 100644 --- a/tests/browser/navigation-session-menu.spec.mjs +++ b/tests/browser/navigation-session-menu.spec.mjs @@ -35,21 +35,23 @@ test("channel context menu opens and resumes a session draft without a row menu await expect(start).toBeVisible(); await expect(menu.getByRole("menuitem")).toHaveCount(1); await expect(alpha).toHaveAttribute("aria-current", "page"); - // CSS inset geometry needs a real layout engine. A lone item follows all four - // popup corners in both themes, independent of available viewport width. + // CSS geometry needs a real layout engine. A lone item uses the shared full- + // round token in both themes, independent of available viewport width. for (const mode of ["light", "dark"]) { await page.evaluate((value) => { document.documentElement.dataset.colorMode = value; }, mode); for (const width of [720, 1000, 1440]) { await page.setViewportSize({ width, height: 950 }); - const inset = await menu.evaluate((popup) => { - const style = getComputedStyle(popup); - return ( - Number.parseFloat(style.borderTopLeftRadius) - - Number.parseFloat(style.paddingTop) - - Number.parseFloat(style.borderTopWidth) + const radius = await start.evaluate((element) => { + // The shared pill token is rem-based; computed corner values are pixels. + const rem = Number.parseFloat( + getComputedStyle(element).getPropertyValue("--radius-pill"), ); + const rootSize = Number.parseFloat( + getComputedStyle(document.documentElement).fontSize, + ); + return `${rem * rootSize}px`; }); for (const corner of [ "top-left", @@ -57,7 +59,7 @@ test("channel context menu opens and resumes a session draft without a row menu "bottom-left", "bottom-right", ]) { - await expect(start).toHaveCSS(`border-${corner}-radius`, `${inset}px`); + await expect(start).toHaveCSS(`border-${corner}-radius`, radius); } } } diff --git a/tests/fixtures/design-system/viewer.spec.ts b/tests/fixtures/design-system/viewer.spec.ts index 01e81f6a1..1d029ec7d 100644 --- a/tests/fixtures/design-system/viewer.spec.ts +++ b/tests/fixtures/design-system/viewer.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test, type Locator } from "@playwright/test"; import { COMPONENTS } from "../../../src/shared/design-system/ui/registry"; import { PHOSPHOR_ICONS } from "../../../src/shared/design-system/icons/inventory"; @@ -1014,6 +1014,27 @@ test("menu items retain keyboard navigation with hidden focus outlines in both m const submenu = page.getByRole("menuitem", { name: "Sort", exact: true }); const recent = page.getByRole("menuitemradio", { name: "Recent" }); const alpha = page.getByRole("menuitemradio", { name: "A–Z" }); + // Every position and grouped choice uses the shared full-round token. + const expectRounded = async (item: Locator) => { + const radius = await item.evaluate((element) => { + // The shared pill token is rem-based; computed corner values are pixels. + const rem = Number.parseFloat( + getComputedStyle(element).getPropertyValue("--radius-pill"), + ); + const rootSize = Number.parseFloat( + getComputedStyle(document.documentElement).fontSize, + ); + return `${rem * rootSize}px`; + }); + for (const corner of [ + "top-left", + "top-right", + "bottom-left", + "bottom-right", + ]) { + await expect(item).toHaveCSS(`border-${corner}-radius`, radius); + } + }; const tab = browserName === "webkit" && process.platform === "darwin" ? "Alt+Tab" @@ -1023,6 +1044,10 @@ test("menu items retain keyboard navigation with hidden focus outlines in both m if (await toggle.count()) await toggle.click(); await trigger.click(); await expect(page.getByRole("menu")).toHaveCSS("transform", "none"); + for (const width of [390, 800, 1280]) { + await page.setViewportSize({ width, height: 900 }); + for (const item of [action, checkbox, submenu]) await expectRounded(item); + } await page.mouse.move(0, 0); await action.hover(); // Programmatic focus following a pointer open must stay quiet too. @@ -1042,6 +1067,10 @@ test("menu items retain keyboard navigation with hidden focus outlines in both m } await page.keyboard.press("ArrowRight"); await expect(recent).toBeFocused(); + for (const width of [390, 800, 1280]) { + await page.setViewportSize({ width, height: 900 }); + for (const item of [recent, alpha]) await expectRounded(item); + } await expect(recent).toHaveCSS("outline-style", "none"); await page.keyboard.press("ArrowDown"); await expect(alpha).toBeFocused(); From f071f8774af990d550869477411976e7657cd448 Mon Sep 17 00:00:00 2001 From: Carl Date: Wed, 23 Sep 2026 23:45:49 -0700 Subject: [PATCH 3/4] test(channels): settle activity hover before keyboard entry Signed-off-by: Carl --- tests/browser/thread-unread.spec.mjs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index 06efa230b..2516d723c 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -125,7 +125,13 @@ test("thread buttons show observed unread independently, clear only after readin await page.keyboard.press("Escape"); await expect(actions).toHaveCount(0); await expect(alpha).toBeFocused(); + // Context-menu dismissal can leave Activity hover-open under the pointer. + // Leave hover and finish its exit before Enter tests keyboard opening rather + // than toggling it closed and observing the still-visible exit animation. + await page.mouse.move(0, 0); + await expect(popover).toHaveCount(0); await alpha.press("Enter"); + await expect(alpha).toHaveAttribute("aria-expanded", "true"); await expect(popover).toBeVisible(); const item = popover .getByRole("button", { From 8f8407152707531bb349ee456c34d28193d560e7 Mon Sep 17 00:00:00 2001 From: Carl Date: Thu, 24 Sep 2026 09:13:44 -0700 Subject: [PATCH 4/4] test(channels): preserve sidebar selection across new message routing Signed-off-by: Carl --- tests/browser/new-message.spec.mjs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/browser/new-message.spec.mjs b/tests/browser/new-message.spec.mjs index 2dd90b90a..afc334102 100644 --- a/tests/browser/new-message.spec.mjs +++ b/tests/browser/new-message.spec.mjs @@ -547,10 +547,14 @@ test("empty compose, keyboard selection, pagination, removal effects, retry, the // must be gone before another New message starts. await page.reload(); await expect(message).toBeVisible(); + await expect(sidebarDm).toHaveAttribute("aria-current", "page"); // Resolving an existing DM keeps its row visible while the next send is held. await page.getByText("DMs", { exact: true }).hover(); await page.getByRole("button", { name: "New message", exact: true }).click(); await page.getByRole("option", { name: "Avery Chen", exact: true }).click(); + // Composing is a separate route, not the previously selected conversation. + await expect(sidebarDm).toBeVisible(); + await expect(sidebarDm).not.toHaveAttribute("aria-current", "page"); await page .getByRole("textbox", { name: "Message Avery Chen", exact: true }) .fill("Another message"); @@ -564,5 +568,6 @@ test("empty compose, keyboard selection, pagination, removal effects, retry, the await expect( page.locator("[data-message-id]", { hasText: "Another message" }), ).toBeVisible(); + await expect(sidebarDm).toHaveAttribute("aria-current", "page"); expect(app.errors).toEqual([]); });