Skip to content

Commit 9d4c36c

Browse files
saravmajesticclaude
andcommitted
fix(tui): address welcome-panel round-3 review (reserves, overlay, tests)
- correct per-route vertical reserves to the real chrome: HOME 10->15, SESSION 7->9, derived from PROMPT_REST_HEIGHT=6 plus the counted home_bottom slot and footer (the prompt is 5-6 rows at rest, not ~4) - retune height thresholds honestly: MEDIUM_MIN_HEIGHT 13->8 (the real medium panel height, so 80x24 is a genuine medium fit, not a pinned margin), FULL_MIN_HEIGHT 34->30 (a documented product breakpoint, not a fit minimum) - centralize the route arithmetic: export homeAvailable/sessionAvailable and call them from both the routes and the tests, so a wrong reserve/threshold fails a test instead of being mirrored in a test-only copy - centralize PANEL_HORIZONTAL_PADDING and add SIDEBAR_WIDTH (drop the duplicated 42) - keep the panel on session's contentWidth basis (narrows with the sidebar, incl. the dimmed overlay) so the panel stays aligned with the messages; documented - tests: reserve-pinned-to-chrome, overlay narrowing, negative dimensions Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 2eac798 commit 9d4c36c

4 files changed

Lines changed: 142 additions & 58 deletions

File tree

‎packages/tui/src/component/welcome-panel-utils.ts‎

Lines changed: 70 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,43 +5,58 @@
55
// bordered box, so on a typical terminal it eats ~40% of the screen with no way
66
// to shrink it (issue #1067). We scale it down by the space the panel ACTUALLY
77
// has on both axes:
8-
// - width: the terminal minus the caller's padding and any sibling sidebar.
8+
// - width: the terminal minus the caller's padding and any sibling sidebar
9+
// that consumes layout width.
910
// - height: the terminal minus the fixed chrome that always shares the column
10-
// with the panel (see the per-route reserves below), so a big panel
11-
// is chosen only when it won't crowd the prompt off a short terminal.
11+
// with the panel (the per-route reserves below), so a big panel is
12+
// chosen only when it won't crowd the prompt off a short terminal.
1213
// `medium` (no wordmark) is the common case; `full` only when there's real room.
1314
//
14-
// Naming: these are the MINIMUMS a tier requires, matched with strict `<`
15-
// (`width < FULL_MIN_WIDTH` -> not full). MEDIUM_MIN_* is the floor for medium
16-
// (below -> compact); FULL_MIN_* is the floor for full (below -> medium). All in
17-
// terms of AVAILABLE (usable) size, not the raw terminal.
15+
// Route arithmetic lives here (homeAvailable / sessionAvailable) so the routes
16+
// and the tests share ONE definition — a test that maps a terminal size to a
17+
// variant then exercises the real call-site math, not a copy of it.
1818

1919
export type WelcomePanelVariant = "full" | "medium" | "compact"
2020

