fix(tui): fill alt-screen frame with theme panel background - #850
fix(tui): fill alt-screen frame with theme panel background#850euxaristia wants to merge 2 commits into
Conversation
Blank and cleared cells fell back to the terminal's own default background instead of the theme's panel color, flashing white on light themes during resize or when ClearScreen erases the frame. Wrap the rendered content in zeroTheme.panel sized to the full frame so every cell is explicitly painted.
|
Warning Review limit reached
Next review available in: 59 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAlt-screen rendering wraps view content in a full-size theme panel at the current terminal dimensions. Blank and cleared regions use the configured theme background. ChangesAlt-screen rendering
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/tui/model.go`:
- Around line 2774-2776: After applying the panel in the model view-rendering
flow, update the existing tea.View instance by calling view.SetContent(content).
Do this within the width and height condition after zeroTheme.panel renders the
content, ensuring tea.View.Content reflects the themed panel output rather than
the original string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 98e2fd70-7bb7-4ffb-8885-04b6ca0fa51b
📒 Files selected for processing (1)
internal/tui/model.go
tea.NewView stores the original string; reassigning the local content variable left the view unchanged. Call SetContent after the panel render so the alt-screen frame uses the themed output.
|
Addressed CodeRabbit finding in
@coderabbitai full review |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 59 minutes. |
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Approving. The diagnosis matches what I see: view.BackgroundColor only paints the cells the view actually covers, so blank and padding cells — and anything ClearScreen erases — fell through to the terminal default, which is white on a light terminal. Wrapping the content in the theme panel at full frame size paints every cell, which is the right fix.
I did check the thing that worries me about wrapping already-laid-out content, since the TUI does its own wrapping and a second pass could corrupt the layout. Rendering through zeroTheme.panel.Width(40).Height(10):
short lines 1->10 widest 2->40
exact width lines lines 5->10 widest 40->40
more lines than h lines 16->17 widest 1->40
over-wide line lines 1->10 widest 55->40
The two cases I was worried about are both fine. Content taller than the frame is not truncated (16 lines stayed 16, not clipped to 10), so nothing gets hidden on a full screen. And lines that are already exactly the frame width are not re-wrapped, so the normal path is untouched.
One behaviour change worth naming, not blocking: a line wider than the frame now wraps where it previously ran off and was clipped by the terminal. That is arguably better, but it is a change — if anything in the TUI deliberately emits a line wider than m.width expecting the terminal to clip it, that content will now reflow and push everything below it down by a line. I could not find a case that does, so I am not asking you to change it; just worth knowing which behaviour you are choosing if a layout oddity shows up later on a narrow terminal.
The m.width > 0 && m.height > 0 guard is the right call too — rendering at zero size before the first WindowSizeMsg would be the obvious way to break this.
2 participants
Summary
zeroTheme.panelsized to the full frame so every cell is explicitly painted, not just the cellsview.BackgroundColorcoversTest plan
Summary by CodeRabbit