fix(sidebar): keep collapsed status dot visible past workspace 9 - #2218
fix(sidebar): keep collapsed status dot visible past workspace 9#2218joonhwan wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughCollapsed sidebar workspace indices now use two character positions. A regression test verifies that the tenth workspace displays its working indicator with the expected color. ChangesCollapsed Sidebar Alignment
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThe PR adjusts collapsed workspace numbering so two-digit positions leave room for the status dot and adds a regression test for position 10.
Confidence Score: 4/5The remaining three-digit workspace case should be addressed before merging because it leaves the status indicator hidden for a reachable workspace count. The two-column minimum formatting fixes positions 10–99, but an unconstrained workspace list can reach position 100, where the three-digit label fills the collapsed row and clips the appended status icon. Files Needing Attention: src/ui/sidebar.rs
|
| Filename | Overview |
|---|---|
| src/ui/sidebar.rs | Corrects status rendering for positions 10–99 and adds focused coverage, but three-digit positions still consume the status column. |
Reviews (1): Last reviewed commit: "fix(sidebar): keep collapsed status dot ..." | Re-trigger Greptile
| Paragraph::new(Line::from(vec![ | ||
| Span::styled(format!("{}", visible_idx + 1), num_style), | ||
| Span::styled(" ", row_style), | ||
| Span::styled(format!("{:<2}", visible_idx + 1), num_style), |
There was a problem hiding this comment.
Confirmed the clipping at 100+, but it is not a regression from this PR — rendering is identical before and after. Probe run on both revisions:
| position | before | after |
|---|---|---|
| 1 | 1 · |
1 · |
| 10 | 10 |
10· |
| 99 | 99 |
99· |
| 100 | 100 |
100 |
The content area is 3 columns wide (COLLAPSED_WIDTH is 4, minus the separator column), so a 3-digit position leaves no column for the dot either way. Showing both would need a new display rule — abbreviating the number or widening COLLAPSED_WIDTH — and the agent list directly below uses the same {position:<2} convention, so both lists would have to change together.
That is a visual-language change, which CONTRIBUTING.md asks to raise in a discussion rather than fold into a focused fix: PR. Keeping this PR to the reported 10..=99 range.
The collapsed sidebar hides the status dot for workspaces at position 10
and above.
refs #2216
Cause
The collapsed sidebar content area is 3 columns wide (
COLLAPSED_WIDTHis 4,minus the separator column). The workspace row drew the position number at its
natural width plus a literal space span before the dot, so a two-digit position
produced
10 ·— 4 columns — and the dot fell outside the row rect.The agent list directly below already pads its position to two columns, so it
was not affected.
Change
Pad the workspace position to two columns and drop the separate space span, so
the workspace rows match the agent rows below them. Single-digit rows keep the
dot in the same column as before.
Verification
On Windows:
scripts/windows_check.ps1 -Mode checkpasses:cargo fmt --check,cargo clippy -D warnings, thewindows_(143) andserver::client_transport::tests(22) filters, andcargo build.cargo test --bin herdr ui::— 145 passed.cargo test --bin herdr sidebar— 123 passed.I could not run
just cion this machine: thetests/integration suite isUnix-only and does not compile on Windows, and a full
cargo test --bin herdrrun hangs here. Leaving those to CI.
New test
collapsed_sidebar_keeps_workspace_status_visible_for_two_digit_positionsfails without the change (
left: " ", right: "●").No docs change needed — the collapsed sidebar row layout is not described in
the docs.