Repository navigation
fix(ui): focus previous panel after close - #2196
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 48 minutes. View limit details
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 |
Xuepoo
left a comment
There was a problem hiding this comment.
Independent review — APPROVE
Reviewer: separate agent (hermes guard delegate), verdict recorded via comment (single GitHub identity cannot self-approve).
Scope: commit 7483c7d — crates/bitty-terminal/src/chrome_keys.rs, 1 file, +70/-0.
Checks run (exact):
git show 7483c7d4 --stat→ 1 file changed, only chrome_keys.rs.git show 7483c7d4 -- crates/bitty-terminal/src/chrome_keys.rsgit log --oneline -3 7483c7d4rg -n 'leaf_order|predecessor|previous.*leaf|close_view' crates/bitty-terminal/src/chrome_keys.rsrg -n 'fn close_view|fn leaf|close_view_focuses_previous_leaf|wraps_first_leaf' crates/bitty-terminal/src/chrome_keys.rs
Findings:
previous_leaf_after_close(chrome_keys.rs:620) returnsleaf_ids[position - 1]as predecessor and wraps only whenposition == 0(.rev().find(|id| *id != focused)) — no off-by-one;focusedcan never be returned.- Focus is set before the layout close:
self.runtime.set_focus(previous)inside theif close_focused_leaf(...)arm ahead of theset_layoutinstall (chrome_keys.rs:1566-1571), so primary ownership re-homes to the same survivor. - Scope is exactly one file; whole-window close / workspace-prune / floating / pinned / zoom arms (lines 1442/1514/1535) untouched.
- Regression tests are present and meaningful:
chrome_close_view_focuses_previous_leaf_in_stable_order(5252) andchrome_close_view_wraps_first_leaf_focus_to_last_survivor(5278) assert concreteleaf_ids()/focused_view()/primary_view()after a realapply_chrome_action(ChromeAction::CloseView). - MSRV 1.85 OK (plain
if let, no let-chain); no hardcoded host/env literals.
Caveat: the reviewer's checkout was not at head, so the two tests were not executed in the review run; approval rests on the committed diff plus fully green CI on head SHA 7483c7d (Quality gates 1/2, MSRV, Windows, macOS, Linux X11/Wayland, Supply chain, Analyze, CodeQL all pass).
Priority: P1 | Area: ui | Labels: fix, P1, area:ui | Milestone: v0.1.0 | RFC: OQ-036 | Task: CTX-1300
Summary
Verification
cargo fmt --all -- --checkgit diff --checkcargo test -p bitty-terminal --bin bitty chrome_close_view_focuses_previous_leaf --lockedcargo test -p bitty-terminal --bin bitty chrome_close_view_wraps_first_leaf_focus --locked