fix(ui): keep selections visible without host colors - #2710
Conversation
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe selection background calculation now falls back to the palette background when the host does not report one. The change includes contrast tests for dark and light palettes and an unreleased changelog entry. ChangesSelection Background Handling
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/ui/panes.rs (1)
1612-1626: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStrengthen the fallback regression test.
The test only checks
abs(selected_luminance - base_luminance) > 0.08. A fixed contrasting color could satisfy this for both inputs. Compare the unknown-host result with the same palette used as a knownTerminalTheme.background, then assert that dark inputs become lighter and light inputs become darker.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f657565a-2f59-4ba4-af93-c02b94c95939
📒 Files selected for processing (2)
docs/next/CHANGELOG.mdsrc/ui/panes.rs
|
CodeRabbit review 4915525043: agreed, strengthened the regression to match the known-host tint path and assert the dark/light direction in bb85fdd. |
Greptile SummaryThe PR improves selection visibility when the outer terminal does not report its background color.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/ui/panes.rs | Uses the palette background as the selection-tint source when the host background is unavailable and adds focused fallback tests. |
| docs/next/CHANGELOG.md | Documents improved selection visibility in terminals that do not report their background color. |
Reviews (2): Last reviewed commit: "Merge branch 'master' into akbash/2708-m..." | Re-trigger Greptile
Summary
Checks
cargo test automatic_selection_background_contrasts_when_host_background_is_unknowncargo test 'ui::panes::tests'just ci 'all() - test(live_server_holds_one_pty_master_fd_per_pane)'(3418 tests passed; excluded test cannot discover its successful replacement process because Kennel's isolatedCARGO_TARGET_DIRis outside the checkout)just windows-lintjust checkreached the same unrelated live-handoff harness incompatibility; server logs show the handoff and replacement process completed successfully.refs #2708