From c8318e9d685557868c82ce6003cfb6f9302892d8 Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Mon, 21 Sep 2026 17:03:00 -0700 Subject: [PATCH 1/2] =?UTF-8?q?fix(web):=20pin=20"Add=20workspace=E2=80=A6?= =?UTF-8?q?"=20below=20the=20New=20session=20workspace=20list?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The New session dialog offered its workspaces in a native select with "Create new workspace…" as the last option — at the end of a long list it was the row nobody scrolled to. The picker is now the app's own Menu: a row per folder (the path as a second line only where two folders share a name, the current one checked) in a scrolling list, with "Add workspace…" pinned below it after a divider, as the reference does. Menu gains a `footer` (rows outside the scrolling viewport, after a divider, never checked) and a `block` mode (the trigger fills the owner and the list spans it — the form-control shape). Rows, separators and labels no longer shrink in a height-constrained list: they are flex items of the viewport, and with an explicit min-height a long list squashed every row before it scrolled, so a two-line row's hint overlapped the row below. Unreachable before — the composer menus are short. Escape and a press outside the open list close only the list; the dialog closes on the next one. Co-Authored-By: Claude Fable 5.1 --- ui-web/README.md | 9 +- .../src/sidebar/NewSessionDialog.module.css | 51 ++++++- ui-web/src/sidebar/NewSessionDialog.test.tsx | 105 ++++++++++++-- ui-web/src/sidebar/NewSessionDialog.tsx | 133 ++++++++++++++---- ui-web/src/ui/primitives/Menu.module.css | 28 ++++ ui-web/src/ui/primitives/Menu.test.tsx | 49 +++++++ ui-web/src/ui/primitives/Menu.tsx | 74 ++++++---- 7 files changed, 378 insertions(+), 71 deletions(-) create mode 100644 ui-web/src/ui/primitives/Menu.test.tsx diff --git a/ui-web/README.md b/ui-web/README.md index 775e8e07c..117e447d0 100644 --- a/ui-web/README.md +++ b/ui-web/README.md @@ -282,9 +282,12 @@ transcript included, as before. ## New session **New session** (the sidebar button, the brand mark, `⌘⇧N`) opens a dialog -rather than starting a session on the spot. It offers every workspace the -sidebar knows plus **Create new workspace…**, which takes an absolute folder -path and creates the folder if it is not there yet (`session.create` with +rather than starting a session on the spot. Its workspace picker lists every +folder the sidebar knows (a row per folder, the path as a second line only +where two folders share a name, the current one checked) with **Add +workspace…** pinned below the list after a divider — at the end of a long +list it was the row nobody scrolled to. That takes an absolute folder path +and creates the folder if it is not there yet (`session.create` with `create_dir`). The **Worktree** switch runs the session in a fresh git worktree of that repo — the CLI's `--worktree`, under `.clawcodex/worktrees/` — so parallel sessions in one repo cannot step diff --git a/ui-web/src/sidebar/NewSessionDialog.module.css b/ui-web/src/sidebar/NewSessionDialog.module.css index 60de25eaf..17eb58433 100644 --- a/ui-web/src/sidebar/NewSessionDialog.module.css +++ b/ui-web/src/sidebar/NewSessionDialog.module.css @@ -76,7 +76,7 @@ line-height: 16px; } -.select, +.picker, .input { box-sizing: border-box; width: 100%; @@ -90,12 +90,59 @@ font-size: 13px; } -.select:focus-visible, +.picker:focus-visible, .input:focus-visible { outline: 2px solid var(--cc-alias-button-info-fill); outline-offset: -1px; } +/* The workspace picker's trigger: the folder's name, its path dimmed beside + it, a chevron — a form control that opens the themed list, where a native + select would hand its popup to the OS. */ +.picker { + display: flex; + align-items: center; + gap: 8px; + text-align: left; + cursor: pointer; +} + +.picker:hover, +.picker[aria-expanded='true'] { + background: var(--cc-alias-interactive-bg-hover); +} + +.pickerIcon { + flex: none; + display: inline-flex; + align-items: center; + color: var(--cc-alias-label-tertiary); +} + +.pickerName { + flex: none; + max-width: 50%; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.pickerPath { + flex: 1; + min-width: 0; + overflow: hidden; + color: var(--cc-alias-label-tertiary); + font-size: 12px; + text-overflow: ellipsis; + white-space: nowrap; +} + +.pickerChevron { + flex: none; + margin-left: auto; + color: var(--cc-alias-label-tertiary); +} + .input::placeholder { color: var(--cc-alias-label-caption); } diff --git a/ui-web/src/sidebar/NewSessionDialog.test.tsx b/ui-web/src/sidebar/NewSessionDialog.test.tsx index 265107c6b..1b308ce59 100644 --- a/ui-web/src/sidebar/NewSessionDialog.test.tsx +++ b/ui-web/src/sidebar/NewSessionDialog.test.tsx @@ -1,4 +1,4 @@ -import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' +import { act, cleanup, fireEvent, render, screen, within } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { ProjectNode } from '../gateway/protocol.ts' @@ -10,12 +10,24 @@ vi.mock('../state/actions.ts', () => ({ createSession: (options?: Record) => createSession(options), })) -const { NEW_WORKSPACE, NewSessionDialog, knownWorkspaces, openNewSessionDialog } = await import('./NewSessionDialog.tsx') +const { NEW_WORKSPACE, NewSessionDialog, knownWorkspaces, openNewSessionDialog, workspaceRows } = + await import('./NewSessionDialog.tsx') function project(path: string | null): ProjectNode { return { id: path ?? 'home', label: path ?? 'Home', path, repos: [] } } +/** The picker's trigger: the one control labelled "Workspace". */ +function picker(): HTMLButtonElement { + return screen.getByRole('button', { name: /^Workspace/ }) as HTMLButtonElement +} + +function openPicker(): HTMLElement { + fireEvent.click(picker()) + + return screen.getByRole('menu') +} + beforeEach(() => { createSession.mockReset() createSession.mockResolvedValue(null) @@ -58,8 +70,17 @@ describe('knownWorkspaces', () => { }) }) +describe('workspaceRows', () => { + it('names a row by its folder, and adds the path only where two folders share a name', () => { + const rows = workspaceRows(['/work/alpha', '/other/alpha', '/work/beta']) + + expect(rows.map(row => ('label' in row ? row.label : null))).toEqual(['alpha', 'alpha', 'beta']) + expect(rows.map(row => ('hint' in row ? row.hint : undefined))).toEqual(['/work/alpha', '/other/alpha', undefined]) + }) +}) + describe('NewSessionDialog', () => { - it('is closed until opened, and starts a session in the chosen workspace', async () => { + it('is closed until opened, and starts a session in the workspace picked from the list', async () => { render() expect(screen.queryByRole('dialog')).toBeNull() @@ -68,11 +89,20 @@ describe('NewSessionDialog', () => { openNewSessionDialog() }) - const select = screen.getByRole('combobox') as HTMLSelectElement - expect(select.value).toBe('/work/current') + // The trigger names the current workspace; the list is closed. + expect(picker().textContent).toContain('current') + expect(picker().textContent).toContain('/work/current') + expect(screen.queryByRole('menu')).toBeNull() expect(screen.queryByPlaceholderText('/absolute/path/to/project')).toBeNull() - fireEvent.change(select, { target: { value: '/work/alpha' } }) + const menu = openPicker() + const rows = within(menu).getAllByRole('menuitem') + expect(rows.map(row => row.textContent)).toEqual(['current', 'alpha', 'Add workspace…']) + + fireEvent.click(within(menu).getByRole('menuitem', { name: 'alpha' })) + expect(screen.queryByRole('menu')).toBeNull() + expect(picker().textContent).toContain('/work/alpha') + fireEvent.click(screen.getByRole('button', { name: 'Create' })) await act(async () => { await Promise.resolve() @@ -82,12 +112,29 @@ describe('NewSessionDialog', () => { expect($newSessionDialog.get()).toBe(false) }) + it('pins Add workspace… below the scrolling list, after a divider', () => { + // A long list: the action must not be its last row. + $projects.set(Array.from({ length: 40 }, (_, index) => project(`/work/project-${String(index)}`))) + $newSessionDialog.set(true) + render() + + const menu = openPicker() + const viewport = menu.querySelector('[class*="viewport"]') + const add = within(menu).getByRole('menuitem', { name: 'Add workspace…' }) + + expect(viewport).not.toBeNull() + expect(viewport?.contains(add)).toBe(false) + expect(within(viewport as HTMLElement).getAllByRole('menuitem')).toHaveLength(41) + // The divider sits between the list and the pinned row. + expect(add.previousElementSibling?.getAttribute('role')).toBe('separator') + }) + it('creates a new workspace from an absolute path, in a worktree when asked', async () => { $newSessionDialog.set(true) render() - const select = screen.getByRole('combobox') as HTMLSelectElement - fireEvent.change(select, { target: { value: NEW_WORKSPACE } }) + fireEvent.click(within(openPicker()).getByRole('menuitem', { name: 'Add workspace…' })) + expect(picker().textContent).toContain('New workspace') const create = screen.getByRole('button', { name: 'Create' }) as HTMLButtonElement expect(create.disabled).toBe(true) @@ -107,6 +154,20 @@ describe('NewSessionDialog', () => { expect($newSessionDialog.get()).toBe(false) }) + it('marks the picked workspace in the list, and none while adding one', () => { + $newSessionDialog.set(true) + render() + + let menu = openPicker() + const checked = (row: HTMLElement) => row.querySelector('svg[class*="check"]') !== null + expect(checked(within(menu).getByRole('menuitem', { name: 'current' }))).toBe(true) + expect(checked(within(menu).getByRole('menuitem', { name: 'alpha' }))).toBe(false) + + fireEvent.click(within(menu).getByRole('menuitem', { name: 'Add workspace…' })) + menu = openPicker() + expect(within(menu).getAllByRole('menuitem').some(checked)).toBe(false) + }) + it('keeps the dialog open with the reason when the backend refuses', async () => { createSession.mockResolvedValue('no such directory: /nope') $newSessionDialog.set(true) @@ -136,13 +197,39 @@ describe('NewSessionDialog', () => { expect(createSession).not.toHaveBeenCalled() }) + it('lets Escape and an outside press close the open list, not the dialog', () => { + $newSessionDialog.set(true) + render() + + openPicker() + fireEvent.keyDown(document, { key: 'Escape' }) + expect(screen.queryByRole('menu')).toBeNull() + expect($newSessionDialog.get()).toBe(true) + + openPicker() + const scrim = screen.getByRole('dialog').parentElement as HTMLElement + fireEvent.pointerDown(scrim) + fireEvent.click(scrim) + expect(screen.queryByRole('menu')).toBeNull() + expect($newSessionDialog.get()).toBe(true) + + // With the list closed, the same press on the scrim closes the dialog. + fireEvent.pointerDown(scrim) + fireEvent.click(scrim) + expect($newSessionDialog.get()).toBe(false) + }) + it('offers only the new-workspace path when no workspace is known', () => { $workspace.set('') $projects.set([]) $newSessionDialog.set(true) render() - expect((screen.getByRole('combobox') as HTMLSelectElement).value).toBe(NEW_WORKSPACE) + expect(picker().textContent).toContain('New workspace') expect(screen.getByPlaceholderText('/absolute/path/to/project')).toBeTruthy() + + const menu = openPicker() + expect(within(menu).getAllByRole('menuitem').map(row => row.textContent)).toEqual(['Add workspace…']) + expect(NEW_WORKSPACE).toBe('__new_workspace__') }) }) diff --git a/ui-web/src/sidebar/NewSessionDialog.tsx b/ui-web/src/sidebar/NewSessionDialog.tsx index 43d800f1a..8b4e9ef1e 100644 --- a/ui-web/src/sidebar/NewSessionDialog.tsx +++ b/ui-web/src/sidebar/NewSessionDialog.tsx @@ -4,14 +4,15 @@ import { useEffect, useMemo, useRef, useState, type FormEvent } from 'react' import { createSession } from '../state/actions.ts' import { $newSessionDialog, $projects, $workspace } from '../state/store.ts' import { Button } from '../ui/primitives/Button.tsx' -import { XIcon } from '../ui/icons.tsx' +import { Menu, type MenuEntry, type MenuItem } from '../ui/primitives/Menu.tsx' +import { ChevronDownIcon, FolderIcon, PlusIcon, XIcon } from '../ui/icons.tsx' import css from './NewSessionDialog.module.css' -/** The select value that reveals the folder-path field. */ +/** The picker choice that reveals the folder-path field. */ export const NEW_WORKSPACE = '__new_workspace__' /** The last path segment, for a label; the whole path when it has none. */ -function baseName(path: string): string { +export function baseName(path: string): string { const segments = path.split(/[/\\]/).filter(Boolean) return segments[segments.length - 1] ?? path @@ -51,6 +52,39 @@ export function knownWorkspaces(current: string, projects: readonly WorkspaceSou return paths } +/** + * The picker's rows: one per workspace, named by its folder. The full path + * rides along as the second line only where two folders share a name — the + * common case stays one compact line per row, and a `clawcodex` next to + * another `clawcodex` still tells them apart. + */ +export function workspaceRows(workspaces: readonly string[]): MenuEntry[] { + const counts = new Map() + + for (const path of workspaces) { + const name = baseName(path) + counts.set(name, (counts.get(name) ?? 0) + 1) + } + + return workspaces.map(path => { + const name = baseName(path) + + return { + icon: , + id: path, + label: name, + ...((counts.get(name) ?? 0) > 1 && { hint: path }), + } + }) +} + +/** + * Pinned below the list, after a divider, so it is in reach however many + * workspaces there are: at the end of a long list it was the row nobody + * scrolled to. + */ +const ADD_WORKSPACE: MenuItem[] = [{ icon: , id: NEW_WORKSPACE, label: 'Add workspace…' }] + export function openNewSessionDialog(): void { $newSessionDialog.set(true) } @@ -65,12 +99,13 @@ export function closeNewSessionDialog(): void { * * A session runs somewhere, and until now the only somewhere was the current * workspace: starting work in another project meant browsing to it first. - * The dialog puts the choice where the intent is. "Create new workspace…" - * takes an absolute path and makes the folder if it is not there yet; the - * worktree switch runs the session in a fresh checkout of the repo, the - * CLI's `--worktree`, so parallel sessions cannot step on each other's files. - * Errors stay in the dialog: a path the backend refuses is corrected here, - * not read off a status line behind a closed dialog. + * The dialog puts the choice where the intent is. The workspace picker lists + * every folder the sidebar knows, with **Add workspace…** pinned below the + * list; that takes an absolute path and makes the folder if it is not there + * yet. The worktree switch runs the session in a fresh checkout of the repo, + * the CLI's `--worktree`, so parallel sessions cannot step on each other's + * files. Errors stay in the dialog: a path the backend refuses is corrected + * here, not read off a status line behind a closed dialog. */ export function NewSessionDialog() { const open = useStore($newSessionDialog) @@ -84,17 +119,26 @@ function NewSessionForm() { const workspace = useStore($workspace) const projects = useStore($projects) const workspaces = useMemo(() => knownWorkspaces(workspace, projects), [projects, workspace]) + const rows = useMemo(() => workspaceRows(workspaces), [workspaces]) const [choice, setChoice] = useState(() => workspaces[0] ?? NEW_WORKSPACE) + const [menuOpen, setMenuOpen] = useState(false) const [path, setPath] = useState('') const [worktree, setWorktree] = useState(false) const [error, setError] = useState('') const [creating, setCreating] = useState(false) + // Read by the document-level handlers below, which must know the menu's + // state at the moment of the key or the press, not at their registration. + const menuOpenRef = useRef(menuOpen) + menuOpenRef.current = menuOpen + const close = closeNewSessionDialog useEffect(() => { const onKeyDown = (event: KeyboardEvent) => { - if (event.key === 'Escape') { + // With the picker open, Escape is the picker's: its own listener runs + // after this one and closes just the menu. + if (event.key === 'Escape' && !menuOpenRef.current) { event.stopPropagation() closeNewSessionDialog() } @@ -108,7 +152,10 @@ function NewSessionForm() { }, []) // The scrim closes on a click that STARTED on it: a drag that begins in - // the path field and ends outside must not throw the form away. + // the path field and ends outside must not throw the form away. A press + // that lands while the picker is open only closes the picker — this + // handler runs before the picker's document listener does, so it still + // sees the menu open. const pressedOnScrim = useRef(false) const creatingNew = choice === NEW_WORKSPACE @@ -143,8 +190,8 @@ function NewSessionForm() { pressedOnScrim.current = false }} - onMouseDown={event => { - pressedOnScrim.current = event.target === event.currentTarget + onPointerDown={event => { + pressedOnScrim.current = event.target === event.currentTarget && !menuOpen }} >
- + open={menuOpen} + selectedId={creatingNew ? undefined : choice} + /> + {creatingNew && (