21-
// Rows the panel must leave for the always-present chrome that shares its column.
22-
// The two routes have different chrome, so they reserve different amounts; the
23-
// shared thresholds below then transition ~3 rows of terminal height apart
24-
// between the routes (session, with less chrome, shows a bigger variant sooner).
25-
// Estimates — the prompt can grow with multi-line input; this is the at-rest cost.
21+
// --- Chrome reserves (rows the panel must leave for always-present siblings) ---
2622
//
27-
// home (routes/home.tsx): top spacer (2) + prompt wrapper paddingTop (1) + prompt
28-
// (~4) + footer (~3, feature-plugins/home/footer.tsx) ≈ 10.
29-
export const HOME_VERTICAL_RESERVE = 10
30-
// session (routes/session/index.tsx): two column gaps (2) + paddingBottom (1) +
31-
// prompt (~4); no top spacer, no footer in that column ≈ 7.
32-
export const SESSION_VERTICAL_RESERVE = 7
33-
34-
// Columns the home slot spends on its own left/right padding (2 + 2); the caller
35-
// subtracts this to get the panel's usable width. (Session's contentWidth applies
36-
// the same 4 as part of its own content-column math.)
23+
// Prompt tree at rest (component/prompt/index.tsx): top rule 1 + inner paddingTop
24+
// 1 + textarea 1 + separator 1 + agent/model meta row 1 + idle hint row 1 = 6.
25+
// Measured against the rendered tree, not estimated — the earlier "~4" undercounted
26+
// it (the meta and hint rows are always in flow).
27+
const PROMPT_REST_HEIGHT = 6
28+
29+
// home (routes/home.tsx): top spacer 2 (`<box height={2}>`) + prompt wrapper
30+
// paddingTop 1 + prompt 6 + home_bottom slot 3 (feature-plugins/home/tips.tsx
31+
// paddingTop, rendered unconditionally — flexbox shrinks content, not padding) +
32+
// footer 3 (feature-plugins/home/footer.tsx) = 15.
33+
export const HOME_VERTICAL_RESERVE = 2 + 1 + PROMPT_REST_HEIGHT + 3 + 3
34+
// session (routes/session/index.tsx): two column gaps 2 + paddingBottom 1 +
35+
// prompt 6 = 9. No top spacer and no footer share this column.
36+
export const SESSION_VERTICAL_RESERVE = 2 + 1 + PROMPT_REST_HEIGHT
37+
38+
// Columns the home slot spends on its own left/right padding (2 + 2). Session
39+
// subtracts the same via sessionAvailable(); both routes import this constant so
40+
// the value has a single source of truth.
3741
export const PANEL_HORIZONTAL_PADDING = 4
42+
// Width the session sidebar occupies WHEN it consumes layout width (i.e. it is
43+
// rendered in-flow, not as an overlay). Shared with the route + tests so they
44+
// can't drift — the drift that produced #1067.
45+
export const SIDEBAR_WIDTH = 42
3846

39-
/** Minimum usable size for the medium panel; below either -> compact (one line). */
47+
// --- Thresholds ---
48+
// MEDIUM_MIN_* is a real FIT requirement: below it the medium panel (~8 rows /
49+
// ~a title + one/two-line description) would not fit, so drop to the one-line
50+
// compact. FULL_MIN_* is a product BREAKPOINT, not a fit minimum: the full panel
51+
// is only ~13 rows, but we require far more available space so the branded
52+
// wordmark appears only on a genuinely large terminal and never dominates a
53+
// small one (the #1067 ask). All in AVAILABLE (usable) terms, not the raw
54+
// terminal, matched with strict `<`.
4055
export const MEDIUM_MIN_WIDTH = 60
41-
export const MEDIUM_MIN_HEIGHT = 13
42-
/** Minimum usable size for the full wordmark panel; below either -> medium. */
56+
export const MEDIUM_MIN_HEIGHT = 8
4357
export const FULL_MIN_WIDTH = 110
44-
export const FULL_MIN_HEIGHT = 34
58+
// ~45-row home terminal / ~39-row session terminal after the reserves above.
59+
export const FULL_MIN_HEIGHT = 30
4560

4661
/**
4762
* Choose the WelcomePanel layout from the panel's AVAILABLE size — width already
@@ -54,3 +69,32 @@ export function welcomePanelVariant(width: number, height: number): WelcomePanel
5469
if (width < FULL_MIN_WIDTH || height < FULL_MIN_HEIGHT) return "medium"
5570
return "full"
5671
}
72+
73+
/** Available panel size on the home route for a given terminal size. */
74+
export function homeAvailable(terminalWidth: number, terminalHeight: number): { width: number; height: number } {
75+
return {
76+
width: terminalWidth - PANEL_HORIZONTAL_PADDING,
77+
height: terminalHeight - HOME_VERTICAL_RESERVE,
78+
}
79+
}
80+
81+
/**
82+
* Available panel size on the session route. Subtracts SIDEBAR_WIDTH whenever the
83+
* sidebar is open (`sidebarVisible`) — the panel shares the same content-column
84+
* basis as the messages (session's `contentWidth`), so the two stay aligned.
85+
*
86+
* On narrow terminals the sidebar renders as a dimmed full-area overlay rather
87+
* than in-flow; the content column still narrows uniformly (panel + messages)
88+
* and both restore to full width when it closes, so we deliberately size to the
89+
* narrowed column rather than the transient obscured width.
90+
*/
91+
export function sessionAvailable(
92+
terminalWidth: number,
93+
terminalHeight: number,
94+
sidebarVisible: boolean,
95+
): { width: number; height: number } {
96+
return {
97+
width: terminalWidth - (sidebarVisible ? SIDEBAR_WIDTH : 0) - PANEL_HORIZONTAL_PADDING,
98+
height: terminalHeight - SESSION_VERTICAL_RESERVE,
99+
}
100+
}

