diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d1722f2ec..4617fc4cb 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the multi-select (checkbox) `ask` picker showing no visible cursor on themes where `accent` fg is close to the terminal foreground; the focused option now renders as a full-width `selectedBg` highlight band spanning label and wrapped description rows, matching the Ctrl+R history overlay ([#4157](https://github.com/can1357/oh-my-pi/issues/4157)). + ## [16.2.12] - 2026-07-01 ### Breaking Changes diff --git a/packages/coding-agent/src/modes/components/hook-selector.ts b/packages/coding-agent/src/modes/components/hook-selector.ts index 600907290..f99bb07bd 100644 --- a/packages/coding-agent/src/modes/components/hook-selector.ts +++ b/packages/coding-agent/src/modes/components/hook-selector.ts @@ -113,11 +113,23 @@ function splitLeadingSpacesForWrap(line: string, width: number): { indent: strin }; } -class OutlinedList extends Container { - #lines: string[] = []; +/** One row fed to {@link OutlinedList} or the plain list container. `highlight` + * causes the row (and its wrapped continuations, plus trailing padding) to be + * painted with the theme's `selectedBg` band — the focus cue that survives + * themes where `accent` fg is close to the terminal foreground. */ +type SelectorRow = { text: string; highlight: boolean }; - setLines(lines: string[]): void { - this.#lines = lines; +/** Paint `content` with the `selectedBg` background, applied AFTER any inner + * ANSI styling so the band spans padding as well as content. */ +function paintSelectedRow(content: string): string { + return theme.bg("selectedBg", content); +} + +class OutlinedList extends Container { + #rows: SelectorRow[] = []; + + setLines(rows: readonly SelectorRow[]): void { + this.#rows = rows.slice(); this.invalidate(); } @@ -126,15 +138,17 @@ class OutlinedList extends Container { const horizontal = borderColor(theme.boxRound.horizontal.repeat(Math.max(1, width))); const innerWidth = Math.max(1, width - 2); const content: string[] = []; - for (const line of this.#lines) { - const normalized = replaceTabs(line); + for (const row of this.#rows) { + const normalized = replaceTabs(row.text); const { indent, body } = splitLeadingSpacesForWrap(normalized, innerWidth); const wrapped = wrapTextWithAnsi(body, Math.max(1, innerWidth - visibleWidth(indent))); for (const wrappedBody of wrapped.length > 0 ? wrapped : [""]) { const wrappedLine = `${indent}${wrappedBody}`; const pad = Math.max(0, innerWidth - visibleWidth(wrappedLine)); + const filled = `${wrappedLine}${padding(pad)}`; + const painted = row.highlight ? paintSelectedRow(filled) : filled; content.push( - `${borderColor(theme.boxRound.vertical)}${wrappedLine}${padding(pad)}${borderColor(theme.boxRound.vertical)}`, + `${borderColor(theme.boxRound.vertical)}${painted}${borderColor(theme.boxRound.vertical)}`, ); } } @@ -472,7 +486,7 @@ export class HookSelectorComponent extends Container { } #updateList(renderWidth = this.#lastRenderWidth): void { - const lines: string[] = []; + const rows: SelectorRow[] = []; const total = this.#filteredOptions.length; const mdTheme = getMarkdownTheme(); // Compact mode kicks in exactly when the fully-expanded list (all @@ -500,34 +514,41 @@ export class HookSelectorComponent extends Container { const filtered = this.#filteredOptions[i]; if (filtered === undefined) continue; const isSelected = i === this.#selectedIndex; + const isDisabled = this.#isDisabled(filtered.index); const descMode: number | "full" = compact ? (isSelected ? selectedDescRows : 0) : "full"; - lines.push( - ...this.#renderOptionLines( - filtered.option, - isSelected, - this.#isDisabled(filtered.index), - mdTheme, - descMode, - renderWidth, - filtered.index, - ), - ); + // Highlight the whole option block (label + wrapped description rows) + // so the focus band reads as one continuous bar rather than a stripe + // under the label alone. Disabled rows never claim focus even if the + // index momentarily lands on one during initial coercion. + const highlight = isSelected && !isDisabled; + for (const text of this.#renderOptionLines( + filtered.option, + isSelected, + isDisabled, + mdTheme, + descMode, + renderWidth, + filtered.index, + )) { + rows.push({ text, highlight }); + } } if (total === 0) { - lines.push(theme.fg("dim", " No matching options")); + rows.push({ text: theme.fg("dim", " No matching options"), highlight: false }); } if (startIndex > 0 || endIndex < total || this.#shouldRenderSearchStatus(renderWidth, mdTheme)) { - lines.push(this.#renderStatusLine(total)); + rows.push({ text: this.#renderStatusLine(total), highlight: false }); } if (this.#outlinedList) { - this.#outlinedList.setLines(lines); + this.#outlinedList.setLines(rows); return; } this.#listContainer?.clear(); - for (const line of lines) { - this.#listContainer?.addChild(new Text(line, 1, 0)); + for (const row of rows) { + const bgFn = row.highlight ? paintSelectedRow : undefined; + this.#listContainer?.addChild(new Text(row.text, 1, 0, bgFn)); } } diff --git a/packages/coding-agent/test/hook-selector-overflow.test.ts b/packages/coding-agent/test/hook-selector-overflow.test.ts index ee20c689a..0b2e0d9bd 100644 --- a/packages/coding-agent/test/hook-selector-overflow.test.ts +++ b/packages/coding-agent/test/hook-selector-overflow.test.ts @@ -298,4 +298,115 @@ describe("HookSelectorComponent", () => { expect(done).not.toContain(theme.checkbox.checked); expect(done).not.toContain(theme.checkbox.unchecked); }); + + it("paints a selectedBg focus band across the highlighted checkbox row (outlined)", () => { + // Bug #4157: multi-select checkbox picker used only an accent fg to signal + // focus, which vanished on themes where accent ≈ text. The focused row + // must now carry the selectedBg band regardless of accent/text contrast. + const component = new HookSelectorComponent( + "Pick many", + ["Apple", "Banana", "Cherry", "Done selecting", "Other (type your own)"], + () => {}, + () => {}, + { + outline: true, + selectionMarker: "checkbox", + markableCount: 3, + checkedIndices: [], + initialIndex: 0, + }, + ); + const bgProbe = theme.bg("selectedBg", "|"); + const openBg = bgProbe.slice(0, bgProbe.indexOf("|")); + + const rendered = component.render(80); + const appleRow = rendered.find(line => Bun.stripANSI(line).includes("Apple")); + const bananaRow = rendered.find(line => Bun.stripANSI(line).includes("Banana")); + const otherRow = rendered.find(line => Bun.stripANSI(line).includes("Other")); + expect(appleRow).toBeDefined(); + expect(bananaRow).toBeDefined(); + expect(otherRow).toBeDefined(); + expect(appleRow).toContain(openBg); + expect(bananaRow).not.toContain(openBg); + expect(otherRow).not.toContain(openBg); + }); + + it("moves the selectedBg band with the cursor and paints control rows too", () => { + const build = (initialIndex: number) => + new HookSelectorComponent( + "Pick many", + ["Apple", "Banana", "Cherry", "Done selecting", "Other (type your own)"], + () => {}, + () => {}, + { + outline: true, + selectionMarker: "checkbox", + markableCount: 3, + checkedIndices: [], + initialIndex, + }, + ); + const bgProbe = theme.bg("selectedBg", "|"); + const openBg = bgProbe.slice(0, bgProbe.indexOf("|")); + const bandedLabel = (component: HookSelectorComponent) => { + const rendered = component.render(80); + const banded = rendered.filter(line => line.includes(openBg)); + const labels = ["Apple", "Banana", "Cherry", "Done selecting", "Other"]; + return labels.find(label => banded.some(line => Bun.stripANSI(line).includes(label))); + }; + expect(bandedLabel(build(1))).toBe("Banana"); + // Control rows past markableCount ("Other") still receive the focus band + // even though they carry no checkbox marker. + expect(bandedLabel(build(4))).toBe("Other"); + }); + + it("highlights the whole selected block including wrapped description rows", () => { + const component = new HookSelectorComponent( + "Choose", + [ + { + label: "Alpha", + description: + "Detailed first-choice explanation long enough to wrap across multiple rendered rows once outlined inside a tight width.", + }, + { label: "Beta", description: "Second." }, + ], + () => {}, + () => {}, + { outline: true, initialIndex: 0 }, + ); + const bgProbe = theme.bg("selectedBg", "|"); + const openBg = bgProbe.slice(0, bgProbe.indexOf("|")); + + const rendered = component.render(60); + const bandedRows = rendered.filter(line => line.includes(openBg)); + // Label row plus at least one wrapped description continuation, all under + // the Alpha option; Beta must stay unbanded. + expect(bandedRows.length).toBeGreaterThanOrEqual(2); + const bandedContainsBeta = bandedRows.some(line => Bun.stripANSI(line).includes("Beta")); + expect(bandedContainsBeta).toBe(false); + }); + + it("paints the selectedBg band on the non-outlined plain list too", () => { + const component = new HookSelectorComponent( + "Pick", + ["Apple", "Banana", "Cherry"], + () => {}, + () => {}, + { initialIndex: 1 }, + ); + const bgProbe = theme.bg("selectedBg", "|"); + const openBg = bgProbe.slice(0, bgProbe.indexOf("|")); + + const rendered = component.render(80); + const appleRow = rendered.find(line => Bun.stripANSI(line).includes("Apple")); + const bananaRow = rendered.find(line => Bun.stripANSI(line).includes("Banana")); + const cherryRow = rendered.find(line => Bun.stripANSI(line).includes("Cherry")); + expect(appleRow).toBeDefined(); + expect(bananaRow).toBeDefined(); + expect(cherryRow).toBeDefined(); + expect(bananaRow).toContain(openBg); + expect(appleRow).not.toContain(openBg); + expect(cherryRow).not.toContain(openBg); + }); });