Size the editor viewport to the live terminal height - #95
Conversation
EditorView always rendered exactly 20 rows because EditorArea never passed a viewportHeight prop at all, so EditorView silently fell back to its own hardcoded DEFAULT_VIEWPORT_HEIGHT regardless of the real terminal size. EditorArea now reads the live terminal height (useLiveTerminalHeight, wrapping @opentui/react's resize event without crashing when no renderer is mounted) and subtracts exactly the chrome it renders this pass - tab bar, find widget, Shell's sibling Panel, and the status bar - via a new pure computeEditorViewportHeight (viewport.ts), then threads the result into EditorView's existing viewportHeight prop. Each chrome height is derived from the same condition that decides whether that region renders at all, so it can't drift from what's actually drawn. When no live terminal is available, viewportHeight is left undefined and EditorView keeps falling back to its own constant, unchanged. Added a regression test that renders a 60-line document into a terminal taller than the old 20-row cap and asserts lines past index 20 are visible, plus unit tests for computeEditorViewportHeight covering chrome combinations and the minimum-1-row clamp. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WELSsojQQL1cTAR5iUUsTK
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Walkthrough
Changes実測ビューポート測定
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk:⚪ Minimal · up to The editor viewport now follows the live terminal height while preserving the existing fallback and explicit prop behavior; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant CliRenderer
participant EditorArea
participant ViewportCalculator
participant EditorView
CliRenderer->>EditorArea: 端末高とリサイズイベントを提供
EditorArea->>ViewportCalculator: 端末高とクローム高さを渡す
ViewportCalculator-->>EditorArea: viewportHeightを返す
EditorArea->>EditorView: viewportHeightを渡す
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
🚀 Post-Merge Actions
Comment |
Uh oh!
There was an error while loading. Please reload this page.
🚀 Post-Merge Actions
|
fix#92
The editor showed 20 rows no matter how tall the terminal was.
Root cause
EditorViewtakes an optionalviewportHeightprop and falls back toDEFAULT_VIEWPORT_HEIGHT = 20.shell.tsx'sEditorAreanever passed that prop, so the fallback was the only value it ever used.editorView.tsx's own TSDoc said as much under "Scope note onviewportHeight" — live measurement was deferred to a later task. This is that task.The fix
computeEditorViewportHeight(terminalHeight, chrome)— a pure function inviewport.ts, besidecomputeVisibleLineRange— subtracts the chrome from the terminal height and clamps to a minimum of 1 row.EditorAreafeeds it the live terminal height and the heights of exactly the chrome this render actually draws, each derived from the same condition that decides whether that region renders at all: the tab bar (tabs.length > 0), the find widget (the samefind && isFindOpen && props.findServicethe JSX uses),Shell's siblingPanel, and the status bar. Nothing is hardcoded that could drift from what is drawn.EditorView's prop keeps its shape and its prop-wins behaviour;DEFAULT_VIEWPORT_HEIGHTis now only the no-live-terminal fallback, and the TSDoc says that instead of describing the deferral.useAppContext()rather thanuseTerminalDimensions()useTerminalDimensions()cannot be used here. It callsuseRenderer(), which throws"Renderer not found."when no renderer is mounted (@opentui/react@0.1.107,index.js:50-55,72-83).EditorAreais constructed in tests outside a liveCliRenderer, so that throw would break them rather than degrade.useAppContext()returns{ renderer }which may beundefined, souseLiveTerminalHeight()subscribes toCliRenderEvents.RESIZEwhen a renderer exists and returnsundefinedwhen one does not — leavingviewportHeightunset andEditorViewon its existing constant. It mirrorsuseLiveTheme'sthemeService === undefinedfallback, including re-syncing to the current height before subscribing so a resize landing between render and effect is not missed.TAB_BAR_HEIGHT = 3was measured against the vendored headless renderer rather than assumed:@opentui/core'sTabSelectRenderabledefaults bothshowUnderlineandshowDescriptiontotrue.Validation
bun test1748 pass / 1 skip / 0 fail;bunx tsc --noEmitclean;bun run lintclean. No existing snapshot needed regenerating —editorView.snapshot.test.tsxalways passes an explicitviewportHeight, so prop-wins behaviour is confirmed rather than assumed.The regression test reproduces the bug exactly when the fix is reverted. Removing
viewportHeight={viewportHeight}fromEditorAreafails it, with the captured frame in a 50-row terminal showingline0..line19and then 30 blank rows:Removing the
Math.max(1, ...)clamp fails the clamp unit tests withExpected: 1, Received: 0. Both mutations were re-run independently of the implementing agent.🤖 Generated with Claude Code
https://claude.ai/code/session_01WELSsojQQL1cTAR5iUUsTK
Generated by Claude Code
Summary by CodeRabbit
新機能
バグ修正