‎packages/tui/src/routes/home.tsx‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { useTheme } from "../context/theme"
1919
// one-line "Get started: /connect ... /discover ..." hint below, which duplicated the
2020
// same guidance the panel's "Tips for getting started" section now covers.
2121
import { WelcomePanel } from "../component/welcome-panel"
22-
import { HOME_VERTICAL_RESERVE, PANEL_HORIZONTAL_PADDING } from "../component/welcome-panel-utils"
22+
import { homeAvailable } from "../component/welcome-panel-utils"
2323
// altimate_change end
2424

2525
let once = false
@@ -68,6 +68,9 @@ export function Home() {
6868
if (configured === "auto") return Math.max(75, Math.floor(dimensions().width * 0.7))
6969
return configured ?? 75
7070
})
71+
// Panel's available space (terminal minus this route's padding + vertical
72+
// reserve). Shared arithmetic in welcome-panel-utils so tests exercise it.
73+
const panelAvailable = createMemo(() => homeAvailable(dimensions().width, dimensions().height))
7174
let sent = false
7275

7376
onMount(() => {
@@ -109,13 +112,10 @@ export function Home() {
109112
<box height={2} flexShrink={0} />
110113
<box width="100%" flexShrink={0}>
111114
<pluginRuntime.Slot name="home_logo" mode="replace">
112-
{/* Size to the panel's real space, not the whole terminal (#1067):
113-
-PANEL_HORIZONTAL_PADDING for this column's paddingLeft/Right;
114-
-HOME_VERTICAL_RESERVE for the top spacer + prompt + footer. */}
115-
<WelcomePanel
116-
availableWidth={dimensions().width - PANEL_HORIZONTAL_PADDING}
117-
availableHeight={dimensions().height - HOME_VERTICAL_RESERVE}
118-
/>
115+
{/* Size to the panel's real space, not the whole terminal (#1067).
116+
homeAvailable() subtracts this column's padding and the top
117+
spacer + prompt + home_bottom + footer reserve. */}
118+
<WelcomePanel availableWidth={panelAvailable().width} availableHeight={panelAvailable().height} />
119119
</pluginRuntime.Slot>
120120
</box>
121121
<box flexGrow={1} minHeight={0} />

‎packages/tui/src/routes/session/index.tsx‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { useTuiPaths, useTuiTerminalEnvironment } from "../../context/runtime"
2626
import { Spinner } from "../../component/spinner"
2727
// altimate_change — shared boot box at the top of the session scrollback
2828
import { WelcomePanel } from "../../component/welcome-panel"
29-
import { SESSION_VERTICAL_RESERVE } from "../../component/welcome-panel-utils"
29+
import { PANEL_HORIZONTAL_PADDING, SIDEBAR_WIDTH, sessionAvailable } from "../../component/welcome-panel-utils"
3030
import { createSyntaxStyleMemo, generateSubtleSyntax, selectedForeground, useTheme } from "../../context/theme"
3131
import { BoxRenderable, ScrollBoxRenderable, addDefaultParsers, TextAttributes, RGBA } from "@opentui/core"
3232
import { Prompt, type PromptRef } from "../../component/prompt"
@@ -272,7 +272,15 @@ export function Session() {
272272
return false
273273
})
274274
const showTimestamps = createMemo(() => timestamps() === "show")
275-
const contentWidth = createMemo(() => dimensions().width - (sidebarVisible() ? 42 : 0) - 4)
275+
const contentWidth = createMemo(
276+
() => dimensions().width - (sidebarVisible() ? SIDEBAR_WIDTH : 0) - PANEL_HORIZONTAL_PADDING,
277+
)
278+
// Welcome panel's available space — same content-column basis as contentWidth
279+
// (narrows with the sidebar whenever open, incl. the dimmed overlay on narrow
280+
// terminals) so the panel stays aligned with the messages and both restore
281+
// together when it closes. Height reserves the prompt row. Shared arithmetic in
282+
// welcome-panel-utils so the tests exercise this exact math.
283+
const panelAvailable = createMemo(() => sessionAvailable(dimensions().width, dimensions().height, sidebarVisible()))
276284
const providers = createMemo(() => Model.index(sync.data.provider))
277285

278286
const scrollAcceleration = createMemo(() => getScrollAcceleration(tuiConfig))
@@ -1187,13 +1195,10 @@ export function Session() {
11871195
/discover) starts a session. Outside the scrollbox: the bordered panel
11881196
does not paint reliably inside the scroll viewport. */}
11891197
<box flexShrink={0}>
1190-
{/* Size to the panel's real space, not the terminal (#1067):
1191-
contentWidth already subtracts the sidebar + padding;
1192-
-SESSION_VERTICAL_RESERVE leaves room for the prompt. */}
1193-
<WelcomePanel
1194-
availableWidth={contentWidth()}
1195-
availableHeight={dimensions().height - SESSION_VERTICAL_RESERVE}
1196-
/>
1198+
{/* Size to the panel's real space, not the terminal (#1067).
1199+
panelAvailable() subtracts the in-flow sidebar + padding and
1200+
reserves the prompt row (see the memo above). */}
1201+
<WelcomePanel availableWidth={panelAvailable().width} availableHeight={panelAvailable().height} />
11971202
</box>
11981203
{/* altimate_change end */}
11991204
<scrollbox

‎packages/tui/test/component/welcome-panel-utils.test.ts‎

Lines changed: 50 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ import {
55
HOME_VERTICAL_RESERVE,
66
MEDIUM_MIN_HEIGHT,
77
MEDIUM_MIN_WIDTH,
8-
PANEL_HORIZONTAL_PADDING,
98
SESSION_VERTICAL_RESERVE,
9+
homeAvailable,
10+
sessionAvailable,
1011
welcomePanelVariant,
1112
} from "../../src/component/welcome-panel-utils"
1213

@@ -16,10 +17,25 @@ import {
1617
const TALL = FULL_MIN_HEIGHT + 10
1718
const WIDE = FULL_MIN_WIDTH + 20
1819

19-
// Map a terminal size to the AVAILABLE size each route feeds the pure function.
20-
const home = (w: number, h: number) => welcomePanelVariant(w - PANEL_HORIZONTAL_PADDING, h - HOME_VERTICAL_RESERVE)
21-
const session = (w: number, h: number, sidebar: boolean) =>
22-
welcomePanelVariant(w - (sidebar ? 42 : 0) - PANEL_HORIZONTAL_PADDING, h - SESSION_VERTICAL_RESERVE)
20+
// Map a terminal size to a variant THROUGH the exact functions the routes call
21+
// (homeAvailable / sessionAvailable), so these tests exercise the real call-site
22+
// arithmetic rather than a private copy of it. `sidebarVisible` is the route's
23+
// sidebarVisible() — the content column narrows whenever the sidebar is open.
24+
const home = (w: number, h: number) => {
25+
const a = homeAvailable(w, h)
26+
return welcomePanelVariant(a.width, a.height)
27+
}
28+
const session = (w: number, h: number, sidebarVisible: boolean) => {
29+
const a = sessionAvailable(w, h, sidebarVisible)
30+
return welcomePanelVariant(a.width, a.height)
31+
}
32+
33+
test("reserves stay pinned to the counted chrome", () => {
34+
// If someone changes the reserve to a wrong literal, this fails. The values are
35+
// derived sums of the documented per-route chrome (see welcome-panel-utils.ts).
36+
expect(HOME_VERTICAL_RESERVE).toBe(15)
37+
expect(SESSION_VERTICAL_RESERVE).toBe(9)
38+
})
2339

2440
test("full requires BOTH width and height to clear the full floor", () => {
2541
expect(welcomePanelVariant(WIDE, TALL)).toBe("full")
@@ -51,37 +67,56 @@ test("compact→medium boundary is exact (at the floor is medium)", () => {
5167
test("everyday terminals get medium, not the oversized wordmark", () => {
5268
expect(home(106, 31)).toBe("medium")
5369
expect(home(80, 24)).toBe("medium")
54-
// #1067 session case: a 130-col terminal with the 42-col sidebar leaves ~84
55-
// usable cols → medium (was wrongly full when it used the whole terminal width).
70+
// #1067 session case: a 130-col terminal with the in-flow 42-col sidebar leaves
71+
// ~84 usable cols → medium (was wrongly full when it used the whole terminal).
5672
expect(session(130, 50, true)).toBe("medium")
5773
})
5874

59-
test("the classic 80x24 is medium on both routes, with margin off the compact floor", () => {
60-
// The review flagged 80x24 sitting on the exact medium floor; it now clears it
61-
// on both routes (home reserves more chrome, so it's the tighter one).
75+
test("the classic 80x24 is medium on both routes (a real fit, not a fake margin)", () => {
76+
// 80x24 → home available (76 × 9), session available (76 × 15). Both clear the
77+
// medium floor (60 × 8) — the medium panel is ~8 rows, so this actually fits.
6278
expect(home(80, 24)).toBe("medium")
6379
expect(session(80, 24, false)).toBe("medium")
6480
})
6581

6682
test("full engages on a large window; ~one row below the floor stays medium", () => {
67-
expect(home(120, 44)).toBe("full") // FULL_MIN_HEIGHT(34) + HOME_VERTICAL_RESERVE(10)
68-
expect(home(120, 43)).toBe("medium")
83+
// full needs available height ≥ FULL_MIN_HEIGHT(30); home reserves 15, so the
84+
// terminal must be ≥ 45 rows.
85+
expect(home(120, 45)).toBe("full")
86+
expect(home(120, 44)).toBe("medium")
6987
})
7088

71-
test("toggling the session sidebar flips the panel full → medium on a wide window (#1067)", () => {
72-
// The exact regression #1067 reports: on a wide window, opening the 42-col
73-
// sidebar must shrink the panel out of `full` — it no longer has ~110 usable cols.
89+
test("toggling the in-flow session sidebar flips the panel full → medium on a wide window (#1067)", () => {
90+
// The exact regression #1067 reports: on a wide window the sidebar is in-flow,
91+
// and opening it must shrink the panel out of `full` (no longer ≥110 usable cols).
7492
expect(session(150, 50, false)).toBe("full")
7593
expect(session(150, 50, true)).toBe("medium")
7694
})
7795

96+
test("the content column narrows whenever the sidebar is open, incl. the overlay (aligned with messages)", () => {
97+
// On a ≤120-col terminal the sidebar is a dimmed full-area overlay, but the
98+
// content column (messages + panel) still narrows uniformly so they stay
99+
// aligned and both restore when it closes — sizing to the narrowed column, not
100+
// the transient obscured width. 100 cols: open → 54 usable → compact; closed →
101+
// 96 → medium. (Matches session's contentWidth basis; deliberate, per review.)
102+
expect(session(100, 50, true)).toBe("compact")
103+
expect(session(100, 50, false)).toBe("medium")
104+
})
105+
78106
test("a short terminal drops to compact once the route's chrome is reserved (#1067 height)", () => {
79107
// 120x22: wide, but too few usable rows after the home chrome → compact, where
80108
// the raw terminal height (22) would have picked medium.
81109
expect(home(120, 22)).toBe("compact")
82110
})
83111

112+
test("negative available height (a tiny terminal) collapses to compact, never throws", () => {
113+
// 80x5 → home available height 5 - 15 = -10; must resolve, not crash.
114+
expect(home(80, 5)).toBe("compact")
115+
expect(session(80, 5, false)).toBe("compact")
116+
})
117+
84118
test("degenerate sizes collapse to compact", () => {
85119
expect(welcomePanelVariant(0, 0)).toBe("compact")
86120
expect(welcomePanelVariant(1, 1)).toBe("compact")
121+
expect(welcomePanelVariant(-5, -5)).toBe("compact")
87122
})

0 commit comments

Comments
 (0)