Pane tabs: Scroll entire new tab into view (#36827)

The state of the child bounds is not up-to-date when `scroll_to_item`
gets triggered, causing the new tab to not scroll completely into view.

Closes #36317 

Release Notes:

- Fix an issue where a new tab is only partially visible on creation.
This commit is contained in:
hrou0003
2025-09-30 11:04:34 +02:00
committed by GitHub
parent 624e448492
commit 49335d54be
3 changed files with 94 additions and 34 deletions
+57 -7
View File
@@ -367,6 +367,9 @@ pub struct Pane {
max_tabs: Option<NonZeroUsize>,
_subscriptions: Vec<Subscription>,
tab_bar_scroll_handle: ScrollHandle,
/// This is set to true if a user scroll has occurred more recently than a system scroll
/// We want to suppress certain system scrolls when the user has intentionally scrolled
suppress_scroll: bool,
/// Is None if navigation buttons are permanently turned off (and should not react to setting changes).
/// Otherwise, when `display_nav_history_buttons` is Some, it determines whether nav buttons should be displayed.
display_nav_history_buttons: Option<bool>,
@@ -497,6 +500,7 @@ impl Pane {
}))),
toolbar: cx.new(|_| Toolbar::new()),
tab_bar_scroll_handle: ScrollHandle::new(),
suppress_scroll: false,
drag_split_direction: None,
workspace,
project: project.downgrade(),
@@ -573,6 +577,9 @@ impl Pane {
if !self.was_focused {
self.was_focused = true;
self.update_history(self.active_item_index);
if !self.suppress_scroll && self.items.get(self.active_item_index).is_some() {
self.update_active_tab(self.active_item_index);
}
cx.emit(Event::Focus);
cx.notify();
}
@@ -618,6 +625,7 @@ impl Pane {
self.toolbar.update(cx, |toolbar, cx| {
toolbar.focus_changed(false, window, cx);
});
cx.notify();
}
@@ -1124,6 +1132,7 @@ impl Pane {
}
} else {
self.items.insert(insertion_index, item.clone());
cx.notify();
if activate {
if insertion_index <= self.active_item_index
@@ -1134,7 +1143,6 @@ impl Pane {
self.activate_item(insertion_index, activate_pane, focus_item, window, cx);
}
cx.notify();
}
cx.emit(Event::AddItem { item });
@@ -1272,15 +1280,18 @@ impl Pane {
focus_changed: focus_item,
});
if !self.is_tab_pinned(index) {
self.tab_bar_scroll_handle
.scroll_to_item(index - self.pinned_tab_count);
}
self.update_active_tab(index);
cx.notify();
}
}
fn update_active_tab(&mut self, index: usize) {
if !self.is_tab_pinned(index) {
self.suppress_scroll = false;
self.tab_bar_scroll_handle.scroll_to_item(index);
}
}
fn update_history(&mut self, index: usize) {
if let Some(newly_active_item) = self.items.get(index) {
self.activation_history
@@ -3028,6 +3039,9 @@ impl Pane {
.overflow_x_scroll()
.w_full()
.track_scroll(&self.tab_bar_scroll_handle)
.on_scroll_wheel(cx.listener(|this, _, _, _| {
this.suppress_scroll = true;
}))
.children(unpinned_tabs)
.child(
div()
@@ -4095,7 +4109,7 @@ mod tests {
use super::*;
use crate::item::test::{TestItem, TestProjectItem};
use gpui::{TestAppContext, VisualTestContext};
use gpui::{TestAppContext, VisualTestContext, size};
use project::FakeFs;
use settings::SettingsStore;
use theme::LoadThemes;
@@ -6310,6 +6324,42 @@ mod tests {
});
}
#[gpui::test]
async fn test_new_tab_scrolls_into_view_completely(cx: &mut TestAppContext) {
// Arrange
init_test(cx);
let fs = FakeFs::new(cx.executor());
let project = Project::test(fs, None, cx).await;
let (workspace, cx) =
cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx));
let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone());
cx.simulate_resize(size(px(300.), px(300.)));
add_labeled_item(&pane, "untitled", false, cx);
add_labeled_item(&pane, "untitled", false, cx);
add_labeled_item(&pane, "untitled", false, cx);
add_labeled_item(&pane, "untitled", false, cx);
// Act: this should trigger a scroll
add_labeled_item(&pane, "untitled", false, cx);
// Assert
let tab_bar_scroll_handle =
pane.update_in(cx, |pane, _window, _cx| pane.tab_bar_scroll_handle.clone());
assert_eq!(tab_bar_scroll_handle.children_count(), 6);
let tab_bounds = cx.debug_bounds("TAB-3").unwrap();
let new_tab_button_bounds = cx.debug_bounds("ICON-Plus").unwrap();
let scroll_bounds = tab_bar_scroll_handle.bounds();
let scroll_offset = tab_bar_scroll_handle.offset();
assert!(tab_bounds.right() <= scroll_bounds.right() + scroll_offset.x);
// -39.75 is the magic number for this setup
assert_eq!(scroll_offset.x, px(-39.75));
assert!(
!tab_bounds.intersects(&new_tab_button_bounds),
"Tab should not overlap with the new tab button, if this is failing check if there's been a redesign!"
);
}
#[gpui::test]
async fn test_close_all_items_including_pinned(cx: &mut TestAppContext) {
init_test(cx);