fix: reclaim wasted rows below the shortcut footer - #55
Merged
Merged
Conversation
The ShortcutFooter always renders at least 1 row (the "? for shortcuts" text), but getShortcutFooterHeight returns 0 when shortcuts are hidden. This left 1 unaccounted row creating a gap at the bottom of the terminal. Fix by adding +1 to the footer height calculation when showShortcuts is false, in both the main page and logs page height calculations.
…m rows
The previous fix only addressed 1 of 3 wasted rows. Two additional issues:
1. headerHeight was 4 in MainPage, but the View header only renders 2 rows
(Curse v{version} and Config: {filename}). This wasted 2 rows.
2. Math.max(4, availableForLogs - 1) added an extra unused row as a buffer.
Removed since the math now adds up exactly to terminalHeight.
3. LogPage used 6 as the fixed-row offset, but the actual fixed overhead
is 3 (View header 2 + LogPage title 1). Also fixed sign on the
showShortcuts adjustment - hidden footer takes 1 row, so LogTable
should be smaller by 1, not larger.
Updated normalMinHeight in View.tsx to match.
wu-json
changed the title
The TUI height calculations were previously inline in MainPage.tsx and LogPage.tsx and mixed with ink hooks, making them hard to unit test. This extracts the math into src/ui/layout.ts as pure functions: - getShortcutFooterColumns / getShortcutFooterHeight - computeMainPageLayout - computeLogPageLayout getShortcutFooterHeight now returns 1 (instead of 0) when shortcuts are hidden, accurately reflecting the rendered "? for shortcuts" row. This removes the need for the +1 / -1 adjustments at every call site. Adds src/ui/layout.test.ts covering: - Column thresholds for the footer - Footer returns 1 row when collapsed (regression) - MainPage and LogPage heights summing to exactly terminalHeight in collapsed, expanded, and search modes (the core invariant that broke) - Min log preview height clamping in tight terminals - Expanding shortcuts shrinks the log area by the footer delta
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Eliminate ~3 wasted rows between the shortcut footer and the bottom of the terminal in both the main page and the log view, and add regression tests.
Commentary
There were three separate height-accounting bugs causing the bottom of the terminal to look empty:
MainPage.tsx:headerHeightwas hardcoded to4, but the View header only renders 2 rows (Curse v{version}andConfig: {filename}). This wasted 2 rows.MainPage.tsx:Math.max(4, availableForLogs - 1)had an extra- 1safety buffer that wasted another row.ShortcutFooter:getShortcutFooterHeight()returned0when shortcuts were hidden, but the component still renders the? for shortcutstext (1 row).LogPage.tsx: The fixed-row offset was6but the actual overhead is3(View header 2 + LogPage title 1), and the sign on the collapsed-footer adjustment was inverted.After the fix, the math adds up to exactly
terminalHeightin both views:2 (header) + (N+3) (table) + logPreview + 1 (footer) = T2 (header) + 1 (title) + logTable + 1 (footer) = TTo prevent regressions and improve testability, the layout math has been extracted into a new pure module at
src/ui/layout.ts:getShortcutFooterColumns/getShortcutFooterHeightcomputeMainPageLayoutcomputeLogPageLayoutgetShortcutFooterHeightnow returns1(instead of0) when shortcuts are hidden, which removes the need for+1/-1fixups at every call site.MainPage,LogPage,ShortcutFooter, andViewall import from the new module so the constants stay in sync.Tests
src/ui/layout.test.tsadds 18 new tests covering:terminalHeightin collapsed, expanded, and search modes — this is the core invariant that was broken