Skip to content

Commit f73e1da

Browse files
Merge palette chrome and overlay geometry fixes
2 parents 967ba4e + bd4c380 commit f73e1da

11 files changed

Lines changed: 245 additions & 194 deletions

File tree

src/tui-opentui/command-catalog.test.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,12 @@ describe("commandItemsFromRegistry", () => {
6060
label: "/tasks — Show work list",
6161
keywords: ["tasks", "slash", "command"],
6262
dispatch: "command",
63-
category: "command",
6463
},
6564
{
6665
id: "clear",
6766
label: "/clear — Clear screen",
6867
keywords: ["clear", "slash", "command"],
6968
dispatch: "command",
70-
category: "session",
7169
},
7270
])
7371
})

src/tui-opentui/geometry.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,27 @@ describe("resolveGeometry — overlay modes", () => {
244244
expect(layout.transcriptHeight).toBeGreaterThanOrEqual(OVERLAY_TRANSCRIPT_FLOOR);
245245
});
246246

247+
test("a large list overlay on a short terminal never exceeds terminal rows", () => {
248+
// A ~30-command palette asks for far more body rows than a short terminal
249+
// has; the resolver must still sum to exactly terminal.rows rather than
250+
// let the overlay's own border/title chrome overflow past the screen.
251+
for (let rows = 4; rows <= 12; rows++) {
252+
const layout = resolveGeometry({
253+
terminal: { columns: 80, rows },
254+
overlay: { mode: "inset", bodyRows: 48 },
255+
});
256+
const total = layout.chromeHeight + layout.overlayHeight + layout.transcriptHeight;
257+
expect(total).toBe(rows);
258+
}
259+
});
260+
261+
test("overlay gets at least its border/title minimum before the transcript floor", () => {
262+
const layout = idle80x24({
263+
overlay: { mode: "inset", bodyRows: 48, minBodyRows: 5 },
264+
});
265+
expect(layout.overlayHeight).toBeGreaterThanOrEqual(5);
266+
});
267+
247268
test("full_shell hides transcript and gives residual to overlay_host", () => {
248269
const layout = idle80x24({
249270
overlay: { mode: "full_shell", bodyRows: 20 },

src/tui-opentui/geometry/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ export {
22
COLLAPSE_ORDER,
33
IDLE_TRANSCRIPT_FLOOR,
44
OVERLAY_MAX_FRACTION,
5+
OVERLAY_MIN_ROWS,
56
OVERLAY_TRANSCRIPT_FLOOR,
67
PAINT_ORDER,
78
PROMPT_BASE_ROWS,

src/tui-opentui/geometry/resolve.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
COLLAPSE_ORDER,
77
IDLE_TRANSCRIPT_FLOOR,
88
OVERLAY_MAX_FRACTION,
9+
OVERLAY_MIN_ROWS,
910
OVERLAY_TRANSCRIPT_FLOOR,
1011
PAINT_ORDER,
1112
PROMPT_BASE_ROWS,
@@ -26,6 +27,12 @@ export type OverlayInput = {
2627
readonly mode: OverlayMode;
2728
/** Requested overlay body rows (measured by host). Capped by fraction + floor. */
2829
readonly bodyRows?: number;
30+
/**
31+
* Rows the overlay's own chrome cannot render without (border + title +
32+
* at least one content row). Falls back to `OVERLAY_MIN_ROWS` when the
33+
* caller has not measured its actual chrome.
34+
*/
35+
readonly minBodyRows?: number;
2936
};
3037

3138
/**
@@ -315,6 +322,15 @@ export function resolveGeometry(input: GeometryInput): GeometryLayout {
315322
);
316323
if (heights.prompt > promptCap) heights.prompt = promptCap;
317324

325+
// An open overlay with real content needs its own border/title rows or it
326+
// renders past whatever height it was actually assigned. The transcript
327+
// floor below cannot be satisfied at that overlay's expense.
328+
const requestedOverlayRows = input.overlay?.bodyRows ?? 0;
329+
const minOverlay =
330+
mode !== "closed" && requestedOverlayRows > 0
331+
? Math.min(input.overlay?.minBodyRows ?? OVERLAY_MIN_ROWS, requestedOverlayRows)
332+
: 0;
333+
318334
// Iteratively collapse optional chrome until transcript meets floor with overlay.
319335
// Enough steps to walk a grown prompt back to base one row at a time on top
320336
// of dropping every optional zone.
@@ -328,15 +344,16 @@ export function resolveGeometry(input: GeometryInput): GeometryLayout {
328344
floor,
329345
);
330346
const transcript = terminal.rows - chrome - overlay;
331-
if (transcript >= floor) {
347+
if (transcript >= floor && overlay >= minOverlay) {
332348
heights.transcript = Math.max(0, transcript);
333349
heights.overlay_host = overlay;
334350
break;
335351
}
336352
// Need more space: collapse one zone, then retry.
337353
const cut = collapseOnce(heights, collapsed);
338354
if (cut === null) {
339-
// Nothing left — accept best effort (may be below floor on tiny terminals).
355+
// Nothing left — relax the transcript floor rather than leave the
356+
// overlay under its own render minimum; accept best effort past that.
340357
heights.overlay_host = desiredOverlayHeight(
341358
{ ...input, terminal },
342359
mode,

src/tui-opentui/geometry/zones.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,14 @@ export const PROMPT_CAP_FRACTION = 0.4;
114114
/** Overlay host body may not exceed this fraction of terminal rows (proposed). */
115115
export const OVERLAY_MAX_FRACTION = 0.7;
116116

117+
/**
118+
* Smallest overlay_host an open overlay can render into: two border rows plus
119+
* one content row. The transcript floor exists to keep conversation visible,
120+
* but it must not starve an overlay the operator just opened below the rows
121+
* its own border costs — that renders past its box instead of shrinking.
122+
*/
123+
export const OVERLAY_MIN_ROWS = 3;
124+
117125
/**
118126
* Prompt floor: labelled borders + one content line. Only a terminal too short
119127
* to seat the transcript floor alongside a composing area gets squeezed here.

src/tui-opentui/palette-paint.test.ts

Lines changed: 79 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
2-
* Frame-level checks for the command palette's three-column rows: the category
3-
* prefix, the right-aligned chord, and how they degrade at narrow widths.
2+
* Frame-level checks for the command palette's rows: the label, the
3+
* right-aligned chord, and how the chord degrades at narrow widths.
44
*/
55
import { describe, expect, test } from "bun:test"
66

@@ -39,33 +39,31 @@ function rowFor(rows: readonly string[], label: string): string | undefined {
3939
}
4040

4141
describe("command palette rows", () => {
42-
test("titles the box with a broken rule and shows the filter prompt", async () => {
42+
test("shows the filter prompt with no title rule above it", async () => {
4343
const rows = await paletteFrame(100)
44-
expect(rows.some((r) => r.startsWith("─ command palette ─"))).toBe(true)
44+
expect(rows.some((r) => r.startsWith("─ command palette ─"))).toBe(false)
4545
expect(rows.some((r) => r.trim() === ">")).toBe(true)
4646
})
4747

48-
test("paints the category prefix and right-aligned chord at 100 columns", async () => {
48+
test("has no leading selection marker or kind column", async () => {
4949
const rows = await paletteFrame(100)
5050
const help = rowFor(rows, "Show keymap help")
5151
expect(help).toBeDefined()
52-
expect(help).toMatch(/^\s+[> ] view\s+Show keymap help\s+\?$/)
52+
expect(help).toMatch(/^\s*Show keymap help\s+\?$/)
53+
expect(help).not.toContain(">")
54+
expect(help).not.toContain("view")
55+
})
5356

57+
test("keeps the right-aligned chord at 100 columns", async () => {
58+
const rows = await paletteFrame(100)
5459
const copy = rowFor(rows, "Copy active message / tool")
5560
expect(copy?.endsWith("Alt+C")).toBe(true)
5661
})
5762

58-
test("keeps both side columns at 60 columns", async () => {
59-
const rows = await paletteFrame(60)
60-
const help = rowFor(rows, "Show keymap help")
61-
expect(help).toContain("view")
62-
expect(help?.endsWith("?")).toBe(true)
63-
})
64-
65-
test("drops the chord first at 48 columns, keeping the category", async () => {
66-
const rows = await paletteFrame(48)
63+
test("drops the chord at a narrow width, and the label always survives", async () => {
64+
const rows = await paletteFrame(36)
6765
const help = rowFor(rows, "Show keymap help")
68-
expect(help).toContain("view")
66+
expect(help).toBeDefined()
6967
expect(help?.endsWith("?")).toBe(false)
7068

7169
const copy = rowFor(rows, "Copy active")
@@ -193,3 +191,68 @@ describe("palette filters as you type", () => {
193191
)
194192
})
195193
})
194+
195+
describe("command palette width", () => {
196+
// Both boxes are children of the same padded root; a width computed a
197+
// second way for the floating palette drifts from the prompt box's "100%".
198+
test("shares the prompt box's left/right edges while floating over landing", async () => {
199+
const rows = await withTestRenderer(
200+
async (h) => {
201+
const shell = createAppShell(h.renderer, {
202+
terminal: { columns: 80, rows: 24 },
203+
wireKeys: false,
204+
run: "idle",
205+
})
206+
openPalette(shell)
207+
await h.renderOnce()
208+
return h.captureCharFrame().split("\n")
209+
},
210+
{ width: 80, height: 24 },
211+
)
212+
const overlayTop = rows.find((r) => r.includes("┌"))
213+
const promptTop = rows.find((r) => r.includes("╭"))
214+
expect(overlayTop).toBeDefined()
215+
expect(promptTop).toBeDefined()
216+
expect(overlayTop?.indexOf("┌")).toBe(promptTop?.indexOf("╭"))
217+
expect(overlayTop?.lastIndexOf("┐")).toBe(promptTop?.lastIndexOf("╮"))
218+
})
219+
})
220+
221+
describe("command palette selection colour", () => {
222+
test("marks the active row by text colour, not a filled background", async () => {
223+
await withTestRenderer(
224+
async (h) => {
225+
const shell = createAppShell(h.renderer, {
226+
terminal: { columns: 100, rows: 32 },
227+
wireKeys: false,
228+
run: "idle",
229+
})
230+
openPalette(shell)
231+
await h.renderOnce()
232+
const frame = h.captureSpans()
233+
const activeLine = frame.lines.find((line) =>
234+
line.spans.some((s) => s.text.includes("Open permissions")),
235+
)
236+
const groundLine = frame.lines.find((line) =>
237+
line.spans.some((s) => s.text.includes("Ask operator question")),
238+
)
239+
expect(activeLine).toBeDefined()
240+
expect(groundLine).toBeDefined()
241+
const activeBg = activeLine!.spans[0]!.bg
242+
const groundBg = groundLine!.spans[0]!.bg
243+
// Same background either way — selection reads through text colour
244+
// (fg), not a filled band behind the row.
245+
expect(activeBg).toEqual(groundBg)
246+
const activeFg = activeLine!.spans.find((s) =>
247+
s.text.includes("Open permissions"),
248+
)!.fg
249+
const groundFg = groundLine!.spans.find((s) =>
250+
s.text.includes("Ask operator question"),
251+
)!.fg
252+
expect(activeFg).not.toEqual(groundFg)
253+
},
254+
{ width: 100, height: 32 },
255+
)
256+
})
257+
})
258+

src/tui-opentui/palette.test.ts

Lines changed: 10 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -84,27 +84,6 @@ describe("buildPaletteCatalog", () => {
8484
})
8585

8686
describe("palette row columns", () => {
87-
test("residual openers carry a category", () => {
88-
const cols = DEFAULT_PALETTE_COMMANDS.map((c) =>
89-
paletteRowColumns(c, shortcutForPaletteId),
90-
)
91-
expect(cols.every((c) => c.category.length > 0)).toBe(true)
92-
expect(cols.find((c) => c.label === "Show keymap help")?.category).toBe(
93-
"view",
94-
)
95-
})
96-
97-
test("registry commands get a category from their name", () => {
98-
const catalog = buildPaletteCatalog({
99-
commands: [
100-
{ name: "rename", description: "Name the session" },
101-
{ name: "wobble", description: "A plugin command" },
102-
],
103-
})
104-
expect(catalog.find((c) => c.id === "rename")?.category).toBe("session")
105-
expect(catalog.find((c) => c.id === "wobble")?.category).toBe("command")
106-
})
107-
10887
test("shortcuts come from the shell keybinding table", () => {
10988
const help = DEFAULT_PALETTE_COMMANDS.find((c) => c.id === "help")
11089
expect(paletteRowColumns(help!, shortcutForPaletteId).shortcut).toBe("?")
@@ -115,16 +94,15 @@ describe("palette row columns", () => {
11594

11695
describe("formatPaletteRows", () => {
11796
const ROWS: readonly PaletteRowColumns[] = [
118-
{ category: "view", label: "Show keymap help", shortcut: "?" },
119-
{ category: "edit", label: "Copy active message / tool", shortcut: "Alt+C" },
120-
{ category: "session", label: "Resume prior session", shortcut: "" },
97+
{ label: "Show keymap help", shortcut: "?" },
98+
{ label: "Copy active message / tool", shortcut: "Alt+C" },
99+
{ label: "Resume prior session", shortcut: "" },
121100
]
122101

123-
test("renders category, label, and right-aligned shortcut at full width", () => {
102+
test("renders the label and right-aligned shortcut at full width", () => {
124103
const [help, copy] = formatPaletteRows(ROWS, 55)
125104
expect(help).toHaveLength(55)
126-
expect(help?.startsWith("view ")).toBe(true)
127-
expect(help).toContain("Show keymap help")
105+
expect(help?.startsWith("Show keymap help")).toBe(true)
128106
expect(help?.trimEnd().endsWith("?")).toBe(true)
129107
expect(copy?.trimEnd().endsWith("Alt+C")).toBe(true)
130108
})
@@ -137,29 +115,11 @@ describe("formatPaletteRows", () => {
137115
}
138116
})
139117

140-
// The palette host spends the box border and the selection marker before the
141-
// row starts, so a terminal N columns wide hands these rows N - 5.
142-
const ROW_WIDTH_AT_60 = 55
143-
const ROW_WIDTH_AT_48 = 43
144-
145-
test("the shortcut column drops first as width narrows", () => {
146-
expect(paletteRowLayout(ROWS, ROW_WIDTH_AT_60)).toMatchObject({
147-
showCategory: true,
148-
showShortcut: true,
149-
})
150-
expect(paletteRowLayout(ROWS, ROW_WIDTH_AT_48)).toMatchObject({
151-
showCategory: true,
152-
showShortcut: false,
153-
})
154-
const rows = formatPaletteRows(ROWS, ROW_WIDTH_AT_48)
118+
test("the shortcut column drops as width narrows, and the label always survives", () => {
119+
expect(paletteRowLayout(ROWS, 40)).toMatchObject({ showShortcut: true })
120+
expect(paletteRowLayout(ROWS, 30)).toMatchObject({ showShortcut: false })
121+
const rows = formatPaletteRows(ROWS, 30)
155122
expect(rows[0]?.includes("?")).toBe(false)
156-
expect(rows[0]?.startsWith("view")).toBe(true)
157-
})
158-
159-
test("the category drops next, and the label always survives", () => {
160-
const layout = paletteRowLayout(ROWS, 34)
161-
expect(layout.showCategory).toBe(false)
162-
expect(layout.showShortcut).toBe(false)
163-
expect(formatPaletteRows(ROWS, 34)[0]?.trimEnd()).toBe("Show keymap help")
123+
expect(rows[0]?.trimEnd()).toBe("Show keymap help")
164124
})
165125
})

0 commit comments

Comments
 (0)