diff --git a/.changeset/active-pane-dividers.md b/.changeset/active-pane-dividers.md new file mode 100644 index 000000000..ffe2ca547 --- /dev/null +++ b/.changeset/active-pane-dividers.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": minor +--- + +Highlight active built-in and extension panes with accent-weighted separators. diff --git a/src/extensions/default/ui/index.test.ts b/src/extensions/default/ui/index.test.ts index 1d925701d..3bde0e67f 100644 --- a/src/extensions/default/ui/index.test.ts +++ b/src/extensions/default/ui/index.test.ts @@ -10,7 +10,7 @@ describe("bundled UI registry", () => { expect(reviewInfo).toMatchObject({ placement: "top", defaultOpen: true, - height: { preferred: 3, min: 3, max: 3 }, + height: { preferred: 2, min: 2, max: 2 }, }); expect( reviewInfo.available?.({ diff --git a/src/extensions/default/ui/reviewInfo/index.test.tsx b/src/extensions/default/ui/reviewInfo/index.test.tsx index 7728fb99f..4e34ada6e 100644 --- a/src/extensions/default/ui/reviewInfo/index.test.tsx +++ b/src/extensions/default/ui/reviewInfo/index.test.tsx @@ -36,7 +36,7 @@ function backgroundsAtColumn( } describe("ReviewInfoPane", () => { - test("separates review chrome with an accent rail and panel background", async () => { + test("paints review metadata with an accent rail and panel background", async () => { const theme = resolveTheme("github-dark-default", null); const width = 30; const setup = await testRender( @@ -44,11 +44,11 @@ describe("ReviewInfoPane", () => { {...({ review, width, - height: 3, + height: 2, theme, } as unknown as ExtensionPaneProps)} />, - { width, height: 3 }, + { width, height: 2 }, ); try { @@ -56,27 +56,21 @@ describe("ReviewInfoPane", () => { await setup.renderOnce(); }); expect(backgroundsAtColumn(setup, 0)).toEqual([ - theme.panel.toLowerCase(), theme.accent.toLowerCase(), theme.accent.toLowerCase(), ]); expect(backgroundsAtColumn(setup, 1)).toEqual([ theme.panel.toLowerCase(), theme.panel.toLowerCase(), - theme.panel.toLowerCase(), ]); expect(backgroundsAtColumn(setup, width - 1)).toEqual([ theme.panel.toLowerCase(), theme.panel.toLowerCase(), - theme.panel.toLowerCase(), ]); expect(backgroundsAtColumn(setup, 1)).not.toContain(theme.panelAlt.toLowerCase()); const [primary, secondary] = reviewInfoLines(review, width - 3); const frame = setup.captureCharFrame(); - expect(frame.split("\n")[0]).toBe("─".repeat(width)); - const borderSpan = setup.captureSpans().lines[0]?.spans.find((span) => span.width > 0); - expect(capturedTestColorToHex(borderSpan?.fg)).toBe(theme.border.toLowerCase()); expect(frame).toContain(` ${primary}`); expect(frame).toContain(` ${secondary}`); } finally { @@ -84,25 +78,25 @@ describe("ReviewInfoPane", () => { } }); - test("keeps the border deterministic when no metadata text fits", async () => { + test("keeps the rail deterministic when no metadata text fits", async () => { const theme = resolveTheme("github-dark-default", null); const setup = await testRender( , - { width: 1, height: 3 }, + { width: 1, height: 2 }, ); try { await act(async () => { await setup.renderOnce(); }); - expect(setup.captureCharFrame().split("\n").slice(0, 3)).toEqual(["─", " ", " "]); + expect(setup.captureCharFrame().split("\n").slice(0, 2)).toEqual([" ", " "]); } finally { setup.renderer.destroy(); } diff --git a/src/extensions/default/ui/reviewInfo/index.tsx b/src/extensions/default/ui/reviewInfo/index.tsx index 5478bc762..ea4bb0827 100644 --- a/src/extensions/default/ui/reviewInfo/index.tsx +++ b/src/extensions/default/ui/reviewInfo/index.tsx @@ -13,14 +13,11 @@ export function ReviewInfoPane({ review, theme, width }: ExtensionPaneProps): Re - - {"─".repeat(Math.max(0, width))} - { id: BUNDLED_REVIEW_INFO_VIEW_ID, title: "Review info", placement: "top", - height: { preferred: 3, min: 3, max: 3 }, + height: { preferred: 2, min: 2, max: 2 }, defaultOpen: true, available: ({ review }) => review?.kind === "change-request", component: ReviewInfoPane, diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 6ff7452df..0db0edf89 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -3,6 +3,7 @@ import type { MouseEvent as TuiMouseEvent, ScrollBoxRenderable, } from "@opentui/core"; +import { MouseButton } from "@opentui/core"; import { useRenderer, useTerminalDimensions } from "@opentui/react"; import { Suspense, @@ -108,6 +109,7 @@ import { setMouseCapture } from "./lib/mouseCapture"; import { openSelectedFileInEditor } from "./lib/openInEditor"; import { resolveResponsiveLayout } from "./lib/responsive"; import type { WorkspaceRefreshRequest } from "./currentReviewRefresh"; +import { useActivePaneController } from "./hooks/useActivePaneController"; type FocusArea = "files" | "filter" | "note"; @@ -480,6 +482,12 @@ export function App({ pagerMode, responsiveShowsSidebar: responsiveLayout.showSidebar, }); + const visiblePaneKeys = useMemo( + () => paneLayout.panes.map(({ pane }) => pane.key), + [paneLayout.panes], + ); + const { activePaneKey, activatePane, activateReview, paneSurfaceRef, reviewSurfaceRef } = + useActivePaneController({ renderer, visiblePaneKeys }); useEffect(() => { if (resizingPaneKey === null) { @@ -1199,6 +1207,7 @@ export function App({ return ( reportPaneRenderFailure(pane) } @@ -1246,37 +1256,39 @@ export function App({ // so a fast motion or a sidebar projection swap cannot transfer the gesture to a transient row. const beginCapturedPaneResize = (planned: PlannedPane, event: TuiMouseEvent) => { if (!beginPaneResize(planned, event)) return; + activatePane(planned.pane.key); if (paneResizeCaptureRef.current) { setMouseCapture(renderer, paneResizeCaptureRef.current); } closeMenu(); }; - const renderDivider = (planned: PlannedPane) => - planned.divider ? ( - - beginCapturedPaneResize(planned, event)} - onMouseDrag={updatePaneResize} - onMouseDragEnd={endPaneResize} - onMouseUp={endPaneResize} - /> - - ) : null; + const renderDivider = (planned: PlannedPane) => ( + + beginCapturedPaneResize(planned, event)} + onMouseDrag={updatePaneResize} + onMouseDragEnd={endPaneResize} + onMouseUp={endPaneResize} + /> + + ); return ( { + if (event.button === MouseButton.LEFT) activateReview(); + }} > { join(extension, "index.tsx"), `import { createElement, useState } from "react"; export default function (hunk) { + let showPrompt = () => {}; hunk.registerPane({ id: "prompt", placement: "bottom", defaultOpen: true, height: { preferred: 3, min: 3, max: 3 }, component: () => { + const [visible, setVisible] = useState(false); const [value, setValue] = useState(""); - return createElement("input", { value, focused: true, onInput: setValue }); + showPrompt = () => setVisible(true); + return visible + ? createElement("input", { value, focused: true, onInput: setValue }) + : createElement("text", { content: "PROMPT CLOSED" }); }, }); + hunk.registerCommand({ id: "focus", title: "Focus prompt", key: "o" }, () => showPrompt()); hunk.registerCommand({ id: "letter", title: "Letter command", key: "j" }, (ctx) => { ctx.notify("COMMAND FIRED"); }); @@ -325,6 +331,9 @@ export default function (hunk) { }); try { + await waitForFrame(setup, () => setup.captureCharFrame().includes("PROMPT CLOSED"), 12); + expect(setup.renderer.currentFocusedEditor).toBeNull(); + await act(async () => setup.mockInput.typeText("o")); await waitForFrame(setup, () => setup.renderer.currentFocusedEditor !== null, 12); expect(setup.renderer.currentFocusedEditor).not.toBeNull(); @@ -333,6 +342,7 @@ export default function (hunk) { const frame = setup.captureCharFrame(); expect(frame).toContain("j?"); + expect(frame).toContain("━"); expect(frame).not.toContain("COMMAND FIRED"); expect(frame).not.toContain("Controls help"); } finally { diff --git a/src/ui/AppHost.review-metadata.test.tsx b/src/ui/AppHost.review-metadata.test.tsx index 74d4aa194..1d2bfcf90 100644 --- a/src/ui/AppHost.review-metadata.test.tsx +++ b/src/ui/AppHost.review-metadata.test.tsx @@ -108,6 +108,32 @@ async function flushUntil( } describe("delegated review metadata reloads", () => { + test("gives the fixed built-in review pane an active host separator", async () => { + const fixture = await createTestBootstrap(); + const setup = await testRender(, { + width: 100, + height: 12, + }); + try { + await flushUntil( + setup, + () => setup.captureCharFrame().includes("OPEN · #123 · Metadata pane"), + "the delegated review pane to mount", + ); + expect(setup.captureCharFrame().split("\n")[3]).toContain("─"); + + await act(async () => setup.mockMouse.click(50, 1)); + await flushUntil( + setup, + () => setup.captureCharFrame().split("\n")[3]?.includes("━") === true, + "the fixed pane separator to activate", + ); + } finally { + await act(async () => setup.renderer.destroy()); + rmSync(fixture.directory, { recursive: true, force: true }); + } + }); + test("the bundled review pane occupies exactly three rows only for delegated change requests", async () => { const delegated = await createTestBootstrap(); const ordinary = await createTestBootstrap(); diff --git a/src/ui/AppHost.sidebar-resize.test.tsx b/src/ui/AppHost.sidebar-resize.test.tsx index f0e905e10..78692d057 100644 --- a/src/ui/AppHost.sidebar-resize.test.tsx +++ b/src/ui/AppHost.sidebar-resize.test.tsx @@ -69,7 +69,17 @@ async function flush(setup: Awaited>) { /** Column of the vertical sidebar/diff divider on the probe row, or -1 when absent. */ function dividerColumn(setup: Awaited>) { const row = setup.captureCharFrame().split("\n")[PROBE_ROW] ?? ""; - return row.indexOf("│"); + const normal = row.indexOf("│"); + const active = row.indexOf("┃"); + if (normal < 0) return active; + if (active < 0) return normal; + return Math.min(normal, active); +} + +/** Read the sidebar divider's normal or emphasized vertical glyph. */ +function dividerGlyph(setup: Awaited>) { + const row = setup.captureCharFrame().split("\n")[PROBE_ROW] ?? ""; + return row[dividerColumn(setup)] ?? ""; } /** Return only the file-sidebar columns so diff headers cannot satisfy sidebar assertions. */ @@ -139,6 +149,20 @@ afterEach(() => { }); describe("AppHost sidebar resize", () => { + test("moves active emphasis between the built-in files pane and review", async () => { + setup = await testRender(, WIDE); + await flush(setup); + expect(dividerGlyph(setup)).toBe("│"); + + await act(async () => setup!.mockMouse.click(8, PROBE_ROW)); + await flush(setup); + expect(dividerGlyph(setup)).toBe("┃"); + + await act(async () => setup!.mockMouse.click(INITIAL_DIVIDER_COLUMN + 8, PROBE_ROW)); + await flush(setup); + expect(dividerGlyph(setup)).toBe("│"); + }); + test("resizes the default sidebar with the terminal until the user drags it", async () => { setup = await testRender(, WIDE); await flush(setup); @@ -166,6 +190,7 @@ describe("AppHost sidebar resize", () => { // The divider follows the new width: startWidth + (currentX - originX). expect(dividerColumn(setup)).toBeGreaterThan(INITIAL_DIVIDER_COLUMN); + expect(dividerGlyph(setup)).toBe("┃"); }); test("resizing across the content-width threshold switches the file projection", async () => { diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index fe8b6f44c..43dfdaa7d 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -352,6 +352,7 @@ export function DiffPane({ onFocusDraftNote, onCopyFeedback, onCopySelectionText, + onActivateSurface, onFileViewRowFailure, onScrollCodeHorizontally = () => {}, onSelectFile, @@ -423,6 +424,7 @@ export function DiffPane({ onFocusDraftNote?: () => void; onCopyFeedback?: (text: string) => void; onCopySelectionText?: (text: string) => void | boolean; + onActivateSurface?: () => void; onFileViewRowFailure?: (failure: FileViewRowFailure) => void; onScrollCodeHorizontally?: (delta: number) => void; onSelectFile: (fileId: string) => void; @@ -449,6 +451,8 @@ export function DiffPane({ const hoveredFileIdRef = useRef(null); const onActiveAddNoteAffordanceChangeRef = useRef(onActiveAddNoteAffordanceChange); onActiveAddNoteAffordanceChangeRef.current = onActiveAddNoteAffordanceChange; + const onActivateSurfaceRef = useRef(onActivateSurface); + onActivateSurfaceRef.current = onActivateSurface; /** Hide hover-only row controls when content scrolls under a stationary mouse pointer. */ const clearAddNoteHoverForScroll = useCallback(() => { @@ -1512,6 +1516,7 @@ export function DiffPane({ if (event.button !== MouseButton.LEFT) { return; } + onActivateSurfaceRef.current?.(); const point = resolveCopySelectionPoint(event); if (!point) { @@ -2679,6 +2684,9 @@ export function DiffPane({ ? { paddingY: 1 } : { paddingTop: 0, paddingBottom: pagerMode ? 0 : 1 }), }} + onMouseDown={(event) => { + if (event.button === MouseButton.LEFT) onActivateSurfaceRef.current?.(); + }} onMouseDragEnd={endCopySelection} onMouseUp={endCopySelection} > diff --git a/src/ui/components/panes/ExtensionPane.test.tsx b/src/ui/components/panes/ExtensionPane.test.tsx index 3afe6cbc6..941883a5d 100644 --- a/src/ui/components/panes/ExtensionPane.test.tsx +++ b/src/ui/components/panes/ExtensionPane.test.tsx @@ -168,6 +168,7 @@ describe("ExtensionPaneHost activation", () => { const files = createTestFiles(); const theme = resolveTheme("github-dark-default", null); let activations = 0; + const surfaceActivations: string[] = []; let childPresses = 0; const registered = registeredView(() => ( @@ -203,14 +204,17 @@ describe("ExtensionPaneHost activation", () => { onSelectFile={() => {}} onSelectHunk={() => {}} onRevealLine={() => "line"} + onActivateSurface={(key) => surfaceActivations.push(key)} />, async (setup) => { await act(async () => setup.mockMouse.click(2, 0, MouseButtons.LEFT)); expect(activations).toBe(1); + expect(surfaceActivations).toEqual(["probe:probe-view"]); expect(childPresses).toBe(1); await act(async () => setup.mockMouse.click(2, 0, MouseButtons.RIGHT)); expect(activations).toBe(1); + expect(surfaceActivations).toEqual(["probe:probe-view"]); expect(childPresses).toBe(2); }, ); diff --git a/src/ui/components/panes/ExtensionPane.tsx b/src/ui/components/panes/ExtensionPane.tsx index 651996dc8..17ab05f5a 100644 --- a/src/ui/components/panes/ExtensionPane.tsx +++ b/src/ui/components/panes/ExtensionPane.tsx @@ -93,6 +93,7 @@ export interface ExtensionPaneHostProps { onSelectFile: (fileId: string) => void; onSelectHunk: (fileId: string, hunkIndex: number) => void; onRevealLine: (fileId: string, side: "old" | "new", line: number) => "line" | "hunk" | "none"; + onActivateSurface?: (paneKey: string) => void; onRenderFailure?: () => void; } @@ -115,6 +116,7 @@ function ExtensionPaneHostView({ onSelectFile, onSelectHunk, onRevealLine, + onActivateSurface, onRenderFailure, }: ExtensionPaneHostProps) { const { extensionId } = registered; @@ -159,7 +161,9 @@ function ExtensionPaneHostView({ }; const filesChrome = paneKey(registered) === HUNK_FILES_PANE_KEY; const onMouseDown = (event: TuiMouseEvent) => { - if (event.button === MouseButton.LEFT) activatePane(registered, notify); + if (event.button !== MouseButton.LEFT) return; + onActivateSurface?.(paneKey(registered)); + activatePane(registered, notify); }; const box = (children: ReactNode) => ( {}; + +/** Render one divider and capture both its cells and paint spans. */ +async function captureDivider(isActive: boolean) { + const setup = await testRender( + , + { width: 8, height: 1 }, + ); + try { + await act(async () => setup.renderOnce()); + return { frame: setup.captureCharFrame(), spans: setup.captureSpans() }; + } finally { + await act(async () => setup.renderer.destroy()); + } +} + +describe("PaneDivider", () => { + test("changes weight and semantic color when its pane is active", async () => { + const inactive = await captureDivider(false); + const active = await captureDivider(true); + const inactiveSpan = inactive.spans.lines.flatMap((line) => line.spans)[0]; + const activeSpan = active.spans.lines.flatMap((line) => line.spans)[0]; + + expect(inactive.frame).toContain("────────"); + expect(active.frame).toContain("━━━━━━━━"); + expect(capturedTestColorToHex(inactiveSpan?.fg)?.toLowerCase()).toBe( + theme.border.toLowerCase(), + ); + expect(capturedTestColorToHex(activeSpan?.fg)?.toLowerCase()).toBe(theme.accent.toLowerCase()); + }); +}); diff --git a/src/ui/components/panes/PaneDivider.tsx b/src/ui/components/panes/PaneDivider.tsx index e1f3c69df..7b4c8bd40 100644 --- a/src/ui/components/panes/PaneDivider.tsx +++ b/src/ui/components/panes/PaneDivider.tsx @@ -4,12 +4,14 @@ import type { AppTheme } from "../../themes"; const PANE_DIVIDER_HIT_AREA_SIZE = 5; const PANE_DIVIDER_HIT_AREA_OFFSET = Math.floor(PANE_DIVIDER_HIT_AREA_SIZE / 2); -/** Render a one-cell pane divider with a larger pointer target on either axis. */ +/** Render a one-cell pane separator, adding a larger pointer target when it is resizable. */ export function PaneDivider({ orientation, width, height, + isActive, isResizing, + resizable, theme, onMouseDown, onMouseDrag, @@ -19,13 +21,18 @@ export function PaneDivider({ orientation: "vertical" | "horizontal"; width: number; height: number; + isActive: boolean; isResizing: boolean; + resizable: boolean; theme: AppTheme; onMouseDown: (event: TuiMouseEvent) => void; onMouseDrag: (event: TuiMouseEvent) => void; onMouseDragEnd: (event: TuiMouseEvent) => void; onMouseUp: (event: TuiMouseEvent) => void; }) { + const emphasized = isActive || isResizing; + const horizontal = emphasized ? "━" : "─"; + const vertical = emphasized ? "┃" : "│"; const handlers = { onMouseDown, onMouseDrag, onMouseUp, onMouseDragEnd }; const hitAreaStyle = orientation === "vertical" @@ -54,15 +61,15 @@ export function PaneDivider({ flexShrink: 0, backgroundColor: isResizing ? theme.accentMuted : theme.panel, border: orientation === "vertical" ? ["left"] : ["top"], - borderColor: isResizing ? theme.accent : theme.border, + borderColor: emphasized ? theme.accent : theme.border, }} customBorderChars={{ - topLeft: orientation === "vertical" ? "│" : "─", - topRight: "─", - bottomLeft: "│", - bottomRight: "─", - horizontal: "─", - vertical: "│", + topLeft: orientation === "vertical" ? vertical : horizontal, + topRight: horizontal, + bottomLeft: vertical, + bottomRight: horizontal, + horizontal, + vertical, topT: "┬", bottomT: "┴", leftT: "├", @@ -70,7 +77,7 @@ export function PaneDivider({ cross: "┼", }} /> - + {resizable ? : null} ); } diff --git a/src/ui/hooks/useActivePaneController.test.ts b/src/ui/hooks/useActivePaneController.test.ts new file mode 100644 index 000000000..dc10d4e31 --- /dev/null +++ b/src/ui/hooks/useActivePaneController.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, test } from "bun:test"; +import type { Renderable } from "@opentui/core"; +import { paneKeyFromFocusedRenderable } from "./useActivePaneController"; + +/** Build the minimal renderable ancestry needed to test host surface ownership. */ +function renderable(id: string, parent: Renderable | null = null): Renderable { + return { id, parent } as Renderable; +} + +describe("active pane focus ownership", () => { + test("finds pane and review roots through focused descendants", () => { + const pane = renderable("pane"); + const editor = renderable("prompt", renderable("card", pane)); + const review = renderable("review"); + const noteEditor = renderable("draft", review); + const roots = new Map([ + [pane, "fixture:agent"], + [review, null], + ]); + + expect(paneKeyFromFocusedRenderable(editor, roots)).toBe("fixture:agent"); + expect(paneKeyFromFocusedRenderable(noteEditor, roots)).toBeNull(); + }); + + test("ignores focus outside the review workspace", () => { + const roots = new Map(); + expect(paneKeyFromFocusedRenderable(renderable("menu"), roots)).toBeUndefined(); + expect(paneKeyFromFocusedRenderable(null, roots)).toBeUndefined(); + }); +}); diff --git a/src/ui/hooks/useActivePaneController.ts b/src/ui/hooks/useActivePaneController.ts new file mode 100644 index 000000000..40bd16b04 --- /dev/null +++ b/src/ui/hooks/useActivePaneController.ts @@ -0,0 +1,110 @@ +/** + * Tracks the review or docked pane the user most recently engaged. + * + * Pointer activation and focused descendants converge here while modal focus leaves the underlying + * workspace surface intact. Panes that leave the committed layout fall back to the review. + */ + +import type { BoxRenderable, CliRenderer, Renderable } from "@opentui/core"; +import { useCallback, useEffect, useLayoutEffect, useRef, useState } from "react"; + +/** Resolve a focused renderable to its containing workspace surface, when it has one. */ +export function paneKeyFromFocusedRenderable( + renderable: Renderable | null, + surfaceRoots: ReadonlyMap, +): string | null | undefined { + let current = renderable; + while (current) { + if (surfaceRoots.has(current)) return surfaceRoots.get(current); + current = current.parent; + } + return undefined; +} + +/** Own logical workspace activation independently from concrete OpenTUI editor focus. */ +export function useActivePaneController({ + renderer, + visiblePaneKeys, +}: { + renderer: CliRenderer; + visiblePaneKeys: readonly string[]; +}) { + const [activePaneKey, setActivePaneKey] = useState(null); + const surfaceRootsRef = useRef(new Map()); + const paneRootsRef = useRef(new Map()); + const paneRootCallbacksRef = useRef( + new Map void>(), + ); + const reviewRootRef = useRef(null); + const visiblePaneKeysRef = useRef(visiblePaneKeys); + + const paneSurfaceRef = useCallback((paneKey: string) => { + let callback = paneRootCallbacksRef.current.get(paneKey); + if (!callback) { + callback = (renderable) => { + const previous = paneRootsRef.current.get(paneKey); + if (previous) surfaceRootsRef.current.delete(previous); + if (renderable) { + paneRootsRef.current.set(paneKey, renderable); + surfaceRootsRef.current.set(renderable, paneKey); + } else { + paneRootsRef.current.delete(paneKey); + } + }; + paneRootCallbacksRef.current.set(paneKey, callback); + } + return callback; + }, []); + + const reviewSurfaceRef = useCallback((renderable: BoxRenderable | null) => { + if (reviewRootRef.current) surfaceRootsRef.current.delete(reviewRootRef.current); + reviewRootRef.current = renderable; + if (renderable) surfaceRootsRef.current.set(renderable, null); + }, []); + + useEffect(() => { + let live = true; + const activateFocusedSurface = (current: Renderable | null) => { + const paneKey = paneKeyFromFocusedRenderable(current, surfaceRootsRef.current); + if ( + paneKey === undefined || + (paneKey !== null && !visiblePaneKeysRef.current.includes(paneKey)) + ) { + return false; + } + setActivePaneKey(paneKey); + return true; + }; + const syncFocusedSurface = (current: Renderable | null) => { + if (activateFocusedSurface(current) || !current) return; + // Newly focused renderables can emit before React attaches their host-pane ancestry. + queueMicrotask(() => { + if (live && renderer.currentFocusedRenderable === current) activateFocusedSurface(current); + }); + }; + renderer.on("focused_renderable", syncFocusedSurface); + syncFocusedSurface(renderer.currentFocusedRenderable); + return () => { + live = false; + renderer.off("focused_renderable", syncFocusedSurface); + }; + }, [renderer]); + + useLayoutEffect(() => { + visiblePaneKeysRef.current = visiblePaneKeys; + setActivePaneKey((current) => + current !== null && !visiblePaneKeys.includes(current) ? null : current, + ); + }, [visiblePaneKeys]); + + const activatePane = useCallback((paneKey: string) => setActivePaneKey(paneKey), []); + const activateReview = useCallback(() => setActivePaneKey(null), []); + + return { + activePaneKey, + activatePane, + activateReview, + paneSurfaceRef, + reviewSurfaceRef, + }; +} diff --git a/src/ui/hooks/useExtensionPaneController.ts b/src/ui/hooks/useExtensionPaneController.ts index 2692202f8..08e224278 100644 --- a/src/ui/hooks/useExtensionPaneController.ts +++ b/src/ui/hooks/useExtensionPaneController.ts @@ -115,7 +115,7 @@ function activeResizePane( planned.pane.key === resize.key && planned.pane.registered === resize.registered && planned.pane.placement === resize.placement && - planned.divider !== undefined, + planned.resizable, ); } @@ -524,13 +524,13 @@ export function useExtensionPaneController({ // Start a drag only for the divider still owned by this exact pane registration. const beginPaneResize = useCallback( (planned: PlannedPane, event: TuiMouseEvent): boolean => { - if (event.button !== MouseButton.LEFT || !planned.divider) return false; + if (event.button !== MouseButton.LEFT || !planned.resizable) return false; const committed = paneLayoutRef.current?.panes.find( (entry) => entry.pane.key === planned.pane.key && entry.pane.registered === planned.pane.registered && entry.pane.placement === planned.pane.placement && - entry.divider !== undefined, + entry.resizable, ); if (!committed) return false; const vertical = committed.pane.placement === "left" || committed.pane.placement === "right"; diff --git a/src/ui/lib/extensionPanes.test.ts b/src/ui/lib/extensionPanes.test.ts index 72ff1de7c..53bad97a5 100644 --- a/src/ui/lib/extensionPanes.test.ts +++ b/src/ui/lib/extensionPanes.test.ts @@ -168,8 +168,9 @@ describe("extension panes", () => { const files = layout.panes.find((pane) => pane.pane.key === HUNK_FILES_PANE_KEY)!; const info = layout.panes.find((pane) => pane.pane.key === "hunk:review-info")!; expect(files.bounds).toEqual({ x: 0, y: 0, width: 38, height: 30 }); - expect(info.bounds).toEqual({ x: 39, y: 0, width: 201, height: 3 }); - expect(info.divider).toBeUndefined(); + expect(info.bounds).toEqual({ x: 39, y: 0, width: 201, height: 2 }); + expect(info.divider).toEqual({ x: 39, y: 2, width: 201, height: 1 }); + expect(info.resizable).toBeFalse(); expect(layout.reviewBounds).toEqual({ x: 39, y: 3, width: 201, height: 27 }); }); @@ -205,13 +206,20 @@ describe("extension panes", () => { minReviewWidth: 40, minReviewHeight: 5, }); - expect(plan.reviewBounds).toEqual({ x: 20, y: 4, width: 65, height: 23 }); + expect(plan.reviewBounds).toEqual({ x: 21, y: 5, width: 63, height: 21 }); expect(plan.panes.map((entry) => entry.pane.placement)).toEqual([ "left", "right", "top", "bottom", ]); + expect(plan.panes.map((entry) => entry.resizable)).toEqual([false, false, false, false]); + expect(plan.panes.map((entry) => entry.divider)).toEqual([ + { x: 20, y: 0, width: 1, height: 30 }, + { x: 84, y: 0, width: 1, height: 30 }, + { x: 21, y: 4, width: 63, height: 1 }, + { x: 21, y: 26, width: 63, height: 1 }, + ]); }); test("separates commit-phase availability from pure geometry planning", () => { @@ -474,7 +482,7 @@ describe("extension panes", () => { expect(heights(60)).toEqual([15, 15]); }); - test("uses explicit height overrides and reserves a divider only for resizable panes", () => { + test("uses explicit height overrides and marks resizable pane dividers", () => { const registered = registeredPane("a", "top", { placement: "top", height: { preferred: 4, min: 2, max: 8 }, @@ -493,6 +501,7 @@ describe("extension panes", () => { const top = plan.panes.find((entry) => entry.pane.key === "a:top"); expect(top?.bounds).toEqual({ x: 0, y: 0, width: 100, height: 7 }); expect(top?.divider).toEqual({ x: 0, y: 7, width: 100, height: 1 }); + expect(top?.resizable).toBeTrue(); expect(plan.reviewBounds).toEqual({ x: 0, y: 8, width: 100, height: 12 }); }); diff --git a/src/ui/lib/extensionPanes.ts b/src/ui/lib/extensionPanes.ts index ca556e0c2..bebbf6436 100644 --- a/src/ui/lib/extensionPanes.ts +++ b/src/ui/lib/extensionPanes.ts @@ -9,7 +9,7 @@ import type { import { extensionPaneSize } from "../../extensions/panes"; import type { ExtensionLoadResult, RegisteredPane } from "../../extensions/types"; -/** One cell reserved between each resizable pane and its neighbor. */ +/** One cell reserved between each pane and its neighbor. */ export const EXTENSION_PANE_DIVIDER_SIZE = 1; /** Smallest review height preserved while edge panes are open or resized. */ export const MIN_EXTENSION_REVIEW_HEIGHT = 5; @@ -134,7 +134,8 @@ export interface PaneBounds { export interface PlannedPane { pane: SessionPane; bounds: PaneBounds; - divider?: PaneBounds; + divider: PaneBounds; + resizable: boolean; } export interface ExtensionPaneLayoutPlan { panes: readonly PlannedPane[]; @@ -232,7 +233,7 @@ export function planExtensionPanes(options: PlanExtensionPanesOptions): Extensio target: options.sizes[pane.key] ?? automaticSize, min, max, - fixed: min === max, + resizable: min !== max, }; }; @@ -240,7 +241,7 @@ export function planExtensionPanes(options: PlanExtensionPanesOptions): Extensio (pane) => pane.placement === "left" || pane.placement === "right", )) { const spec = sizeSpec(pane); - const dividerSize = spec.fixed ? 0 : EXTENSION_PANE_DIVIDER_SIZE; + const dividerSize = EXTENSION_PANE_DIVIDER_SIZE; const remaining = right - left - options.minReviewWidth - dividerSize; const width = Math.min(Math.max(spec.target, spec.min), spec.max, remaining); if (width < spec.min) { @@ -249,27 +250,23 @@ export function planExtensionPanes(options: PlanExtensionPanesOptions): Extensio } if (pane.placement === "left") { const bounds = { x: left, y: 0, width, height: options.bodyHeight }; - const divider = dividerSize - ? { - x: left + width, - y: 0, - width: EXTENSION_PANE_DIVIDER_SIZE, - height: options.bodyHeight, - } - : undefined; - planned.set(pane.key, { pane, bounds, ...(divider ? { divider } : {}) }); + const divider = { + x: left + width, + y: 0, + width: EXTENSION_PANE_DIVIDER_SIZE, + height: options.bodyHeight, + }; + planned.set(pane.key, { pane, bounds, divider, resizable: spec.resizable }); left += width + dividerSize; } else { const bounds = { x: right - width, y: 0, width, height: options.bodyHeight }; - const divider = dividerSize - ? { - x: right - width - EXTENSION_PANE_DIVIDER_SIZE, - y: 0, - width: EXTENSION_PANE_DIVIDER_SIZE, - height: options.bodyHeight, - } - : undefined; - planned.set(pane.key, { pane, bounds, ...(divider ? { divider } : {}) }); + const divider = { + x: right - width - EXTENSION_PANE_DIVIDER_SIZE, + y: 0, + width: EXTENSION_PANE_DIVIDER_SIZE, + height: options.bodyHeight, + }; + planned.set(pane.key, { pane, bounds, divider, resizable: spec.resizable }); right -= width + dividerSize; } } @@ -278,7 +275,7 @@ export function planExtensionPanes(options: PlanExtensionPanesOptions): Extensio (pane) => pane.placement === "top" || pane.placement === "bottom", )) { const spec = sizeSpec(pane); - const dividerSize = spec.fixed ? 0 : EXTENSION_PANE_DIVIDER_SIZE; + const dividerSize = EXTENSION_PANE_DIVIDER_SIZE; const remaining = bottom - top - options.minReviewHeight - dividerSize; const height = Math.min(Math.max(spec.target, spec.min), spec.max, remaining); if (height < spec.min) { @@ -287,27 +284,23 @@ export function planExtensionPanes(options: PlanExtensionPanesOptions): Extensio } if (pane.placement === "top") { const bounds = { x: left, y: top, width: right - left, height }; - const divider = dividerSize - ? { - x: left, - y: top + height, - width: right - left, - height: EXTENSION_PANE_DIVIDER_SIZE, - } - : undefined; - planned.set(pane.key, { pane, bounds, ...(divider ? { divider } : {}) }); + const divider = { + x: left, + y: top + height, + width: right - left, + height: EXTENSION_PANE_DIVIDER_SIZE, + }; + planned.set(pane.key, { pane, bounds, divider, resizable: spec.resizable }); top += height + dividerSize; } else { const bounds = { x: left, y: bottom - height, width: right - left, height }; - const divider = dividerSize - ? { - x: left, - y: bottom - height - EXTENSION_PANE_DIVIDER_SIZE, - width: right - left, - height: EXTENSION_PANE_DIVIDER_SIZE, - } - : undefined; - planned.set(pane.key, { pane, bounds, ...(divider ? { divider } : {}) }); + const divider = { + x: left, + y: bottom - height - EXTENSION_PANE_DIVIDER_SIZE, + width: right - left, + height: EXTENSION_PANE_DIVIDER_SIZE, + }; + planned.set(pane.key, { pane, bounds, divider, resizable: spec.resizable }); bottom -= height + dividerSize; } } diff --git a/test/pty/extensions-integration.test.ts b/test/pty/extensions-integration.test.ts index 8ba5bac2c..cb82abf95 100644 --- a/test/pty/extensions-integration.test.ts +++ b/test/pty/extensions-integration.test.ts @@ -384,7 +384,7 @@ describe("PTY extensions", () => { ); expect(frame).toContain("octocat · GitHub · modem-dev/hunk · main ← feature/pane"); const infoLine = lineIndexOf(frame, "OPEN · #123"); - expect(frame.split("\n")[infoLine - 1]).toContain("─"); + expect(frame.split("\n")[infoLine + 2]).toContain("─"); expect(infoLine).toBeLessThan(lineIndexOf(frame, "after")); } finally { session.close(); @@ -667,10 +667,18 @@ describe("PTY extensions", () => { try { const frame = await session.waitForText(/PANE ACTIVATE TARGET/, { timeout: 20_000 }); const targetRow = lineIndexOf(frame, "PANE ACTIVATE TARGET"); - const targetColumn = frame.split("\n")[targetRow]!.indexOf("PANE ACTIVATE TARGET") + 5; + const targetStart = frame.split("\n")[targetRow]!.indexOf("PANE ACTIVATE TARGET"); + const targetColumn = targetStart + 5; + expect(frame.split("\n")[targetRow]![targetStart - 1]).toBe("│"); // Stay clear of the pane divider's intentional multi-cell resize hit area. await session.clickAt(targetColumn, targetRow); + await harness.waitForSnapshot( + session, + (text) => text.split("\n")[targetRow]?.[targetStart - 1] === "┃", + 5_000, + ); + const deadline = Date.now() + 5_000; while (!existsSync(activationLog) && Date.now() < deadline) { await Bun.sleep(20);