From 4089864d3f441d895f54ac83e52ee1c4eee30c19 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Tue, 8 Sep 2026 14:56:15 +0800 Subject: [PATCH] fix(ui): let a folded sidebar group hide its active row too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fold left the active tab's row on screen, so folding the group you are working in drew a shut chevron with one row hanging under it and a header counting rows that were not there — it reads as a list that failed to load, not as a group you closed. The exception existed to keep Cmd-T inside a folded group visible, since `spawn_group` seeds a new tab with the group it came from. That cost is taken instead: the pane area shows the fresh shell and the header count goes up, and the row waits for the group to be opened. Auto-unfolding on spawn was the other option and is worse — it only fires when the repo probe already hit the cache, so a cold tab parks in Scratch and moves into its group later without passing through it, and a magic that works half the time is harder to read than none. Claude-Session: https://claude.ai/code/session_01XD6R419Hy1CV1CeVZSRBf7 --- src/ui/tab_sidebar.rs | 47 +++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/src/ui/tab_sidebar.rs b/src/ui/tab_sidebar.rs index 173b0c19..9b8921b1 100644 --- a/src/ui/tab_sidebar.rs +++ b/src/ui/tab_sidebar.rs @@ -284,19 +284,16 @@ impl Tty7App { // rectangle for them, which is what keeps a pane from being // dropped into a group that is shut. // - // The active tab is the one exception: a fold says "I am done - // with this repo for now", never "hide the tab I am looking at". - // Without it ⌘T inside a folded group — `spawn_group` seeds the - // new tab with the group it came from — draws nothing but a - // header count going up by one, and with the tab bar docked left - // that row is the tab's only representation on screen. + // No exception for the active tab. A fold that leaves one row + // hanging under a shut chevron, with the header counting rows + // that are not there, reads as a list that failed to load. The + // cost is that ⌘T inside a folded group — `spawn_group` seeds + // the new tab with the group it came from — puts the new tab + // behind the chevron: the pane area shows the fresh shell and the + // header count goes up, but the row waits for the group to open. let row_count = visible_by_section[group_ix].len(); let visible: Vec = match folded { - true => visible_by_section[group_ix] - .iter() - .copied() - .filter(|&i| i == active) - .collect(), + true => Vec::new(), false => visible_by_section[group_ix].clone(), }; let visible_tabs: Vec = visible.clone(); @@ -1582,8 +1579,6 @@ mod fold_tests { for (i, root) in [(0, &alpha), (1, &alpha), (2, &beta)] { *app.tabs[i].sidebar_group.borrow_mut() = Some(root.clone()); } - // Active in the group that stays open: the folded group's own - // active row has its own test below, and it would mask this one. app.active = 2; cx.notify(); }); @@ -1640,10 +1635,8 @@ mod fold_tests { app.toggle_sidebar_group(&Some(alpha), cx); }); vcx.run_until_parked(); - // Row 1, not row 0: row 0 is the active tab and a fold never takes - // that one off the screen, so it says nothing about the fold. app.update(&mut vcx, |app, _| { - assert!(!drawn(app, 1), "folded, so the inactive row is not drawn"); + assert!(!drawn(app, 1), "folded, so the row is not drawn"); }); // Whatever the row is actually showing — the label is derived from the @@ -1664,11 +1657,13 @@ mod fold_tests { }); } - /// A fold means "I am done with this repo for now", never "hide the tab I - /// am looking at". Without this, ⌘T inside a folded group — the new tab - /// inherits the group it was spawned from — draws nothing at all. + /// A fold hides every row the group has, the active one included. The + /// alternative — leaving the active row on screen under a shut chevron, + /// with the header counting rows that are not drawn — looks like a list + /// that failed to load, which is what folding a group you are working in + /// used to produce. #[gpui::test] - fn the_active_row_stays_on_screen_inside_a_folded_group(cx: &mut TestAppContext) { + fn a_fold_hides_the_active_row_too(cx: &mut TestAppContext) { let (app, mut vcx, _streams) = harness_with_tabs(cx, 2); let alpha = PathBuf::from("/w/alpha"); @@ -1682,21 +1677,17 @@ mod fold_tests { vcx.run_until_parked(); app.update(&mut vcx, |app, _| { - assert!(drawn(app, 0), "the active row survives its group folding"); - assert!(!drawn(app, 1), "everything else in the group is gone"); + assert!(!drawn(app, 0), "the active row folds away with the rest"); + assert!(!drawn(app, 1), "and so does everything else in the group"); }); - // And it follows the active tab, rather than being decided once when - // the fold happened. app.update(&mut vcx, |app, cx| { - app.active = 1; - cx.notify(); + app.toggle_sidebar_group(&Some(alpha), cx) }); vcx.run_until_parked(); app.update(&mut vcx, |app, _| { - assert!(drawn(app, 1), "the row that is active now is the one drawn"); - assert!(!drawn(app, 0), "and the one that no longer is went away"); + assert!((0..2).all(|i| drawn(app, i)), "unfolding brings both back"); }); } }