From bd079eef6ae9ef67f483a232565fd596e2e253b1 Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 05:53:33 +0200 Subject: [PATCH] feat(coding-agent/modes): enabled mouse-hover highlighting for plan review options - Handled SGR mouse motion reports in `PlanReviewOverlay` to set hovered options from hit-test rows and clear highlights outside option rows. - Updated option rendering to paint a background band on hovered, non-disabled options while preserving keyboard selection behavior. - Enabled any-motion mouse tracking for overlays and expanded regression coverage to verify hover tracking setup and teardown. --- .../modes/components/plan-review-overlay.ts | 31 +++++++++-- .../components/plan-review-overlay.test.ts | 52 ++++++++++++++++++- packages/tui/src/tui.ts | 12 +++-- packages/tui/test/render-regressions.test.ts | 4 +- 4 files changed, 87 insertions(+), 12 deletions(-) diff --git a/packages/coding-agent/src/modes/components/plan-review-overlay.ts b/packages/coding-agent/src/modes/components/plan-review-overlay.ts index ef74ae7ea..dd692d7c8 100644 --- a/packages/coding-agent/src/modes/components/plan-review-overlay.ts +++ b/packages/coding-agent/src/modes/components/plan-review-overlay.ts @@ -141,6 +141,9 @@ export class PlanReviewOverlay implements Component { #bodyClickRows = new Set(); /** 1-based column at/under which a region-row click targets the sidebar. */ #sidebarClickMaxCol = 0; + /** Option index the pointer is currently hovering, or undefined. Updated from + * motion mouse reports and cleared when the pointer leaves the option rows. */ + #hoveredOption: number | undefined; #annotating = false; #input: Input; @@ -315,9 +318,10 @@ export class PlanReviewOverlay implements Component { * Hit-test an SGR mouse report (`\x1b[ { const selected = i === this.#selectedIndex; const isDisabled = this.#disabled.has(i); + const hovered = !isDisabled && i === this.#hoveredOption; // The cursor marks the selected option; it dims when actions are not the // focused region so the active region's highlight stays unambiguous. const cursor = selected ? theme.fg(active ? "accent" : "dim", `${theme.nav.cursor} `) : " "; - const text = isDisabled + let text = isDisabled ? theme.fg("dim", label) : selected && active ? theme.bold(theme.fg("accent", label)) : theme.fg("text", label); + // A pointer hovering an option paints a highlight band behind its label, + // distinct from the keyboard selection (cursor glyph + bold accent) which + // stays where it is. One space of padding gives the band a button shape. + if (hovered) text = theme.bg("selectedBg", ` ${text} `); return cursor + text; }); } diff --git a/packages/coding-agent/test/modes/components/plan-review-overlay.test.ts b/packages/coding-agent/test/modes/components/plan-review-overlay.test.ts index 7bfea1265..9e1d03f2b 100644 --- a/packages/coding-agent/test/modes/components/plan-review-overlay.test.ts +++ b/packages/coding-agent/test/modes/components/plan-review-overlay.test.ts @@ -3,7 +3,7 @@ import { stripVTControlCharacters } from "node:util"; import { KeybindingsManager } from "@oh-my-pi/pi-coding-agent/config/keybindings"; import type { HookSelectorSlider } from "@oh-my-pi/pi-coding-agent/modes/components/hook-selector"; import { PlanReviewOverlay } from "@oh-my-pi/pi-coding-agent/modes/components/plan-review-overlay"; -import { getThemeByName, setThemeInstance } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import { getThemeByName, setThemeInstance, theme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import { setKeybindings } from "@oh-my-pi/pi-tui"; const UP = "\x1b[A"; @@ -444,4 +444,54 @@ describe("PlanReviewOverlay", () => { overlay.handleInput(SHIFT_DOWN); // Shift+Down — fastScrollLines (5) at once expect(firstRow() - base).toBe(5); }); + + // SGR button 35 = no-button motion (0x20 motion flag | 0x03 no-button): the + // hover report a terminal sends while the pointer moves with no button held. + const hoverRow = (overlay: PlanReviewOverlay, needle: string, col = 6): boolean => { + const lines = overlay.render(80); + const row = lines.findIndex(line => stripVTControlCharacters(line).includes(needle)); + if (row < 0) return false; + overlay.handleInput(`\x1b[<35;${col};${row + 1}M`); + return true; + }; + + const optionLineRaw = (overlay: PlanReviewOverlay, needle: string): string | undefined => + overlay.render(80).find(line => stripVTControlCharacters(line).includes(needle)); + + it("paints a hover band on the option the pointer is over and clears it on leave", () => { + const onPick = vi.fn(); + const overlay = new PlanReviewOverlay( + "plan body text", + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick, onCancel: vi.fn() }, + ); + const selectedBg = theme.getBgAnsi("selectedBg"); + render(overlay); // populate the click maps before hit-testing + + // Hover a non-selected option (selection rests on index 0). + expect(optionLineRaw(overlay, "Approve and keep context")).not.toContain(selectedBg); + expect(hoverRow(overlay, "Approve and keep context")).toBe(true); + expect(optionLineRaw(overlay, "Approve and keep context")).toContain(selectedBg); + + // Hover is visual only: the keyboard cursor stays on index 0, so Enter still + // confirms the first option rather than the hovered one. + overlay.handleInput(ENTER); + expect(onPick).toHaveBeenCalledWith("Approve and execute"); + + // Pointer onto the top border (a non-option row) drops the highlight. + overlay.handleInput("\x1b[<35;6;1M"); + expect(optionLineRaw(overlay, "Approve and keep context")).not.toContain(selectedBg); + }); + + it("never hovers a disabled option", () => { + const overlay = new PlanReviewOverlay( + "plan body text", + { promptTitle: "next", options: APPROVAL_OPTIONS, disabledIndices: [2] }, + { onPick: vi.fn(), onCancel: vi.fn() }, + ); + const selectedBg = theme.getBgAnsi("selectedBg"); + render(overlay); + expect(hoverRow(overlay, "Approve and keep context")).toBe(true); + expect(optionLineRaw(overlay, "Approve and keep context")).not.toContain(selectedBg); + }); }); diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index 2af7f90fc..a8ad9ed67 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -74,11 +74,13 @@ const CURSOR_BEGIN = `${HIDE_CURSOR}${SYNC_OUTPUT_BEGIN}`; const CURSOR_BEGIN_NO_SYNC = HIDE_CURSOR; const CURSOR_END = SYNC_OUTPUT_END; const CURSOR_END_NO_SYNC = ""; -// Mouse reporting (normal click tracking + SGR extended coordinates), enabled -// only for the lifetime of a fullscreen overlay so the rest of the app keeps the -// terminal's native text selection. -const MOUSE_TRACKING_ON = "\x1b[?1000h\x1b[?1006h"; -const MOUSE_TRACKING_OFF = "\x1b[?1006l\x1b[?1000l"; +// Mouse reporting, enabled only for the lifetime of a fullscreen overlay so the +// rest of the app keeps the terminal's native text selection. 1000h = button +// click tracking, 1003h = any-motion tracking so overlays can light up hover +// targets (the pointer moving with no button held), 1006h = SGR extended +// coordinates so columns/rows past 223 are reported. +const MOUSE_TRACKING_ON = "\x1b[?1000h\x1b[?1003h\x1b[?1006h"; +const MOUSE_TRACKING_OFF = "\x1b[?1006l\x1b[?1003l\x1b[?1000l"; type InputListenerResult = { consume?: boolean; data?: string } | undefined; type InputListener = (data: string) => InputListenerResult; diff --git a/packages/tui/test/render-regressions.test.ts b/packages/tui/test/render-regressions.test.ts index 29ff72ace..24c97879c 100644 --- a/packages/tui/test/render-regressions.test.ts +++ b/packages/tui/test/render-regressions.test.ts @@ -3286,8 +3286,9 @@ describe("TUI terminal-state regressions", () => { const modalWrites = writes.slice(showFrom).join(""); // Borrowed the alternate screen buffer … expect(modalWrites).toContain("\x1b[?1049h"); - // … enabled mouse tracking for click/scroll support … + // … enabled mouse tracking for click/scroll/hover support … expect(modalWrites).toContain("\x1b[?1000h"); + expect(modalWrites).toContain("\x1b[?1003h"); // any-motion tracking drives hover expect(modalWrites).toContain("\x1b[?1006h"); // … and never erased scrollback (ED3) or otherwise touched the transcript. expect(modalWrites).not.toContain("\x1b[3J"); @@ -3301,6 +3302,7 @@ describe("TUI terminal-state regressions", () => { expect(hideWrites).toContain("\x1b[?1049l"); // Mouse tracking is disabled again so the rest of the app keeps native // terminal selection. + expect(hideWrites).toContain("\x1b[?1003l"); // motion tracking torn down too expect(hideWrites).toContain("\x1b[?1000l"); // Transcript is back on the normal screen after leaving the alt buffer. expect(visible(term).some(line => line.includes("base-"))).toBeTrue();