From ee2a3670b8c060af968b32fb7989d5acae3167d0 Mon Sep 17 00:00:00 2001 From: Egor Kraev Date: Thu, 29 Oct 2026 16:45:47 +0100 Subject: [PATCH] Focus an empty worktree's preset choices when it is selected The choices took the keyboard focus only after the selected worktree's last terminal closed, so selecting a worktree without terminals left the keys on the sidebar. They now take the focus whenever they show, unless a dialog owns the keys or the focus is in a text field outside the terminals. In the choices, Alt+Up/Down keep switching worktrees; keyboard reordering stays in the picker. --- openspec/specs/web/spec.md | 16 +++++++-- src/web/ui/app.tsx | 36 ++++++++++++-------- src/web/ui/picker.test.ts | 50 +++++++++++++++++++++++++++- src/web/ui/picker.ts | 23 +++++++++++++ test/e2e/terminals.spec.ts | 68 +++++++++++++++++++++++++++++++++++++- 5 files changed, 176 insertions(+), 17 deletions(-) diff --git a/openspec/specs/web/spec.md b/openspec/specs/web/spec.md index a5a4b9b..e415d67 100644 --- a/openspec/specs/web/spec.md +++ b/openspec/specs/web/spec.md @@ -127,7 +127,7 @@ The selected repo tab and each repo's selected worktree SHALL be stored per brow - **THEN** the second repo tab and that worktree are selected ### Requirement: Terminal tabs -The terminal area SHALL show the selected worktree's layout tabs, each labelled by its first terminal, with the layout's active tab shown and its tab shown in the accent colour; live terminals of the worktree missing from its layout SHALL be shown as further single-pane tabs. Choosing a tab SHALL show it at once and then store it as active with `setLayout`. The tab bar SHALL offer controls to create a new tab, split the focused pane right and split it down, each naming its keyboard shortcut on hover and each starting a new terminal through the preset picker; the split controls SHALL be disabled when the focused pane's tab cannot be split further, the new-tab control when the layout holds 64 tabs, and all of them while the host is not `connected`, the worktree is prunable or gone, or no presets have been received. A new tab SHALL send `createTerm` with the chosen preset's name and command and the size of the terminal area, then add the terminal as a new active tab with `setLayout`. A terminal SHALL never be created again because a request failed or the session closed. The tab of a terminal whose process failed, and its pane header in a split, SHALL show that it exited and keep its screen readable until closed; a terminal whose process exited cleanly SHALL leave the tabs of every page, as any closed terminal does. A worktree without terminals SHALL show the presets as choices in the terminal area; no terminal SHALL be created without the user choosing one. When the selected worktree's last terminal closes and the keyboard focus is left on the page body, the choices SHALL take the keyboard focus on their first preset; otherwise showing them SHALL NOT move the keyboard focus. +The terminal area SHALL show the selected worktree's layout tabs, each labelled by its first terminal, with the layout's active tab shown and its tab shown in the accent colour; live terminals of the worktree missing from its layout SHALL be shown as further single-pane tabs. Choosing a tab SHALL show it at once and then store it as active with `setLayout`. The tab bar SHALL offer controls to create a new tab, split the focused pane right and split it down, each naming its keyboard shortcut on hover and each starting a new terminal through the preset picker; the split controls SHALL be disabled when the focused pane's tab cannot be split further, the new-tab control when the layout holds 64 tabs, and all of them while the host is not `connected`, the worktree is prunable or gone, or no presets have been received. A new tab SHALL send `createTerm` with the chosen preset's name and command and the size of the terminal area, then add the terminal as a new active tab with `setLayout`. A terminal SHALL never be created again because a request failed or the session closed. The tab of a terminal whose process failed, and its pane header in a split, SHALL show that it exited and keep its screen readable until closed; a terminal whose process exited cleanly SHALL leave the tabs of every page, as any closed terminal does. A worktree without terminals SHALL show the presets as choices in the terminal area; no terminal SHALL be created without the user choosing one. When the choices show — because a worktree without terminals is selected or the page opens on one, or because the selected worktree's last terminal closes — they SHALL take the keyboard focus on their first preset, unless a picker or dialog owns the keys or the focus is in a text field outside the terminals, which keeps it. Alt+Up and Alt+Down SHALL select the previous and next worktree from the choices too. #### Scenario: New terminal - **WHEN** the user picks a preset in a worktree without terminals @@ -161,6 +161,18 @@ The terminal area SHALL show the selected worktree's layout tabs, each labelled - **WHEN** the only terminal of the selected worktree exits cleanly while the keyboard focus is in the worktree search - **THEN** the preset choices are shown and the search keeps the keyboard focus +#### Scenario: Selected worktree's choices take the focus +- **WHEN** the user selects a worktree without terminals and presses Down and then Enter +- **THEN** the second preset starts as a new terminal of that worktree + +#### Scenario: Worktree navigation from the choices +- **WHEN** the choices of a worktree without terminals have the keyboard focus and the user presses Alt+Down and then 1 +- **THEN** the next worktree is selected without any preset moving, and the first preset starts as its new terminal + +#### Scenario: Worktree search keeps the focus +- **WHEN** the user presses Alt+Down in the worktree search and the next worktree has no terminals +- **THEN** its preset choices are shown and the search keeps the keyboard focus + ### Requirement: Terminal lifetime Each terminal SHALL have exactly one terminal object and container in the page, created on the first view of its worktree, which also attaches every terminal of that worktree. The object SHALL never be re-created or removed from the document while its PTY lives — not by switching, renderer changes, reconnects or snapshots — and SHALL be disposed when its terminal is closed or its daemon instance is gone. Hidden terminals SHALL keep receiving output. Each terminal SHALL keep 10000 lines of scrollback, open links in a new window without access to the page, and measure character widths per Unicode 11. @@ -475,7 +487,7 @@ The sidebar's right edge SHALL be a separator that the user can drag, and move b - **THEN** the sidebar is 384 px wide ### Requirement: Editing presets -The preset picker and a worktree's preset choices SHALL offer "+ Add preset", opening a form for a name and a command; submitting SHALL trim both, treat an empty command as the login shell (`command` null) and send `addPreset`, closing the form on `done` and showing the refusal in the form otherwise. A blank name, a name of more than 64 characters, a name already taken, or a command of more than 4096 characters SHALL be refused in the form without sending anything; Escape SHALL close the form without sending anything and without closing the picker. While more than one preset exists, each SHALL offer a remove control sending `removePreset` for it; a refusal SHALL be shown on the page. While more than one preset exists, each SHALL be draggable to another position, the gap it would be dropped into marked while dragging, and dropping it elsewhere SHALL send `movePreset` with its new position; Alt+Up and Alt+Down SHALL move the highlighted preset one position, which stays highlighted and focused; until the new order arrives, or the move fails, further keyboard moves SHALL be ignored, so each moves the preset that is highlighted. Every list SHALL show the presets the hub last sent, and the picker SHALL stay open while presets are edited. +The preset picker and a worktree's preset choices SHALL offer "+ Add preset", opening a form for a name and a command; submitting SHALL trim both, treat an empty command as the login shell (`command` null) and send `addPreset`, closing the form on `done` and showing the refusal in the form otherwise. A blank name, a name of more than 64 characters, a name already taken, or a command of more than 4096 characters SHALL be refused in the form without sending anything; Escape SHALL close the form without sending anything and without closing the picker. While more than one preset exists, each SHALL offer a remove control sending `removePreset` for it; a refusal SHALL be shown on the page. While more than one preset exists, each SHALL be draggable to another position, the gap it would be dropped into marked while dragging, and dropping it elsewhere SHALL send `movePreset` with its new position; in the picker, Alt+Up and Alt+Down SHALL move the highlighted preset one position, which stays highlighted and focused; until the new order arrives, or the move fails, further keyboard moves SHALL be ignored, so each moves the preset that is highlighted. Every list SHALL show the presets the hub last sent, and the picker SHALL stay open while presets are edited. #### Scenario: Preset added from the choices - **WHEN** the user adds a preset `echo` with a command in a worktree's preset choices while another page is open diff --git a/src/web/ui/app.tsx b/src/web/ui/app.tsx index 7f971b0..702eef0 100644 --- a/src/web/ui/app.tsx +++ b/src/web/ui/app.tsx @@ -7,7 +7,7 @@ import { followArea, type TerminalManager } from '../terminals/index.js'; import { hasBanner, hostAction, statusText } from './hosts.js'; import { hubLineText, hubsMenu, hubStep, initialHubState, type HubAction, type HubLine, type HubTarget } from './hubs.js'; import { aggregate, markTitle, worktreeTitle, type Aggregate } from './marks.js'; -import { dropPosition, pickerKey, presetFromForm } from './picker.js'; +import { choicesTakeFocus, dropPosition, pickerKey, presetFromForm } from './picker.js'; import { SIDEBAR_WIDTH, type SidebarEntry } from './sidebar.js'; import type { RepoTab } from './repo-tabs.js'; import { @@ -691,6 +691,7 @@ const PresetList = ({ cancel, remove, reorder, + keyReorder, focusFirst, }: { presets: readonly Preset[]; @@ -702,6 +703,8 @@ const PresetList = ({ remove: (name: string) => void; /** Moves the named preset to position `to`, by dragging or Alt+Up/Down; resolves with whether the hub did. */ reorder: (name: string, to: number) => Promise; + /** Whether Alt+Up/Down move the highlighted preset; otherwise they stay the page's worktree shortcuts. */ + keyReorder: boolean; /** Whether the list takes the keyboard focus when shown; it follows the highlighted option once it has it. */ focusFirst: boolean; }) => { @@ -740,6 +743,7 @@ const PresetList = ({ ref={list} onKeyDown={(event) => { if (event.altKey && (event.key === 'ArrowUp' || event.key === 'ArrowDown')) { + if (!keyReorder) return; event.preventDefault(); event.stopPropagation(); moveBy(event.key === 'ArrowUp' ? -1 : 1); @@ -956,17 +960,24 @@ const PresetPicker = ({ view, presets }: { view: View; presets: readonly Preset[ view.removePreset(name); }} reorder={(name, to) => view.movePreset(name, to)} + keyReorder /> ); }; -/** The presets a worktree without terminals offers; with `emptied`, they take a keyboard focus left on the page body. */ -const PresetChoices = ({ view, presets, emptied }: { view: View; presets: readonly Preset[]; emptied: boolean }) => { +/** The presets a worktree without terminals offers; on showing, they take the keyboard focus as `choicesTakeFocus` says. */ +const PresetChoices = ({ manager, view, presets }: { manager: TerminalManager; view: View; presets: readonly Preset[] }) => { const [index, setIndex] = useState(0); - // Decided at mount, after the closed terminal's xterm left the document. - const [focus] = useState(() => emptied && (document.activeElement === null || document.activeElement === document.body)); + const [focus] = useState(() => { + const active = document.activeElement; + const focused = + active instanceof HTMLElement + ? { tagName: active.tagName, type: active.getAttribute('type'), isContentEditable: active.isContentEditable } + : null; + return choicesTakeFocus(focused, manager.owns(active), view.modal.peek()); + }); return (
Start a terminal
@@ -984,6 +995,7 @@ const PresetChoices = ({ view, presets, emptied }: { view: View; presets: readon view.removePreset(name); }} reorder={(name, to) => view.movePreset(name, to)} + keyReorder={false} />
@@ -1061,13 +1073,9 @@ const TerminalArea = ({ client, manager, view }: AppProps) => { const presets = client.store.presets.value; const focused = view.focused.value; const empty = view.termTabs.value.tabs.length === 0; - const worktree = view.worktree.value; - /** The worktree and whether it was empty at the last render. */ - const last = useRef<{ worktree: string | null; empty: boolean }>({ worktree, empty }); - const emptied = empty && last.current.worktree === worktree && !last.current.empty; - useLayoutEffect(() => { - last.current = { worktree, empty }; - }); + const tab = view.tab.value; + // Each worktree gets its own choices, which take the focus on showing. + const choicesKey = `${tab === null ? '' : repoKey(tab.host, tab.repo)}\n${view.worktree.value ?? ''}`; return (
{view.panes.value.map((pane) => ( @@ -1082,7 +1090,9 @@ const TerminalArea = ({ client, manager, view }: AppProps) => { {view.dividers.value.map((divider) => ( ))} - {empty && view.canCreate.value && presets !== null && } + {empty && view.canCreate.value && presets !== null && ( + + )} {presets !== null && }
); diff --git a/src/web/ui/picker.test.ts b/src/web/ui/picker.test.ts index 58f68f9..afb509c 100644 --- a/src/web/ui/picker.test.ts +++ b/src/web/ui/picker.test.ts @@ -1,6 +1,15 @@ import { describe, expect, it } from 'vitest'; import type { Layout } from '../../protocol/index.js'; -import { dropPosition, pickerKey, pickerValid, presetFromForm, type PickerContext, type PickerWorld } from './picker.js'; +import { + choicesTakeFocus, + dropPosition, + pickerKey, + pickerValid, + presetFromForm, + type FocusedLike, + type PickerContext, + type PickerWorld, +} from './picker.js'; type Pane = Layout['tabs'][number]['root']; @@ -151,3 +160,42 @@ describe('dropPosition', () => { expect(dropPosition(from, slot)).toBe(position); }); }); + +describe('choicesTakeFocus', () => { + const el = (tagName: string, type: string | null = null, isContentEditable = false): FocusedLike => ({ + tagName, + type, + isContentEditable, + }); + + it.each<[string, FocusedLike | null]>([ + ['nothing', null], + ['the page body', el('BODY')], + ['a sidebar button', el('BUTTON')], + ['a checkbox', el('INPUT', 'checkbox')], + ['a focusable div', el('DIV')], + ])('takes the focus from %s', (_name, focused) => { + expect(choicesTakeFocus(focused, false, false)).toBe(true); + }); + + it.each<[string, FocusedLike]>([ + ['a text input without a type', el('INPUT')], + ['a text input', el('INPUT', 'text')], + ['a search input', el('INPUT', 'search')], + ['a textarea', el('TEXTAREA')], + ['a select', el('SELECT')], + ['an editable element', el('DIV', null, true)], + ])('leaves the focus in %s', (_name, focused) => { + expect(choicesTakeFocus(focused, false, false)).toBe(false); + }); + + it('takes the focus from a terminal, whose input is a textarea', () => { + expect(choicesTakeFocus(el('TEXTAREA'), true, false)).toBe(true); + }); + + it('leaves the focus while a picker or dialog owns the keys', () => { + expect(choicesTakeFocus(null, false, true)).toBe(false); + expect(choicesTakeFocus(el('BODY'), false, true)).toBe(false); + expect(choicesTakeFocus(el('TEXTAREA'), true, true)).toBe(false); + }); +}); diff --git a/src/web/ui/picker.ts b/src/web/ui/picker.ts index 485f0b1..f5d7583 100644 --- a/src/web/ui/picker.ts +++ b/src/web/ui/picker.ts @@ -53,6 +53,29 @@ export const pickerKey = (key: string, index: number, count: number): PickerKey } }; +/** The parts of the focused element `choicesTakeFocus` reads. */ +export interface FocusedLike { + tagName: string; + /** The `type` attribute, if any. */ + type: string | null; + isContentEditable: boolean; +} + +const CHOICE_TYPES = new Set(['checkbox', 'radio', 'button', 'submit', 'reset']); + +/** Whether a worktree's preset choices take the keyboard focus on showing: not from a dialog, nor from a text field outside the terminals. */ +export const choicesTakeFocus = (focused: FocusedLike | null, inTerminal: boolean, modal: boolean): boolean => { + if (modal) return false; + if (focused === null || inTerminal) return true; + const tag = focused.tagName.toUpperCase(); + const field = + focused.isContentEditable || + tag === 'TEXTAREA' || + tag === 'SELECT' || + (tag === 'INPUT' && !CHOICE_TYPES.has(focused.type?.toLowerCase() ?? '')); + return !field; +}; + const MAX_PRESET_NAME = 64; const MAX_PRESET_COMMAND = 4096; diff --git a/test/e2e/terminals.spec.ts b/test/e2e/terminals.spec.ts index 8b8503e..7fed2f7 100644 --- a/test/e2e/terminals.spec.ts +++ b/test/e2e/terminals.spec.ts @@ -1,6 +1,6 @@ import type { Page } from '@playwright/test'; import { decodeMessage } from '../../src/protocol/index.js'; -import { byTestId, pane, terminalBox, termTab, TID } from './contract.js'; +import { byTestId, pane, terminalBox, termTab, TID, worktreeEntry } from './contract.js'; import { activeTerm, confirmClose, @@ -12,6 +12,7 @@ import { newTerminal, openPage, openUi, + presetRequests, PRESETS, ready, screenOf, @@ -132,6 +133,71 @@ test.describe('terminal tabs', () => { expect(wire.sentToHost(LOCAL).filter((m) => m.t === 'createTerm')).toEqual([]); }); + test('selecting a worktree without terminals focuses its choices, and Down then Enter starts the second preset', async ({ + hub, + page, + wire, + }) => { + const { repo, worktrees } = makeRepoWith(hub, 'app', ['feat']); + const feat = worktrees[0] ?? ''; + hub.writeRepos([repo]); + await openUi(hub, page); + await selectWorktree(page, feat); + const options = page.locator(byTestId(TID.presetChoices)).locator(byTestId(TID.presetOption)); + await expect(options.nth(0)).toBeFocused(); + + await page.keyboard.press('ArrowDown'); + await expect(options.nth(1)).toHaveAttribute('aria-selected', 'true'); + await expect(options.nth(1)).toBeFocused(); + const mark = wire.markSent(); + await page.keyboard.press('Enter'); + await expect.poll(() => createsSent(wire, mark)).toEqual([expect.objectContaining({ worktree: feat, preset: PRESETS[1].name })]); + }); + + test('Alt+Down from focused choices selects the next worktree, whose choices take the focus from the first preset', async ({ + hub, + page, + wire, + }) => { + const { repo, worktrees } = makeRepoWith(hub, 'app', ['a', 'b']); + const [a = '', b = ''] = worktrees; + hub.writeRepos([repo]); + await openUi(hub, page); + await selectWorktree(page, a); + const options = page.locator(byTestId(TID.presetChoices)).locator(byTestId(TID.presetOption)); + await page.keyboard.press('ArrowDown'); + await expect(options.nth(1)).toBeFocused(); + + const mark = wire.markSent(); + await page.keyboard.press('Alt+ArrowDown'); + await expect(page.locator(worktreeEntry(b))).toHaveAttribute('aria-selected', 'true'); + await expect(options.nth(0)).toBeFocused(); + await expect(options.nth(0)).toHaveAttribute('aria-selected', 'true'); + await expect(options).toHaveText(PRESETS.map((p) => p.name)); + expect(presetRequests(wire, mark)).toEqual([]); + await page.keyboard.press('1'); + await expect.poll(() => createsSent(wire, mark)).toEqual([expect.objectContaining({ worktree: b, preset: PRESETS[0].name })]); + }); + + test('selecting a worktree without terminals by Alt+Down from the worktree search leaves the focus in the search', async ({ + hub, + page, + }) => { + const { repo, worktrees } = makeRepoWith(hub, 'app', ['a', 'b']); + const [a = '', b = ''] = worktrees; + hub.writeRepos([repo]); + await openUi(hub, page); + await selectWorktree(page, a); + const search = page.locator(byTestId(TID.worktreeSearch)); + await search.focus(); + + await page.keyboard.press('Alt+ArrowDown'); + await expect(page.locator(worktreeEntry(b))).toHaveAttribute('aria-selected', 'true'); + await expect(page.locator(byTestId(TID.presetChoices))).toBeVisible(); + await page.waitForTimeout(500); // NOSONAR(S2925) — absence check: nothing to synchronise on + await expect(search).toBeFocused(); + }); + test('a live terminal missing from the layout is shown as a tab', async ({ hub, page }) => { const { repo, worktrees } = makeRepoWith(hub, 'app', ['feat']); const feat = worktrees[0] ?? '';