fix(tui): make the welcome panel responsive to terminal size (#1067) - #1069
fix(tui): make the welcome panel responsive to terminal size (#1067)#1069saravmajestic wants to merge 1 commit into
Conversation
The full two-column boot box (block wordmark + description) is ~65 cols wide and ~13 rows tall, so on a typical terminal it ate ~40-50% of the screen with no way to shrink it (#1067). It now scales by terminal size, using the repo's breakpoint idiom (useTerminalDimensions + createMemo, cf. routes/session/permission.tsx, component/upgrade-indicator.tsx) — reactive, so it adapts live on resize: - full — the two-column wordmark box, reserved for large windows (>=110w & >=44h) - medium — title + one condensed line, no wordmark (the common case, ~6 rows) - compact — a single line; the border title already shows the version (<60w or <18h) The breakpoint decision is a pure function in welcome-panel-utils.ts (mirroring upgrade-indicator-utils.ts) so it's unit-testable without the render/sync context; 4 tests cover the thresholds and that the smaller dimension wins. Scope: responsiveness only — no dismiss/collapse behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe TUI welcome panel now selects compact, medium, or full layouts from terminal width and height. New breakpoint utilities classify dimensions, and tests cover threshold behavior. The panel renders condensed content for smaller terminals while retaining the original full layout. ChangesResponsive Welcome Panel
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalDimensions
participant WelcomePanel
participant welcomePanelVariant
participant CompactLayout
participant MediumLayout
participant FullLayout
TerminalDimensions->>WelcomePanel: provide width and height
WelcomePanel->>welcomePanelVariant: classify terminal dimensions
welcomePanelVariant-->>WelcomePanel: return panel variant
WelcomePanel->>CompactLayout: render compact content
WelcomePanel->>MediumLayout: render medium content
WelcomePanel->>FullLayout: render full content
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/tui/src/component/welcome-panel-utils.ts">
<violation number="1" location="packages/tui/src/component/welcome-panel-utils.ts:18">
P3: Constant naming `COMPACT_MAX_WIDTH` / `COMPACT_MAX_HEIGHT` / `MEDIUM_MAX_WIDTH` / `MEDIUM_MAX_HEIGHT` conflicts with the strict inequality in `welcomePanelVariant`. The suffix `_MAX_` conventionally implies an inclusive upper bound (so `width ≤ COMPACT_MAX_WIDTH` would be compact), but the function uses strict `<`, meaning at the exact value the variant flips to the next tier. For example, `width = COMPACT_MAX_WIDTH (60)` returns medium, not compact, and `width = MEDIUM_MAX_WIDTH (110)` returns full, not medium. Someone tuning these thresholds later could easily mis-predict behavior.
Recommend renaming to convey an exclusive boundary, e.g. `COMPACT_BELOW_WIDTH`, `COMPACT_BELOW_HEIGHT`, `MEDIUM_BELOW_WIDTH`, `MEDIUM_BELOW_HEIGHT` (mirroring the PR description's `<60w` / `<18h` / `<110w` / `<44h` language).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // ~40% of the screen, which is the problem #1067 is about. So `medium` (no | ||
| // wordmark, ~6 rows) is the common case; `full` only when there's real room. | ||
| /** Below these, drop to the single-line compact panel. */ | ||
| export const COMPACT_MAX_WIDTH = 60 |
There was a problem hiding this comment.
P3: Constant naming COMPACT_MAX_WIDTH / COMPACT_MAX_HEIGHT / MEDIUM_MAX_WIDTH / MEDIUM_MAX_HEIGHT conflicts with the strict inequality in welcomePanelVariant. The suffix _MAX_ conventionally implies an inclusive upper bound (so width ≤ COMPACT_MAX_WIDTH would be compact), but the function uses strict <, meaning at the exact value the variant flips to the next tier. For example, width = COMPACT_MAX_WIDTH (60) returns medium, not compact, and width = MEDIUM_MAX_WIDTH (110) returns full, not medium. Someone tuning these thresholds later could easily mis-predict behavior.
Recommend renaming to convey an exclusive boundary, e.g. COMPACT_BELOW_WIDTH, COMPACT_BELOW_HEIGHT, MEDIUM_BELOW_WIDTH, MEDIUM_BELOW_HEIGHT (mirroring the PR description's <60w / <18h / <110w / <44h language).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/tui/src/component/welcome-panel-utils.ts, line 18:
<comment>Constant naming `COMPACT_MAX_WIDTH` / `COMPACT_MAX_HEIGHT` / `MEDIUM_MAX_WIDTH` / `MEDIUM_MAX_HEIGHT` conflicts with the strict inequality in `welcomePanelVariant`. The suffix `_MAX_` conventionally implies an inclusive upper bound (so `width ≤ COMPACT_MAX_WIDTH` would be compact), but the function uses strict `<`, meaning at the exact value the variant flips to the next tier. For example, `width = COMPACT_MAX_WIDTH (60)` returns medium, not compact, and `width = MEDIUM_MAX_WIDTH (110)` returns full, not medium. Someone tuning these thresholds later could easily mis-predict behavior.
Recommend renaming to convey an exclusive boundary, e.g. `COMPACT_BELOW_WIDTH`, `COMPACT_BELOW_HEIGHT`, `MEDIUM_BELOW_WIDTH`, `MEDIUM_BELOW_HEIGHT` (mirroring the PR description's `<60w` / `<18h` / `<110w` / `<44h` language).</comment>
<file context>
@@ -0,0 +1,29 @@
+// ~40% of the screen, which is the problem #1067 is about. So `medium` (no
+// wordmark, ~6 rows) is the common case; `full` only when there's real room.
+/** Below these, drop to the single-line compact panel. */
+export const COMPACT_MAX_WIDTH = 60
+export const COMPACT_MAX_HEIGHT = 18
+/** At/above these, show the full two-column wordmark panel; below → medium. */
</file context>
sahrizvi
left a comment
sahrizvi
left a comment
There was a problem hiding this comment.
Review — responsiveness change works, but two things block
The direction is right and the decomposition is clean. Splitting the breakpoint decision into a pure, exported function mirroring upgrade-indicator-utils.ts is the right call, the createMemo + useTerminalDimensions wiring matches the idiom in upgrade-indicator.tsx:14 and permission.tsx:450 (genuinely reactive, no destructured props), and the full branch is a faithful move of the original markup so the large-terminal path is low-risk. Theme tokens are unchanged, there's no dead code, and the scope discipline — explicitly declining to bundle dismiss/collapse — is good.
Two blockers, both left inline:
- The breakpoint is computed from terminal width, but the panel never gets terminal width. In the session view with the sidebar open,
fullis still selected when the panel only has ~84–94 columns, and the panel swells back to ~44% of the screen — the exact bug #1067 reports. (welcome-panel.tsx:30) - The tests don't prove the width threshold the change is built around. Deleting
width < MEDIUM_MAX_WIDTHfrom the source leaves all four tests green. (welcome-panel-utils.test.ts:17)
Plus three minors inline: the compact variant's wrapMode="word" breaks its own single-line invariant, medium is ~9 rows at its lower bound rather than ~6, and the *_MAX_* constants are really minimums for the tier above.
MINOR — no render test for the component, though the repo has the harness
The pure function is tested; the part that can actually break isn't — that useTerminalDimensions is wired up, that the memo survives a resize, that each variant fits its row budget. test/feature-plugins/home-onboarding.test.tsx shows testRender takes an explicit { width, height }, so a three-case render test is cheap.
"existing home-onboarding render test still passes" is true but says nothing about this component — that test renders at 120×4 and never mounts WelcomePanel. A render test at session-like widths with the sidebar visible is also the only thing that would have caught blocker #1.
NIT — contradictory height figures inside one file
welcome-panel-utils.ts:4-5 says the full box is "~8 rows tall"; lines 13-16 say "~13 rows tall". Both are defensible (inner content vs. bordered box) but the reader can't tell which is which. Pick one framing.
NIT — structural cleanups worth folding in
welcome-panel.tsx:31-33—compact(),medium(),full()are three memos over one memo feeding three mutually-exclusive<Show>blocks (lines 51, 60, 78).routes/session/permission.tsxandsession/index.tsx:1349-1358both use<Switch><Match when={…}>for exactly this; switching makes the exclusivity structural rather than incidental.- The fixed
width={65}left column (line 82) is ~15 columns wider than its content —src/logo.tsis 32 + 1 gap + 17 = 50 columns, and the title line is 24. Trimming to ~52 would letfullengage on a smaller terminal and would soften blocker #1. - "Connect your AI model to start." is duplicated verbatim at lines 54, 71 and 122 — worth one module constant. (The three descriptions are deliberately different copy, not duplication.)
Open design question, not a defect
MEDIUM_MAX_HEIGHT = 44 means the wordmark is effectively never seen on a laptop — it needs an external monitor or a full-screen tall window. That may well be intentional ("full only when there's real room"), but it's worth an explicit product decision, since it makes the branded panel dead code for most users. Note the tension with blocker #1: lowering the threshold widens the range where the sidebar case misfires, so fix that first.
Missing tests, collected
- Compact→medium boundary at exactly
(60, 18),(60, 40),(120, 18)— the entire lower boundary. (109, 44)— the only case that isolates thefullwidth gate.- Degenerate sizes
(0, 0),(1, 1)— currently returncompact, which is correct, but nothing pins it. - Render tests per variant, with height-budget assertions and a resize transition.
- Session layout with the sidebar visible — where blocker #1 lives.
| const ready = useReady() | ||
| const dimensions = useTerminalDimensions() | ||
|
|
||
| const variant = createMemo(() => welcomePanelVariant(dimensions().width, dimensions().height)) |
There was a problem hiding this comment.
MAJOR (Bug / Logic Error) — the breakpoint reads terminal width, but the panel never gets terminal width.
dimensions().width is the whole terminal. This panel is never that wide:
routes/home.tsx:104wraps it in<box … paddingLeft={2} paddingRight={2}>→width - 4.routes/session/index.tsx:1181-1189— same padding, and it sits in the content column of aflexDirection="row"layout whose sibling is a 42-columnSidebar(routes/session/sidebar.tsx:30), auto-shown wheneverwidth > 120(session/index.tsx:267-272).
The correct quantity already exists four lines from the call site and is unused here:
// session/index.tsx:274
const contentWidth = createMemo(() => dimensions().width - (sidebarVisible() ? 42 : 0) - 4)The full branch has a hard width={65} flexShrink={0} left column (line 82), so it needs ~105 usable columns:
| Terminal | Sidebar | Panel width | Variant chosen | Right-column text |
|---|---|---|---|---|
| 110×44 (home) | n/a | 106 | full | ~33 cols — fine |
| 130×50 (session) | visible (auto, >120) | 84 | full | ~11 cols |
| 140×50 (session) | visible | 94 | full | ~21 cols |
At 140×50 the two description paragraphs wrap to ~13 rows and the panel lands at ~22 rows — 44% of the screen, which is the complaint in #1067. Every terminal roughly 121–151 columns wide and ≥44 rows, in a session with the sidebar visible, reproduces the original bug.
Not a regression (before this change full rendered at every size), but the fix misses its stated goal in a configuration a user on a large monitor will hit.
Fix:
export function WelcomePanel(props: { availableWidth?: number } = {}) {
const dimensions = useTerminalDimensions()
const width = createMemo(() => props.availableWidth ?? dimensions().width - 4)
const variant = createMemo(() => welcomePanelVariant(width(), dimensions().height))// session/index.tsx
<WelcomePanel availableWidth={contentWidth()} />contentWidth is already reactive to sidebarVisible(), so toggling the sidebar re-picks the variant for free.
Belt-and-braces regardless: give the full left column flexShrink={1} or a maxWidth, or a minWidth on the right column, so a mis-sized full degrades instead of collapsing to a text gutter.
|
|
||
| test("a narrow OR short terminal drops the wordmark (medium)", () => { | ||
| expect(welcomePanelVariant(80, 24)).toBe("medium") | ||
| expect(welcomePanelVariant(MEDIUM_MAX_WIDTH - 1, 40)).toBe("medium") // narrow but tall |
There was a problem hiding this comment.
MAJOR (Testing) — these tests don't prove what the PR description claims they prove.
The description says "4 tests cover the thresholds and that the smaller dimension wins." Two separate problems mean they don't.
(a) The full-tier width gate is entirely unexercised. Mutation-test the pure function — delete the width gate from the medium branch:
if (width < MEDIUM_MAX_WIDTH || height < MEDIUM_MAX_HEIGHT) return "medium"
// becomes
if (height < MEDIUM_MAX_HEIGHT) return "medium"…and all four tests still pass. The other three gates (full height, compact width, compact height) are each caught by an equivalent mutant, so this is the one real hole.
This line is the culprit: the comment says "narrow but tall", but 40 < MEDIUM_MAX_HEIGHT (44), so height forces the medium result, not width. "Tall" isn't tall enough to isolate the width axis.
(b) The compact→medium boundary is untested, and test 4 (lines 27-31) duplicates test 3. Line 12 does an "at the threshold" check for the full tier; the compact tier has none, so a < → <= slip on either compact gate ships silently. And test 4's (200, 17) / (59, 200) take the same two branches as test 3's (120, 17) / (59, 40) — only the non-deciding dimension differs, and it stays on the same side of every threshold. No mutant is caught by test 4 that test 3 doesn't already catch.
Fix — repair this assertion and replace test 4 with the boundaries that are actually missing:
expect(welcomePanelVariant(MEDIUM_MAX_WIDTH - 1, MEDIUM_MAX_HEIGHT)).toBe("medium") // narrow, tall enough
expect(welcomePanelVariant(COMPACT_MAX_WIDTH, COMPACT_MAX_HEIGHT)).toBe("medium")
expect(welcomePanelVariant(COMPACT_MAX_WIDTH, 40)).toBe("medium")
expect(welcomePanelVariant(120, COMPACT_MAX_HEIGHT)).toBe("medium")
expect(welcomePanelVariant(0, 0)).toBe("compact")| {/* compact — one line; the border title already carries the version */} | ||
| <Show when={compact()}> | ||
| <box paddingLeft={2} paddingRight={2} width="100%"> | ||
| <text fg={ready() ? theme.text : theme.primary} wrapMode="word" width="100%"> |
There was a problem hiding this comment.
MINOR (Bug) — wrapMode="word" means the "single line" invariant doesn't actually hold.
The compact string is 31 characters ("Connect your AI model to start."). Subtract 2 border + 4 panel padding + 4 home-route gutter and it wraps at roughly 40 columns; near 20 columns it can reach ~5 rows. compact is reachable at any width below 60, so the "a single line; ~3 rows" claim on line 24 is aspirational rather than enforced.
Fix — enforce it structurally:
<text fg={ready() ? theme.text : theme.primary} wrapMode="none" truncate width="100%">| <box paddingLeft={2} paddingRight={2} paddingTop={1} paddingBottom={1} gap={0} width="100%"> | ||
| <text fg={theme.text} attributes={TextAttributes.BOLD}> | ||
| Welcome to Altimate Code | ||
| </text> | ||
| <text fg={theme.text} wrapMode="word" width="100%"> | ||
| It gives your AI real context — column-level lineage, SQL analysis, dbt, and live warehouse metadata — so it | ||
| reasons about your data instead of guessing. | ||
| A data-engineering harness that gives your AI real context — column-level lineage, SQL analysis, dbt, and | ||
| live warehouse metadata. | ||
| </text> | ||
| {/* CTA only until a model is connected — stale afterwards */} | ||
| <Show when={!ready()}> | ||
| <text fg={theme.primary} wrapMode="word" width="100%"> | ||
| Connect your AI model to start. | ||
| </text> | ||
| </Show> | ||
| </box> |
There was a problem hiding this comment.
MINOR (Design) — medium is ~9 rows at its own lower bound, not the advertised ~6.
At the smallest terminal that still selects medium (60×18, home route → 56-wide panel, 50-column text area):
| Element | Rows |
|---|---|
| border | 2 |
paddingTop / paddingBottom |
2 |
| title | 1 |
| description (~130 chars at 50 cols) | 3 |
| CTA (disconnected) | 1 |
| total | ~9 |
That's 50% of an 18-row terminal — the same ratio #1067 complains about, in a different layout. Even at minimum intrinsic size the disconnected medium panel needs 7 rows, so the "~6 rows" figure in welcome-panel-utils.ts:16 isn't reachable while the CTA is present.
Raise COMPACT_MAX_HEIGHT, drop the vertical padding here, or shorten the description so it fits one or two lines at 50 columns.
| /** Below these, drop to the single-line compact panel. */ | ||
| export const COMPACT_MAX_WIDTH = 60 | ||
| export const COMPACT_MAX_HEIGHT = 18 | ||
| /** At/above these, show the full two-column wordmark panel; below → medium. */ | ||
| export const MEDIUM_MAX_WIDTH = 110 | ||
| export const MEDIUM_MAX_HEIGHT = 44 |
There was a problem hiding this comment.
MINOR (Code Quality / Documentation) — the *_MAX_* names are actually minimums for the tier above.
Every comparison in welcomePanelVariant is a strict <, so:
COMPACT_MAX_WIDTH = 60→ a 60-wide terminal is medium, not compact. 60 is the minimum width formedium.MEDIUM_MAX_WIDTH = 110→ 110 is the minimum width forfull.
The doc comment on line 20 ("At/above these, show the full…") already describes minimum semantics and contradicts the name on line 21. These constants are exported and consumed by the tests, so the wrong name propagates — and it's the direct cause of several "is this an off-by-one?" readings that turn out to be wrong.
Suggest MEDIUM_MIN_WIDTH / MEDIUM_MIN_HEIGHT / FULL_MIN_WIDTH / FULL_MIN_HEIGHT.
2 participants
Closes #1067.
What & why
The welcome panel (
home_logoslot) rendered a fixed two-column boot box — blockALTIMATE CODEwordmark on the left (~65 cols), "What is Altimate Code" on the right — that never shrank. On a typical terminal it took ~40–50% of the screen and couldn't be reduced, which is what #1067 reports.What this does
It now scales with terminal size, reactively (adapts live on resize), using the repo's existing breakpoint idiom (
useTerminalDimensions+createMemo, as inroutes/session/permission.tsxandcomponent/upgrade-indicator.tsx). Both axes gate each step — the wordmark is wide and tall:≥110w & ≥44h<60w or <18hOn an everyday laptop terminal (e.g. 106×31, 80×24) you now get medium; the full branded panel returns only when there's genuinely room for it.
Notes
welcome-panel-utils.ts(mirroringupgrade-indicator-utils.ts), so it's unit-testable without the render/sync context. 4 tests cover the thresholds and that the smaller dimension wins.welcome-panel-utils.ts, easy to tune.Verified
tsgo --noEmitclean · 4 unit tests pass · existinghome-onboardingrender test still passes · oxlint 0/0 · prettier.🤖 Generated with Claude Code
Summary by cubic
Make the TUI welcome panel responsive to terminal size so it no longer takes over small terminals. Addresses AI-1067 by switching between full, medium, and compact layouts and updating live on resize.
useTerminalDimensions+createMemo; both width and height decide the layout.welcome-panel-utilswithwelcomePanelVariantand thresholds; 4 unit tests cover edge cases.welcome-panel.tsxto render: full (two-column, ≥110w & ≥44h), medium (no wordmark), compact (single line, <60w or <18h); no changes to CTA/dismiss behavior.Written for commit 721fee6. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests