-
Notifications
You must be signed in to change notification settings - Fork 137
fix(tui): per-route vertical reserve + required panel props (#1069 round-2 follow-up) #1071
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
2eac798
9d4c36c
2762efb
a5e1b50
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,42 +5,96 @@ | |
| // bordered box, so on a typical terminal it eats ~40% of the screen with no way | ||
| // to shrink it (issue #1067). We scale it down by the space the panel ACTUALLY | ||
| // has on both axes: | ||
| // - width: the terminal minus the caller's padding and any sibling sidebar. | ||
| // - width: the terminal minus the caller's padding and any sibling sidebar | ||
| // that consumes layout width. | ||
| // - height: the terminal minus the fixed chrome that always shares the column | ||
| // with the panel (top spacer + prompt + footer), so a big panel is | ||
| // with the panel (the per-route reserves below), so a big panel is | ||
| // chosen only when it won't crowd the prompt off a short terminal. | ||
| // `medium` (no wordmark) is the common case; `full` only when there's real room. | ||
| // | ||
| // Naming: these are the MINIMUMS a tier requires, matched with strict `<` | ||
| // (`width < FULL_MIN_WIDTH` → not full). MEDIUM_MIN_* is the floor for medium | ||
| // (below → compact); FULL_MIN_* is the floor for full (below → medium). All in | ||
| // terms of AVAILABLE (usable) size, not the raw terminal. | ||
| // Route arithmetic lives here (homeAvailable / sessionAvailable) so the routes | ||
| // and the tests share ONE definition — a test that maps a terminal size to a | ||
| // variant then exercises the real call-site math, not a copy of it. | ||
|
|
||
| export type WelcomePanelVariant = "full" | "medium" | "compact" | ||
|
|
||
| /** | ||
| * Rows the panel must leave for the always-present chrome below/around it (the | ||
| * prompt, the footer, and the home top spacer). Callers subtract this from the | ||
| * terminal height to get the panel's usable height. An estimate — the prompt can | ||
| * grow with multi-line input, but at rest this is the fixed cost. | ||
| */ | ||
| export const PANEL_VERTICAL_RESERVE = 8 | ||
| // --- Chrome reserves (rows the panel must leave for always-present siblings) --- | ||
| // | ||
| // Prompt tree at rest (component/prompt/index.tsx): top rule 1 + inner paddingTop | ||
| // 1 + textarea 1 + separator 1 + agent/model meta row 1 + idle hint row 1 = 6. | ||
| // Measured against the rendered tree, not estimated — the earlier "~4" undercounted | ||
| // it (the meta and hint rows are always in flow). | ||
| const PROMPT_REST_HEIGHT = 6 | ||
|
|
||
| /** Minimum usable size for the medium panel; below either → compact (one line). */ | ||
| // home (routes/home.tsx): top spacer 2 (`<box height={2}>`) + prompt wrapper | ||
| // paddingTop 1 + prompt 6 + home_bottom slot 3 (feature-plugins/home/tips.tsx | ||
| // paddingTop, rendered unconditionally — flexbox shrinks content, not padding) + | ||
| // footer 3 (feature-plugins/home/footer.tsx) = 15. | ||
| export const HOME_VERTICAL_RESERVE = 2 + 1 + PROMPT_REST_HEIGHT + 3 + 3 | ||
| // session (routes/session/index.tsx): two column gaps 2 + paddingBottom 1 + | ||
| // prompt 6 = 9. No top spacer and no footer share this column. | ||
| export const SESSION_VERTICAL_RESERVE = 2 + 1 + PROMPT_REST_HEIGHT | ||
|
|
||
| // Columns the home slot spends on its own left/right padding (2 + 2). Session | ||
| // subtracts the same via sessionAvailable(); both routes import this constant so | ||
| // the value has a single source of truth. | ||
| export const PANEL_HORIZONTAL_PADDING = 4 | ||
| // Width the session sidebar occupies WHEN it consumes layout width (i.e. it is | ||
| // rendered in-flow, not as an overlay). Shared with the route + tests so they | ||
| // can't drift — the drift that produced #1067. | ||
| export const SIDEBAR_WIDTH = 42 | ||
|
|
||
| // --- Thresholds --- | ||
| // MEDIUM_MIN_* is a real FIT requirement: below it the medium panel (~8 rows / | ||
| // ~a title + one/two-line description) would not fit, so drop to the one-line | ||
| // compact. FULL_MIN_* is a product BREAKPOINT, not a fit minimum: the full panel | ||
| // is only ~13 rows, but we require far more available space so the branded | ||
| // wordmark appears only on a genuinely large terminal and never dominates a | ||
| // small one (the #1067 ask). All in AVAILABLE (usable) terms, not the raw | ||
| // terminal, matched with strict `<`. | ||
| export const MEDIUM_MIN_WIDTH = 60 | ||
| export const MEDIUM_MIN_HEIGHT = 16 | ||
| /** Minimum usable size for the full wordmark panel; below either → medium. */ | ||
| export const MEDIUM_MIN_HEIGHT = 8 | ||
| export const FULL_MIN_WIDTH = 110 | ||
| export const FULL_MIN_HEIGHT = 36 | ||
| // ~45-row home terminal / ~39-row session terminal after the reserves above. | ||
| export const FULL_MIN_HEIGHT = 30 | ||
|
|
||
| /** | ||
| * Choose the WelcomePanel layout from the panel's AVAILABLE size — width already | ||
| * minus padding/sidebar, height already minus PANEL_VERTICAL_RESERVE. Not the | ||
| * raw terminal (that's the #1067 bug: a sidebar-narrowed column, or a short | ||
| * minus padding/sidebar, height already minus the route's vertical reserve. Not | ||
| * the raw terminal (that's the #1067 bug: a sidebar-narrowed column, or a short | ||
| * terminal, would still pick `full`). | ||
| */ | ||
| export function welcomePanelVariant(width: number, height: number): WelcomePanelVariant { | ||
| if (width < MEDIUM_MIN_WIDTH || height < MEDIUM_MIN_HEIGHT) return "compact" | ||
| if (width < FULL_MIN_WIDTH || height < FULL_MIN_HEIGHT) return "medium" | ||
| return "full" | ||
| } | ||
|
|
||
| /** Available panel size on the home route for a given terminal size. */ | ||
| export function homeAvailable(terminalWidth: number, terminalHeight: number): { width: number; height: number } { | ||
| return { | ||
| width: terminalWidth - PANEL_HORIZONTAL_PADDING, | ||
| height: terminalHeight - HOME_VERTICAL_RESERVE, | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Available panel size on the session route. Subtracts SIDEBAR_WIDTH whenever the | ||
| * sidebar is open (`sidebarVisible`) — the panel shares the same content-column | ||
| * basis as the messages (session's `contentWidth`), so the two stay aligned. | ||
| * | ||
| * On narrow terminals the sidebar renders as a dimmed full-area overlay rather | ||
| * than in-flow; the content column still narrows uniformly (panel + messages) | ||
| * and both restore to full width when it closes, so we deliberately size to the | ||
| * narrowed column rather than the transient obscured width. | ||
| */ | ||
| export function sessionAvailable( | ||
| terminalWidth: number, | ||
| terminalHeight: number, | ||
| sidebarVisible: boolean, | ||
| ): { width: number; height: number } { | ||
| return { | ||
| width: terminalWidth - (sidebarVisible ? SIDEBAR_WIDTH : 0) - PANEL_HORIZONTAL_PADDING, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: On a narrow session with the sidebar toggled open, the welcome panel is sized as though 42 columns were removed even though the overlay leaves its layout box full width, so medium content can be laid out across the obscured area and appear truncated. The available-width calculation should distinguish the in-flow ( Prompt for AI agents |
||
| height: terminalHeight - SESSION_VERTICAL_RESERVE, | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,7 +26,7 @@ import { useTuiPaths, useTuiTerminalEnvironment } from "../../context/runtime" | |
| import { Spinner } from "../../component/spinner" | ||
| // altimate_change — shared boot box at the top of the session scrollback | ||
| import { WelcomePanel } from "../../component/welcome-panel" | ||
| import { PANEL_VERTICAL_RESERVE } from "../../component/welcome-panel-utils" | ||
| import { PANEL_HORIZONTAL_PADDING, SIDEBAR_WIDTH, sessionAvailable } from "../../component/welcome-panel-utils" | ||
| import { createSyntaxStyleMemo, generateSubtleSyntax, selectedForeground, useTheme } from "../../context/theme" | ||
| import { BoxRenderable, ScrollBoxRenderable, addDefaultParsers, TextAttributes, RGBA } from "@opentui/core" | ||
| import { Prompt, type PromptRef } from "../../component/prompt" | ||
|
|
@@ -272,7 +272,15 @@ export function Session() { | |
| return false | ||
| }) | ||
| const showTimestamps = createMemo(() => timestamps() === "show") | ||
| const contentWidth = createMemo(() => dimensions().width - (sidebarVisible() ? 42 : 0) - 4) | ||
| // altimate_change start — WelcomePanel responsive sizing (#1067). panelAvailable is the single | ||
| // source of the panel's usable size (via welcome-panel-utils, unit-tested); it narrows with the | ||
| // sidebar whenever open — including the dimmed full-area overlay on narrow terminals — so the | ||
| // panel and the messages stay aligned and both restore to full width when it closes. contentWidth | ||
| // (the shared content-column width) derives from it, so the width math has one definition and the | ||
| // two cannot structurally drift. | ||
| const panelAvailable = createMemo(() => sessionAvailable(dimensions().width, dimensions().height, sidebarVisible())) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: The note above states panelAvailable shares contentWidth's content-column basis, and Reply with |
||
| const contentWidth = createMemo(() => panelAvailable().width) | ||
| // altimate_change end | ||
| const providers = createMemo(() => Model.index(sync.data.provider)) | ||
|
|
||
| const scrollAcceleration = createMemo(() => getScrollAcceleration(tuiConfig)) | ||
|
|
@@ -1187,13 +1195,10 @@ export function Session() { | |
| /discover) starts a session. Outside the scrollbox: the bordered panel | ||
| does not paint reliably inside the scroll viewport. */} | ||
| <box flexShrink={0}> | ||
| {/* Size to the panel's real space, not the terminal (#1067): | ||
| contentWidth already subtracts the sidebar + padding; | ||
| -PANEL_VERTICAL_RESERVE leaves room for the prompt + footer. */} | ||
| <WelcomePanel | ||
| availableWidth={contentWidth()} | ||
| availableHeight={dimensions().height - PANEL_VERTICAL_RESERVE} | ||
| /> | ||
| {/* Size to the panel's real space, not the terminal (#1067). | ||
| panelAvailable() subtracts the in-flow sidebar + padding and | ||
| reserves the prompt row (see the memo above). */} | ||
| <WelcomePanel availableWidth={panelAvailable().width} availableHeight={panelAvailable().height} /> | ||
| </box> | ||
| {/* altimate_change end */} | ||
| <scrollbox | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: The padding centralization is incomplete: PANEL_HORIZONTAL_PADDING is used at the home call site, but the session route's contentWidth still hardcodes the bare
- 4rather than referencing the constant (the new doc comment even acknowledges session "applies the same 4 as part of its own content-column math"). If this constant ever changes, the session width math won't follow and drifts out of sync; consider having session/index.tsx import PANEL_HORIZONTAL_PADDING so both routes share one source of truth.Prompt for AI agents