fix(ui): selecting a sidebar tab scrolls only as far as the nearest edge (#1060)

* fix(ui): selecting a sidebar tab no longer scrolls the list

activate() called sidebar_scroll.scroll_to_item(tab_index), but the
sidebar list's children are group blocks and dividers, not tab rows, so
the index named some other group and a click on a lower row jumped the
list. The row now brings itself into view when it is drawn: a row with
any part on screen stays put, one out of view scrolls in to the nearest
edge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(ui): key the sidebar reveal by tab id and drop it when the row is not drawn

An index went stale when tabs were reordered or closed before the next
frame, and a reveal aimed at a row in a folded group fired whenever the
group was opened, long after the tab was selected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(changelog): drop the Unreleased entry; the release notes carry it

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(ui): a first row revealed from above brings its group header

When the sidebar scrolled a row in from above, the row landed on the top
edge with its group's heading still cut off above it, so the tab came into
view without the name of the group it is in. The first row of a group that
draws a header now counts the header (and the gap under it) as part of
itself when coming in from above. Rows on screen and rows below are
unchanged.

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>
This commit is contained in:
Adam Hitchcock
2026-10-01 14:58:11 +08:00
committed by GitHub
co-authored by Claude Opus 5.5 l0ng-ai
parent 4e4789a045
commit b63a4ebbe0
2 changed files with 83 additions and 2 deletions
+9 -1
View File
@@ -1037,6 +1037,12 @@ pub struct Tty7App {
pub(crate) right_panel_tab: RightPanelTab,
pub(crate) sidebar_collapsed: bool,
pub(crate) sidebar_scroll: gpui::ScrollHandle,
/// The tab whose sidebar row is brought into view the next time it is
/// drawn. Read by the row, not by `activate`: a new tab has no row yet,
/// and the list's children are groups, so a tab index is no child index.
/// Keyed by the tab's id, not its index, so a reorder or close in between
/// cannot point it at another tab.
pub(crate) sidebar_reveal: Rc<Cell<Option<tty7_core::core::machine::TabId>>>,
pub(crate) reorder: Rc<RefCell<Option<crate::ui::reorder::Reorder>>>,
/// The pane the pointer is over, so only that one offers its drag handle.
pub(crate) pane_hover: Rc<Cell<Option<gpui::EntityId>>>,
@@ -1705,6 +1711,7 @@ impl Tty7App {
right_panel_tab,
sidebar_collapsed,
sidebar_scroll: gpui::ScrollHandle::new(),
sidebar_reveal: Rc::new(Cell::new(None)),
reorder: Rc::new(RefCell::new(None)),
pane_hover: Rc::new(Cell::new(None)),
pane_drag: Rc::new(RefCell::new(None)),
@@ -5277,7 +5284,8 @@ impl Tty7App {
.any(|l| l.entity_id() == leaf.entity_id())
});
self.maybe_refresh_diff_overlay(cx);
self.sidebar_scroll.scroll_to_item(index);
self.sidebar_reveal
.set(Some(self.tabs[index].tree_id.get()));
if self.code_panel_visible() {
self.file_tree_refresh_roots(window, cx);
self.file_tree.focus_handle.focus(window, cx);
+74 -1
View File
@@ -516,6 +516,10 @@ impl Tty7App {
.get(self.workspace)
.is_some_and(|w| w.is_remote());
// A reveal whose row is not drawn this frame (folded away, or the tab
// gone) is dropped, so it cannot fire when the row turns up later.
let reveal = self.sidebar_reveal.get();
let mut reveal_drawn = false;
for (n, (group_slot, group_ix)) in blocks.into_iter().enumerate() {
if n == first_unpinned && show_divider {
list = list.child(self.sidebar_divider(divider_lit, divider_zone, cx));
@@ -594,6 +598,7 @@ impl Tty7App {
for (slot, i) in visible.into_iter().enumerate() {
let badge_pos = badge_pos[i];
let tab = &self.tabs[i];
reveal_drawn |= reveal == Some(tab.tree_id.get());
let is_active = i == active;
let ssh_dot = self.tab_ssh_dot(tab, cx);
let asleep = tab.is_asleep();
@@ -1039,13 +1044,37 @@ impl Tty7App {
// reads the slots of one group, a pane dropped
// on the sidebar reads every row there is.
let by_tab = self.sidebar_slots.clone();
move |bounds, _window, _cx| {
let reveal = self.sidebar_reveal.clone();
let scroll = self.sidebar_scroll.clone();
let id = tab.tree_id.get();
// The header sits directly above a group's
// first row, one gap away.
let lead = match slot == 0 && section.name.is_some() {
true => px(HEADER_HEIGHT + ROW_GAP),
false => px(0.),
};
move |bounds, window, _cx| {
if let Some(s) = slots.borrow_mut().get_mut(slot) {
*s = bounds;
}
if let Some(s) = by_tab.borrow_mut().get_mut(i) {
*s = bounds;
}
if reveal.get() == Some(id) {
reveal.set(None);
let view = scroll.bounds();
let shift = reveal_shift(
(bounds.top(), bounds.bottom()),
lead,
(view.top(), view.bottom()),
);
if shift != px(0.) {
let mut offset = scroll.offset();
offset.y += shift;
scroll.set_offset(offset);
window.refresh();
}
}
}
},
|_, _, _, _| {},
@@ -1647,6 +1676,9 @@ impl Tty7App {
_ => block.into_any_element(),
});
}
if !reveal_drawn {
self.sidebar_reveal.set(None);
}
if show_divider && !divider_drawn {
list = list.child(self.sidebar_divider(divider_lit, divider_zone, cx));
}
@@ -2965,6 +2997,23 @@ impl Section {
}
}
/// How far to move the sidebar's scroll offset so a newly active row shows.
/// A row with any part in view stays put — clicking a row must not move the
/// list under the pointer; one out of view comes in at the nearest edge.
/// `lead` is what sits on top of the row and belongs with it — its group's
/// header, for the first row — so a row coming in from above brings the
/// name of the group it is in, not just itself.
fn reveal_shift(row: (Pixels, Pixels), lead: Pixels, view: (Pixels, Pixels)) -> Pixels {
let ((top, bottom), (view_top, view_bottom)) = (row, view);
if bottom <= view_top {
view_top - (top - lead)
} else if top >= view_bottom {
view_bottom - bottom
} else {
px(0.)
}
}
/// The sidebar's sections, top to bottom: every pinned group in the user's
/// order — an empty one too, since a kept group stays until it is deleted —
/// then the auto groups in the order their first tab appears, then
@@ -4200,6 +4249,30 @@ mod fold_tests {
mod tests {
use super::*;
#[test]
fn reveal_shift_moves_only_rows_out_of_view() {
let view = (px(100.), px(400.));
// On screen, or cut by an edge: a click there scrolls nothing.
assert_eq!(reveal_shift((px(200.), px(230.)), px(0.), view), px(0.));
assert_eq!(reveal_shift((px(90.), px(120.)), px(0.), view), px(0.));
assert_eq!(reveal_shift((px(390.), px(420.)), px(0.), view), px(0.));
// Above: its top lands on the top edge.
assert_eq!(reveal_shift((px(10.), px(40.)), px(0.), view), px(90.));
// Below: its bottom lands on the bottom edge.
assert_eq!(reveal_shift((px(500.), px(530.)), px(0.), view), px(-130.));
}
#[test]
fn a_first_row_revealed_from_above_brings_its_header() {
let view = (px(100.), px(400.));
// The header's top, not the row's, lands on the top edge.
assert_eq!(reveal_shift((px(10.), px(40.)), px(23.), view), px(113.));
// A header half under the top edge, row on screen: still nothing.
assert_eq!(reveal_shift((px(110.), px(140.)), px(23.), view), px(0.));
// Below: the header is already above the row, only the bottom counts.
assert_eq!(reveal_shift((px(500.), px(530.)), px(23.), view), px(-130.));
}
fn p(s: &str) -> PathBuf {
PathBuf::from(s)
}