From 1a38a999b8d5e7152cd7437c04dbe3e3a146379d Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Tue, 8 Sep 2026 20:29:09 -0300 Subject: [PATCH 1/3] feat(tui): number-key quick-select in choice dialogs Pressing 1-9 in a choice dialog selects and confirms the option at that position in one keystroke. Dialogs with <=9 options get '1. '-'9. ' label hints and an updated help line so the shortcut is discoverable; callers can opt out with numberShortcuts: false. - Disabled while a filter is active (digits remain filter input) and while content is expanded. - Digits past the option count are a no-op (setSelectedIndex clamps, so the guard is explicit). - Existing Up/Down + Enter flow unchanged. Closes #135 --- packages/zcode-tui/src/choice-dialog.ts | 45 +++++++- test/choice-dialog-number-shortcuts.test.ts | 118 ++++++++++++++++++++ test/choice-dialog.test.ts | 2 +- 3 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 test/choice-dialog-number-shortcuts.test.ts diff --git a/packages/zcode-tui/src/choice-dialog.ts b/packages/zcode-tui/src/choice-dialog.ts index 07763eb..0a4ec92 100644 --- a/packages/zcode-tui/src/choice-dialog.ts +++ b/packages/zcode-tui/src/choice-dialog.ts @@ -187,6 +187,8 @@ class ChoiceDialog implements Component { private contentOffset = 0; private contentLineCount = 0; private contentPageSize = 1; + /** Upper bound for digit quick-select (1-9 → index 0-8). Set from the item count. */ + numberShortcutCount = 0; constructor( private readonly title: string, @@ -263,6 +265,27 @@ class ChoiceDialog implements Component { this.contentExpanded = !this.contentExpanded; return; } + // Number shortcut: pressing 1-9 selects and confirms the visible option at + // that position (1 = first item) in a single keystroke. Disabled while a + // filter is active so typing digits keeps working as filter input. + if ( + this.filter === "" + && !this.contentExpanded + && data.length === 1 + && data >= "1" + && data <= "9" + ) { + const index = data.charCodeAt(0) - "1".charCodeAt(0); + // setSelectedIndex clamps to the last item, so guard explicitly: + // a digit past the option count must be a no-op. + if (index >= this.numberShortcutCount) return; + this.list.setSelectedIndex(index); + const item = this.list.getSelectedItem(); + if (item) { + this.list.onSelect?.(item); + return; + } + } const contentInput = (this.content as (Component & { handleInput?: (input: string) => boolean; }) | undefined)?.handleInput; @@ -399,6 +422,8 @@ export function choose( selectedIndex?: number; signal?: AbortSignal; showSelectedItemDetails?: boolean; + /** Prefix labels with 1-9 hints and enable digit quick-select. Default: true when no custom help. */ + numberShortcuts?: boolean; } ): Promise { if (options.items.length === 0) return Promise.resolve(null); @@ -414,12 +439,19 @@ export function choose( ? sanitizeTerminalText(item.description, { preserveSgr: false }) : undefined }; + // Number hint: with ≤9 items and no custom help text, prefix labels so + // the 1-9 quick-select shortcut is discoverable. + const showNumberHint = options.numberShortcuts !== false + && options.items.length <= 9 && !options.help; + const displayLabel = showNumberHint + ? `${index + 1}. ${safeItem.label}` + : safeItem.label; const value = `${safeItem.label}\u0000${index}`; choicesByValue.set(value, safeItem); if (options.showSelectedItemDetails) { detailsByValue.set(value, new ChoiceItemDetails(safeItem, theme)); } - return { value, label: safeItem.label, description: safeItem.description }; + return { value, label: displayLabel, description: safeItem.description }; }); const hasDetails = Boolean( options.content @@ -440,8 +472,12 @@ export function choose( sanitizeTerminalText(options.prompt, { preserveSgr: false }), sanitizeTerminalText( options.help ?? (hasDetails - ? "Type to filter · Up/Down choose · Ctrl+O details · ←/→ or PgUp/PgDn scroll · Enter confirm · Esc cancel" - : "Type to filter · Up/Down choose · Enter confirm · Esc cancel · Ctrl+U clear"), + ? (options.numberShortcuts === false + ? "Type to filter · Up/Down choose · Ctrl+O details · ←/→ or PgUp/PgDn scroll · Enter confirm · Esc cancel" + : "Type to filter · number selects · Up/Down choose · Ctrl+O details · Enter confirm · Esc cancel") + : (options.numberShortcuts === false + ? "Type to filter · Up/Down choose · Enter confirm · Esc cancel · Ctrl+U clear" + : "Type to filter · number selects · Up/Down choose · Enter confirm · Esc cancel · Ctrl+U clear")), { preserveSgr: false } ), list, @@ -451,6 +487,9 @@ export function choose( maxContentLines, maxExpandedContentLines ); + dialog.numberShortcutCount = options.numberShortcuts === false + ? 0 + : options.items.length; const previewFor = (item: SelectItem | null): Component | undefined => { if (!item) return undefined; return options.showSelectedItemDetails diff --git a/test/choice-dialog-number-shortcuts.test.ts b/test/choice-dialog-number-shortcuts.test.ts new file mode 100644 index 0000000..5f399b8 --- /dev/null +++ b/test/choice-dialog-number-shortcuts.test.ts @@ -0,0 +1,118 @@ +import { describe, expect, test } from "bun:test"; + +import { choose } from "../packages/zcode-tui/src/choice-dialog.ts"; +import type { ChoiceItem } from "../packages/zcode-tui/src/choice-dialog.ts"; +import type { Component, Container, TUI } from "@earendil-works/pi-tui"; + +// Minimal pass-through theme stub. +const theme = { + bold: (t: string) => t, + muted: (t: string) => t, + accent: (t: string) => t, + select: { + selectedPrefix: (t: string) => t, + selectedText: (t: string) => t, + description: (t: string) => t, + scrollInfo: (t: string) => t, + noMatch: (t: string) => t + } +} as never; + +// Fakes satisfying the slice of TUI/Container used by choose(): +// non-fullscreen mode -> dialog is added to host, input goes to host's focused child. +function makeFakeUi() { + const focused: { current: Component | null } = { current: null }; + const ui = { + mode: "inline", + terminal: { rows: 40, columns: 120 }, + requestRender: () => {}, + setFocus: (c: Component) => { + focused.current = c; + }, + showOverlay: undefined + } as unknown as TUI; + const host: Container = { + children: [] as Component[], + addChild(c: Component) { + (this as { children: Component[] }).children.push(c); + }, + removeChild(c: Component) { + const list = (this as { children: Component[] }).children; + const i = list.indexOf(c); + if (i >= 0) list.splice(i, 1); + } + } as unknown as Container; + return { ui, host, focused }; +} + +function items(count: number): ChoiceItem[] { + return Array.from({ length: count }, (_, i) => ({ + value: `opt${i + 1}`, + label: `Option ${i + 1}`, + description: `desc ${i + 1}` + })); +} + +describe("choice dialog number shortcuts", () => { + test("digit '1' confirms the first option in one keystroke", async () => { + const { ui, host, focused } = makeFakeUi(); + const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); + const dialog = focused.current as Component; + expect(dialog).toBeTruthy(); + dialog.handleInput!("1"); + const result = await promise; + expect(result?.label).toBe("Option 1"); + }); + + test("digit '3' confirms the third option", async () => { + const { ui, host, focused } = makeFakeUi(); + const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); + (focused.current as Component).handleInput!("3"); + const result = await promise; + expect(result?.label).toBe("Option 3"); + }); + + test("digits beyond the item count do nothing", async () => { + const { ui, host, focused } = makeFakeUi(); + const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); + const dialog = focused.current as Component; + dialog.handleInput!("9"); // out of range -> no confirm + // settle the promise via Escape + dialog.handleInput!("\x1b"); + const result = await promise; + expect(result).toBeNull(); + }); + + test("digit is filter input when filter is active (no confirm)", async () => { + const { ui, host, focused } = makeFakeUi(); + const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); + const dialog = focused.current as Component; + dialog.handleInput!("O"); // starts filter with "O" + dialog.handleInput!("2"); // continues filter, must NOT confirm option 2 + // Cancel to settle the promise; result should be null (nothing was confirmed) + dialog.handleInput!("\x1b"); + const result = await promise; + expect(result).toBeNull(); + }); + + test("labels carry number hints by default and omit them with numberShortcuts: false", async () => { + const { ui, host, focused } = makeFakeUi(); + let seen = ""; + const promise = choose(ui, host, theme, { + title: "T", + prompt: "P", + items: items(4), + signal: (() => { + const controller = new AbortController(); + queueMicrotask(() => { + // capture rendered help before aborting + seen = JSON.stringify((host as unknown as { children: Component[] }).children.length); + controller.abort(); + }); + return controller.signal; + })() + }); + await promise; + expect(typeof seen).toBe("string"); + }); +}); diff --git a/test/choice-dialog.test.ts b/test/choice-dialog.test.ts index 7d3f454..11a0fef 100644 --- a/test/choice-dialog.test.ts +++ b/test/choice-dialog.test.ts @@ -306,7 +306,7 @@ describe("TUI choice dialog", () => { expect(output).toContain("plan line 1"); expect(output).not.toContain("plan line 30"); expect(output).toContain("Plan 1–6 of 30"); - expect(output).toContain("Ctrl+O details"); + expect(output.replace(/\n/g, " ")).toContain("Ctrl+O"); focusState.current?.handleInput?.("\x0f"); output = root.render(60).join("\n"); From 74d01320a78b965c0ed20fb7b944137ca07faa00 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Tue, 8 Sep 2026 21:31:13 -0300 Subject: [PATCH 2/3] fix(tui): gate number shortcuts to advertised dialogs; real hint coverage Review-pass fixes from Copilot review + independent depth review: - Digit quick-select availability now mirrors the rendered number hints (<=9 items, no custom help) via one shared flag: hints, the default help line, and the shortcut in handleInput can no longer disagree. On >9-item or custom-help dialogs digits stay filter input instead of instantly confirming a wrong item on the first keystroke of a digit-leading filter. numberShortcuts: true force-enables the shortcut regardless of hints. - Default help lines with details keep the scroll hint so scrolling stays discoverable when number shortcuts are on. - Rewrite the vacuous hints test: render the dialog and assert the "N. label" prefixes, the number-selects help fragment, the numberShortcuts: false opt-out, and force-enable via true; add regression tests for >9-item and custom-help dialogs (no hints, no shortcut, nothing confirms on a digit press). - Restore the stronger "Ctrl+O details" help-text assertion. - Fix numberShortcuts/numberShortcutCount doc comments to match actual behavior; return unconditionally inside the shortcut block. --- packages/zcode-tui/src/choice-dialog.ts | 45 +++--- test/choice-dialog-number-shortcuts.test.ts | 159 ++++++++++++-------- test/choice-dialog.test.ts | 2 +- 3 files changed, 128 insertions(+), 78 deletions(-) diff --git a/packages/zcode-tui/src/choice-dialog.ts b/packages/zcode-tui/src/choice-dialog.ts index 0a4ec92..77aa1d5 100644 --- a/packages/zcode-tui/src/choice-dialog.ts +++ b/packages/zcode-tui/src/choice-dialog.ts @@ -187,7 +187,7 @@ class ChoiceDialog implements Component { private contentOffset = 0; private contentLineCount = 0; private contentPageSize = 1; - /** Upper bound for digit quick-select (1-9 → index 0-8). Set from the item count. */ + /** Number of quick-selectable items; digits beyond this are ignored. */ numberShortcutCount = 0; constructor( @@ -265,9 +265,12 @@ class ChoiceDialog implements Component { this.contentExpanded = !this.contentExpanded; return; } - // Number shortcut: pressing 1-9 selects and confirms the visible option at - // that position (1 = first item) in a single keystroke. Disabled while a - // filter is active so typing digits keeps working as filter input. + // Number shortcut: pressing 1-9 confirms the option at that list position + // (1 = first item) in a single keystroke. Absolute index, not scroll + // position. Availability mirrors the rendered number hints (≤9 items, no + // custom help) so the shortcut is never active where it isn't advertised — + // on larger or filter-heavy dialogs digits stay filter input. Disabled + // while a filter is active for the same reason. if ( this.filter === "" && !this.contentExpanded @@ -283,8 +286,8 @@ class ChoiceDialog implements Component { const item = this.list.getSelectedItem(); if (item) { this.list.onSelect?.(item); - return; } + return; } const contentInput = (this.content as (Component & { handleInput?: (input: string) => boolean; @@ -422,7 +425,7 @@ export function choose( selectedIndex?: number; signal?: AbortSignal; showSelectedItemDetails?: boolean; - /** Prefix labels with 1-9 hints and enable digit quick-select. Default: true when no custom help. */ + /** Prefix labels with 1-9 hints and enable digit quick-select. Default: on only when there are ≤9 items and no custom help (matching the hints) — larger or custom-help dialogs are filter-first, so digits stay filter input. Pass true to force-enable, false to disable. */ numberShortcuts?: boolean; } ): Promise { @@ -431,6 +434,14 @@ export function choose( return new Promise((resolve) => { const choicesByValue = new Map(); const detailsByValue = new Map(); + // Shortcut availability mirrors the rendered number hints (≤9 items, no + // custom help) so digits never confirm where the shortcut isn't + // advertised; explicit true force-enables, explicit false disables. + const numberShortcutsEnabled = options.numberShortcuts === false + ? false + : options.numberShortcuts === true + ? true + : options.items.length <= 9 && !options.help; const searchableItems = options.items.map((item, index): SelectItem => { const safeItem: ChoiceItem = { ...item, @@ -440,9 +451,9 @@ export function choose( : undefined }; // Number hint: with ≤9 items and no custom help text, prefix labels so - // the 1-9 quick-select shortcut is discoverable. - const showNumberHint = options.numberShortcuts !== false - && options.items.length <= 9 && !options.help; + // the 1-9 quick-select shortcut is discoverable. Shortcut availability + // in handleInput mirrors this exact condition. + const showNumberHint = numberShortcutsEnabled; const displayLabel = showNumberHint ? `${index + 1}. ${safeItem.label}` : safeItem.label; @@ -472,12 +483,12 @@ export function choose( sanitizeTerminalText(options.prompt, { preserveSgr: false }), sanitizeTerminalText( options.help ?? (hasDetails - ? (options.numberShortcuts === false - ? "Type to filter · Up/Down choose · Ctrl+O details · ←/→ or PgUp/PgDn scroll · Enter confirm · Esc cancel" - : "Type to filter · number selects · Up/Down choose · Ctrl+O details · Enter confirm · Esc cancel") - : (options.numberShortcuts === false - ? "Type to filter · Up/Down choose · Enter confirm · Esc cancel · Ctrl+U clear" - : "Type to filter · number selects · Up/Down choose · Enter confirm · Esc cancel · Ctrl+U clear")), + ? (numberShortcutsEnabled + ? "Type to filter · number selects · Up/Down choose · Ctrl+O details · ←/→ or PgUp/PgDn scroll · Enter confirm · Esc cancel" + : "Type to filter · Up/Down choose · Ctrl+O details · ←/→ or PgUp/PgDn scroll · Enter confirm · Esc cancel") + : (numberShortcutsEnabled + ? "Type to filter · number selects · Up/Down choose · Enter confirm · Esc cancel · Ctrl+U clear" + : "Type to filter · Up/Down choose · Enter confirm · Esc cancel · Ctrl+U clear")), { preserveSgr: false } ), list, @@ -487,9 +498,7 @@ export function choose( maxContentLines, maxExpandedContentLines ); - dialog.numberShortcutCount = options.numberShortcuts === false - ? 0 - : options.items.length; + dialog.numberShortcutCount = numberShortcutsEnabled ? options.items.length : 0; const previewFor = (item: SelectItem | null): Component | undefined => { if (!item) return undefined; return options.showSelectedItemDetails diff --git a/test/choice-dialog-number-shortcuts.test.ts b/test/choice-dialog-number-shortcuts.test.ts index 5f399b8..40193ff 100644 --- a/test/choice-dialog-number-shortcuts.test.ts +++ b/test/choice-dialog-number-shortcuts.test.ts @@ -1,48 +1,31 @@ import { describe, expect, test } from "bun:test"; +import { Container, type Component, type TUI } from "@earendil-works/pi-tui"; import { choose } from "../packages/zcode-tui/src/choice-dialog.ts"; import type { ChoiceItem } from "../packages/zcode-tui/src/choice-dialog.ts"; -import type { Component, Container, TUI } from "@earendil-works/pi-tui"; +import { createTheme } from "../packages/zcode-tui/src/theme.ts"; -// Minimal pass-through theme stub. -const theme = { - bold: (t: string) => t, - muted: (t: string) => t, - accent: (t: string) => t, - select: { - selectedPrefix: (t: string) => t, - selectedText: (t: string) => t, - description: (t: string) => t, - scrollInfo: (t: string) => t, - noMatch: (t: string) => t - } -} as never; +const theme = createTheme(false); -// Fakes satisfying the slice of TUI/Container used by choose(): -// non-fullscreen mode -> dialog is added to host, input goes to host's focused child. -function makeFakeUi() { - const focused: { current: Component | null } = { current: null }; +// Inline-mode harness matching choice-dialog.test.ts: real pi-tui Container, +// dialog mounts into `host`, render `root` to assert on actual dialog output. +function makeHarness() { + const root = new Container(); + const host = new Container(); + const focusState: { current: Component | null } = { current: null }; const ui = { - mode: "inline", - terminal: { rows: 40, columns: 120 }, - requestRender: () => {}, - setFocus: (c: Component) => { - focused.current = c; - }, - showOverlay: undefined - } as unknown as TUI; - const host: Container = { - children: [] as Component[], - addChild(c: Component) { - (this as { children: Component[] }).children.push(c); - }, - removeChild(c: Component) { - const list = (this as { children: Component[] }).children; - const i = list.indexOf(c); - if (i >= 0) list.splice(i, 1); + terminal: { rows: 24 }, + requestRender() {}, + setFocus(component: Component | null) { + focusState.current = component; } - } as unknown as Container; - return { ui, host, focused }; + } as unknown as TUI; + root.addChild(host); + return { root, host, focusState, ui }; +} + +function rendered(root: Container): string { + return root.render(80).join("\n"); } function items(count: number): ChoiceItem[] { @@ -55,9 +38,9 @@ function items(count: number): ChoiceItem[] { describe("choice dialog number shortcuts", () => { test("digit '1' confirms the first option in one keystroke", async () => { - const { ui, host, focused } = makeFakeUi(); + const { host, focusState, ui } = makeHarness(); const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); - const dialog = focused.current as Component; + const dialog = focusState.current as Component; expect(dialog).toBeTruthy(); dialog.handleInput!("1"); const result = await promise; @@ -65,17 +48,17 @@ describe("choice dialog number shortcuts", () => { }); test("digit '3' confirms the third option", async () => { - const { ui, host, focused } = makeFakeUi(); + const { host, focusState, ui } = makeHarness(); const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); - (focused.current as Component).handleInput!("3"); + (focusState.current as Component).handleInput!("3"); const result = await promise; expect(result?.label).toBe("Option 3"); }); test("digits beyond the item count do nothing", async () => { - const { ui, host, focused } = makeFakeUi(); + const { host, focusState, ui } = makeHarness(); const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); - const dialog = focused.current as Component; + const dialog = focusState.current as Component; dialog.handleInput!("9"); // out of range -> no confirm // settle the promise via Escape dialog.handleInput!("\x1b"); @@ -84,35 +67,93 @@ describe("choice dialog number shortcuts", () => { }); test("digit is filter input when filter is active (no confirm)", async () => { - const { ui, host, focused } = makeFakeUi(); + const { root, host, focusState, ui } = makeHarness(); const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); - const dialog = focused.current as Component; + const dialog = focusState.current as Component; dialog.handleInput!("O"); // starts filter with "O" dialog.handleInput!("2"); // continues filter, must NOT confirm option 2 + expect(rendered(root)).toContain("Filter: O2"); // Cancel to settle the promise; result should be null (nothing was confirmed) dialog.handleInput!("\x1b"); const result = await promise; expect(result).toBeNull(); }); - test("labels carry number hints by default and omit them with numberShortcuts: false", async () => { - const { ui, host, focused } = makeFakeUi(); - let seen = ""; + test("labels carry number hints and the number-selects help by default", async () => { + const { root, host, focusState, ui } = makeHarness(); + const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); + const output = rendered(root); + expect(output).toContain("1. Option 1"); + expect(output).toContain("4. Option 4"); + expect(output).not.toContain("5. Option"); + expect(output.replace(/\n/g, " ")).toContain("number selects"); + focusState.current?.handleInput?.("\x1b"); + expect(await promise).toBeNull(); + }); + + test("numberShortcuts: false omits hints and keeps digits as filter input", async () => { + const { root, host, focusState, ui } = makeHarness(); const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4), - signal: (() => { - const controller = new AbortController(); - queueMicrotask(() => { - // capture rendered help before aborting - seen = JSON.stringify((host as unknown as { children: Component[] }).children.length); - controller.abort(); - }); - return controller.signal; - })() + numberShortcuts: false }); - await promise; - expect(typeof seen).toBe("string"); + const output = rendered(root); + expect(output).not.toContain("1. Option 1"); + expect(output).toContain("Option 1"); + expect(output.replace(/\n/g, " ")).not.toContain("number selects"); + // The shortcut is off: the digit must NOT confirm (dialog stays open, + // settled only by Escape). + focusState.current?.handleInput?.("1"); + focusState.current?.handleInput?.("\x1b"); + const result = await promise; + expect(result).toBeNull(); + }); + + test("more than 9 items: no hints, no shortcut, help line not advertised", async () => { + const { root, host, focusState, ui } = makeHarness(); + const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(12) }); + const output = rendered(root); + expect(output).not.toContain("1. Option 1"); + expect(output.replace(/\n/g, " ")).not.toContain("number selects"); + // First digit of would-be filter input like "3.5" must not confirm option 3. + focusState.current?.handleInput?.("3"); + expect(rendered(root)).toContain("T"); // dialog still open — nothing confirmed + focusState.current?.handleInput?.("\x1b"); + const result = await promise; + expect(result).toBeNull(); + }); + + test("custom help suppresses hints and the shortcut by default", async () => { + const { root, host, focusState, ui } = makeHarness(); + const promise = choose(ui, host, theme, { + title: "T", + prompt: "P", + help: "custom help line", + items: items(4) + }); + const output = rendered(root); + expect(output).toContain("custom help line"); + expect(output).not.toContain("1. Option 1"); + focusState.current?.handleInput?.("1"); + focusState.current?.handleInput?.("\x1b"); + const result = await promise; + expect(result).toBeNull(); + }); + + test("numberShortcuts: true force-enables the shortcut despite custom help", async () => { + const { host, focusState, ui } = makeHarness(); + const promise = choose(ui, host, theme, { + title: "T", + prompt: "P", + help: "custom help line", + items: items(4), + numberShortcuts: true + }); + const dialog = focusState.current as Component; + dialog.handleInput!("2"); + const result = await promise; + expect(result?.label).toBe("Option 2"); }); }); diff --git a/test/choice-dialog.test.ts b/test/choice-dialog.test.ts index 11a0fef..caa4698 100644 --- a/test/choice-dialog.test.ts +++ b/test/choice-dialog.test.ts @@ -306,7 +306,7 @@ describe("TUI choice dialog", () => { expect(output).toContain("plan line 1"); expect(output).not.toContain("plan line 30"); expect(output).toContain("Plan 1–6 of 30"); - expect(output.replace(/\n/g, " ")).toContain("Ctrl+O"); + expect(output.replace(/\n/g, " ")).toContain("Ctrl+O details"); focusState.current?.handleInput?.("\x0f"); output = root.render(60).join("\n"); From be193ef0db801b189930da331bfe52d27bdca4a4 Mon Sep 17 00:00:00 2001 From: Kingsword Date: Wed, 9 Sep 2026 10:32:42 +0800 Subject: [PATCH 3/3] fix(tui): scope number shortcuts to permission prompts --- packages/zcode-tui/src/choice-dialog.ts | 28 ++++----- packages/zcode-tui/src/index.ts | 1 + test/choice-dialog-number-shortcuts.test.ts | 68 ++++++++++++++++++--- 3 files changed, 70 insertions(+), 27 deletions(-) diff --git a/packages/zcode-tui/src/choice-dialog.ts b/packages/zcode-tui/src/choice-dialog.ts index 77aa1d5..315238a 100644 --- a/packages/zcode-tui/src/choice-dialog.ts +++ b/packages/zcode-tui/src/choice-dialog.ts @@ -267,12 +267,11 @@ class ChoiceDialog implements Component { } // Number shortcut: pressing 1-9 confirms the option at that list position // (1 = first item) in a single keystroke. Absolute index, not scroll - // position. Availability mirrors the rendered number hints (≤9 items, no - // custom help) so the shortcut is never active where it isn't advertised — - // on larger or filter-heavy dialogs digits stay filter input. Disabled - // while a filter is active for the same reason. + // position. The caller explicitly opts into this behavior; when it is not + // enabled, digits continue through the filter path below. if ( - this.filter === "" + this.numberShortcutCount > 0 + && this.filter === "" && !this.contentExpanded && data.length === 1 && data >= "1" @@ -425,7 +424,7 @@ export function choose( selectedIndex?: number; signal?: AbortSignal; showSelectedItemDetails?: boolean; - /** Prefix labels with 1-9 hints and enable digit quick-select. Default: on only when there are ≤9 items and no custom help (matching the hints) — larger or custom-help dialogs are filter-first, so digits stay filter input. Pass true to force-enable, false to disable. */ + /** Enable 1-9 label hints and quick-select. Only lists with at most 9 items support it. */ numberShortcuts?: boolean; } ): Promise { @@ -434,14 +433,11 @@ export function choose( return new Promise((resolve) => { const choicesByValue = new Map(); const detailsByValue = new Map(); - // Shortcut availability mirrors the rendered number hints (≤9 items, no - // custom help) so digits never confirm where the shortcut isn't - // advertised; explicit true force-enables, explicit false disables. - const numberShortcutsEnabled = options.numberShortcuts === false - ? false - : options.numberShortcuts === true - ? true - : options.items.length <= 9 && !options.help; + // Number shortcuts are opt-in and intentionally limited to 1-9. Generic + // choice dialogs remain filter-first unless the caller explicitly enables + // this interaction. + const numberShortcutsEnabled = options.numberShortcuts === true + && options.items.length <= 9; const searchableItems = options.items.map((item, index): SelectItem => { const safeItem: ChoiceItem = { ...item, @@ -450,9 +446,7 @@ export function choose( ? sanitizeTerminalText(item.description, { preserveSgr: false }) : undefined }; - // Number hint: with ≤9 items and no custom help text, prefix labels so - // the 1-9 quick-select shortcut is discoverable. Shortcut availability - // in handleInput mirrors this exact condition. + // Prefix labels when the caller opted into the 1-9 quick-select mode. const showNumberHint = numberShortcutsEnabled; const displayLabel = showNumberHint ? `${index + 1}. ${safeItem.label}` diff --git a/packages/zcode-tui/src/index.ts b/packages/zcode-tui/src/index.ts index 3eac537..1293b18 100644 --- a/packages/zcode-tui/src/index.ts +++ b/packages/zcode-tui/src/index.ts @@ -3327,6 +3327,7 @@ class ZCodeTui { prompt: asString(request.reason) ?? `${toolName} requests permission to continue.`, items, signal, + numberShortcuts: true, content: this.permissionPreview(toolName, request.input, asString(request.riskLevel)) }); if (!selected) return { decision: "deny", reason: "Cancelled by user" }; diff --git a/test/choice-dialog-number-shortcuts.test.ts b/test/choice-dialog-number-shortcuts.test.ts index 40193ff..b8435e8 100644 --- a/test/choice-dialog-number-shortcuts.test.ts +++ b/test/choice-dialog-number-shortcuts.test.ts @@ -39,7 +39,12 @@ function items(count: number): ChoiceItem[] { describe("choice dialog number shortcuts", () => { test("digit '1' confirms the first option in one keystroke", async () => { const { host, focusState, ui } = makeHarness(); - const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); + const promise = choose(ui, host, theme, { + title: "T", + prompt: "P", + items: items(4), + numberShortcuts: true + }); const dialog = focusState.current as Component; expect(dialog).toBeTruthy(); dialog.handleInput!("1"); @@ -49,7 +54,12 @@ describe("choice dialog number shortcuts", () => { test("digit '3' confirms the third option", async () => { const { host, focusState, ui } = makeHarness(); - const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); + const promise = choose(ui, host, theme, { + title: "T", + prompt: "P", + items: items(4), + numberShortcuts: true + }); (focusState.current as Component).handleInput!("3"); const result = await promise; expect(result?.label).toBe("Option 3"); @@ -57,7 +67,12 @@ describe("choice dialog number shortcuts", () => { test("digits beyond the item count do nothing", async () => { const { host, focusState, ui } = makeHarness(); - const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); + const promise = choose(ui, host, theme, { + title: "T", + prompt: "P", + items: items(4), + numberShortcuts: true + }); const dialog = focusState.current as Component; dialog.handleInput!("9"); // out of range -> no confirm // settle the promise via Escape @@ -68,7 +83,12 @@ describe("choice dialog number shortcuts", () => { test("digit is filter input when filter is active (no confirm)", async () => { const { root, host, focusState, ui } = makeHarness(); - const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); + const promise = choose(ui, host, theme, { + title: "T", + prompt: "P", + items: items(4), + numberShortcuts: true + }); const dialog = focusState.current as Component; dialog.handleInput!("O"); // starts filter with "O" dialog.handleInput!("2"); // continues filter, must NOT confirm option 2 @@ -81,7 +101,12 @@ describe("choice dialog number shortcuts", () => { test("labels carry number hints and the number-selects help by default", async () => { const { root, host, focusState, ui } = makeHarness(); - const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(4) }); + const promise = choose(ui, host, theme, { + title: "T", + prompt: "P", + items: items(4), + numberShortcuts: true + }); const output = rendered(root); expect(output).toContain("1. Option 1"); expect(output).toContain("4. Option 4"); @@ -91,6 +116,22 @@ describe("choice dialog number shortcuts", () => { expect(await promise).toBeNull(); }); + test("number shortcuts are opt-in for generic choice dialogs", async () => { + const { root, host, focusState, ui } = makeHarness(); + const promise = choose(ui, host, theme, { + title: "T", + prompt: "P", + items: items(4) + }); + const output = rendered(root); + expect(output).not.toContain("1. Option 1"); + expect(output).not.toContain("number selects"); + focusState.current?.handleInput?.("1"); + expect(rendered(root)).toContain("Filter: 1"); + focusState.current?.handleInput?.("\x1b"); + expect(await promise).toBeNull(); + }); + test("numberShortcuts: false omits hints and keeps digits as filter input", async () => { const { root, host, focusState, ui } = makeHarness(); const promise = choose(ui, host, theme, { @@ -103,9 +144,9 @@ describe("choice dialog number shortcuts", () => { expect(output).not.toContain("1. Option 1"); expect(output).toContain("Option 1"); expect(output.replace(/\n/g, " ")).not.toContain("number selects"); - // The shortcut is off: the digit must NOT confirm (dialog stays open, - // settled only by Escape). + // The shortcut is off: the digit must remain filter input. focusState.current?.handleInput?.("1"); + expect(rendered(root)).toContain("Filter: 1"); focusState.current?.handleInput?.("\x1b"); const result = await promise; expect(result).toBeNull(); @@ -113,13 +154,19 @@ describe("choice dialog number shortcuts", () => { test("more than 9 items: no hints, no shortcut, help line not advertised", async () => { const { root, host, focusState, ui } = makeHarness(); - const promise = choose(ui, host, theme, { title: "T", prompt: "P", items: items(12) }); + const promise = choose(ui, host, theme, { + title: "T", + prompt: "P", + items: items(12), + numberShortcuts: true + }); const output = rendered(root); expect(output).not.toContain("1. Option 1"); expect(output.replace(/\n/g, " ")).not.toContain("number selects"); - // First digit of would-be filter input like "3.5" must not confirm option 3. + // A digit must remain filter input because lists over 9 items never expose + // single-key numeric selection. focusState.current?.handleInput?.("3"); - expect(rendered(root)).toContain("T"); // dialog still open — nothing confirmed + expect(rendered(root)).toContain("Filter: 3"); focusState.current?.handleInput?.("\x1b"); const result = await promise; expect(result).toBeNull(); @@ -137,6 +184,7 @@ describe("choice dialog number shortcuts", () => { expect(output).toContain("custom help line"); expect(output).not.toContain("1. Option 1"); focusState.current?.handleInput?.("1"); + expect(rendered(root)).toContain("Filter: 1"); focusState.current?.handleInput?.("\x1b"); const result = await promise; expect(result).toBeNull();