oak-team/oak-gpui
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

settings_ui: Fix page scroll bar lagging behind when jumping to a section (#39897)

Browse Source 
The issue was caused by the scroll handle taking a couple of frames to
update its offset correctly after calling
`ScrollHandle::scroll_to_top_of_item`. The fast fix is forcing 3 frames
to render back-to-back.

In the future, we should look into `ScrollHandle` and see if there's any
way to update its state outside of paint.

Release Notes:

- N/A

Co-authored-by: Danilo Leal <daniloleal09@gmail.com>
Co-authored-by: Mikayla Maki <mikayla.c.maki@gmail.com>
Co-authored-by: Katie Geer <katie@zed.dev>
Co-authored-by: Ben Kunkle <ben@zed.dev>
This commit is contained in:
Anthony Eid
2025-10-09 19:24:02 +00:00
committed by GitHub
co-authored by Danilo Leal Mikayla Maki Katie Geer Ben Kunkle
parent 98d4c34199
commit e4f0fbbf80
1 changed files with 40 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/settings_ui/src/settings_ui.rs
+40 -21
View File
@@ -558,10 +558,10 @@ pub struct SettingsWindow {
/// Index into navbar_entries /// Index into navbar_entries
navbar_entry: usize, navbar_entry: usize,
navbar_entries: Vec<NavBarEntry>, navbar_entries: Vec<NavBarEntry>,
list_handle: UniformListScrollHandle, navbar_scroll_handle: UniformListScrollHandle,
search_matches: Vec<Vec<bool>>, search_matches: Vec<Vec<bool>>,
content_handles: Vec<Vec<Entity<NonFocusableHandle>>>, content_handles: Vec<Vec<Entity<NonFocusableHandle>>>,
scroll_handle: ScrollHandle, page_scroll_handle: ScrollHandle,
focus_handle: FocusHandle, focus_handle: FocusHandle,
navbar_focus_handle: Entity<NonFocusableHandle>, navbar_focus_handle: Entity<NonFocusableHandle>,
content_focus_handle: Entity<NonFocusableHandle>, content_focus_handle: Entity<NonFocusableHandle>,
@@ -923,12 +923,12 @@ impl SettingsWindow {
pages: vec![], pages: vec![],
navbar_entries: vec![], navbar_entries: vec![],
navbar_entry: 0, navbar_entry: 0,
list_handle: UniformListScrollHandle::default(), navbar_scroll_handle: UniformListScrollHandle::default(),
search_bar, search_bar,
search_task: None, search_task: None,
search_matches: vec![], search_matches: vec![],
content_handles: vec![], content_handles: vec![],
scroll_handle: ScrollHandle::new(), page_scroll_handle: ScrollHandle::new(),
focus_handle: cx.focus_handle(), focus_handle: cx.focus_handle(),
navbar_focus_handle: NonFocusableHandle::new( navbar_focus_handle: NonFocusableHandle::new(
NAVBAR_CONTAINER_TAB_INDEX, NAVBAR_CONTAINER_TAB_INDEX,
@@ -1471,7 +1471,7 @@ impl SettingsWindow {
let Some(previous_root_index) = root_index else { let Some(previous_root_index) = root_index else {
return; return;
}; };
this.focus_and_scroll_to_nav_entry(previous_root_index, window); this.focus_and_scroll_to_nav_entry(previous_root_index, window, cx);
}), }),
) )
.on_action(cx.listener(|this, _: &FocusNextRootNavEntry, window, cx| { .on_action(cx.listener(|this, _: &FocusNextRootNavEntry, window, cx| {
@@ -1491,16 +1491,16 @@ impl SettingsWindow {
let Some(next_root_index) = root_index else { let Some(next_root_index) = root_index else {
return; return;
}; };
this.focus_and_scroll_to_nav_entry(next_root_index, window); this.focus_and_scroll_to_nav_entry(next_root_index, window, cx);
})) }))
.on_action(cx.listener(|this, _: &FocusFirstNavEntry, window, _| { .on_action(cx.listener(|this, _: &FocusFirstNavEntry, window, cx| {
if let Some((first_entry_index, _)) = this.visible_navbar_entries().next() { if let Some((first_entry_index, _)) = this.visible_navbar_entries().next() {
this.focus_and_scroll_to_nav_entry(first_entry_index, window); this.focus_and_scroll_to_nav_entry(first_entry_index, window, cx);
} }
})) }))
.on_action(cx.listener(|this, _: &FocusLastNavEntry, window, _| { .on_action(cx.listener(|this, _: &FocusLastNavEntry, window, cx| {
if let Some((last_entry_index, _)) = this.visible_navbar_entries().last() { if let Some((last_entry_index, _)) = this.visible_navbar_entries().last() {
this.focus_and_scroll_to_nav_entry(last_entry_index, window); this.focus_and_scroll_to_nav_entry(last_entry_index, window, cx);
} }
})) }))
.border_color(cx.theme().colors().border) .border_color(cx.theme().colors().border)
@@ -1551,9 +1551,9 @@ impl SettingsWindow {
}), }),
) )
.size_full() .size_full()
.track_scroll(self.list_handle.clone()), .track_scroll(self.navbar_scroll_handle.clone()),
) )
.vertical_scrollbar_for(self.list_handle.clone(), window, cx), .vertical_scrollbar_for(self.navbar_scroll_handle.clone(), window, cx),
) )
.child( .child(
h_flex() h_flex()
@@ -1592,7 +1592,7 @@ impl SettingsWindow {
return; return;
}; };
self.focus_content_element(first_item_index, window, cx); self.focus_content_element(first_item_index, window, cx);
self.scroll_handle.set_offset(point(px(0.), px(0.))); self.page_scroll_handle.set_offset(point(px(0.), px(0.)));
} else { } else {
let entry_item_index = self.navbar_entries[navbar_entry_index] let entry_item_index = self.navbar_entries[navbar_entry_index]
.item_index .item_index
@@ -1603,10 +1603,23 @@ impl SettingsWindow {
else { else {
return; return;
}; };
self.scroll_handle self.page_scroll_handle
.scroll_to_top_of_item(selected_item_index); .scroll_to_top_of_item(selected_item_index);
self.focus_content_element(entry_item_index, window, cx); self.focus_content_element(entry_item_index, window, cx);
} }
// Page scroll handle updates the active item index
// in it's next paint call after using scroll_handle.scroll_to_top_of_item
// The call after that updates the offset of the scroll handle. So to
// ensure the scroll handle doesn't lag behind we need to render three frames
// back to back.
cx.on_next_frame(window, |_, window, cx| {
cx.on_next_frame(window, |_, _, cx| {
cx.notify();
});
cx.notify();
});
cx.notify();
} }
fn is_nav_entry_visible(&self, nav_entry_index: usize) -> bool { fn is_nav_entry_visible(&self, nav_entry_index: usize) -> bool {
@@ -1614,16 +1627,22 @@ impl SettingsWindow {
.any(|(index, _)| index == nav_entry_index) .any(|(index, _)| index == nav_entry_index)
} }
fn focus_and_scroll_to_nav_entry(&self, nav_entry_index: usize, window: &mut Window) { fn focus_and_scroll_to_nav_entry(
&self,
nav_entry_index: usize,
window: &mut Window,
cx: &mut Context<Self>,
) {
let Some(position) = self let Some(position) = self
.visible_navbar_entries() .visible_navbar_entries()
.position(|(index, _)| index == nav_entry_index) .position(|(index, _)| index == nav_entry_index)
else { else {
return; return;
}; };
self.list_handle self.navbar_scroll_handle
.scroll_to_item(position, gpui::ScrollStrategy::Top); .scroll_to_item(position, gpui::ScrollStrategy::Top);
window.focus(&self.navbar_entries[nav_entry_index].focus_handle); window.focus(&self.navbar_entries[nav_entry_index].focus_handle);
cx.notify();
} }
fn visible_page_items(&self) -> impl Iterator<Item = (usize, &SettingsPageItem)> { fn visible_page_items(&self) -> impl Iterator<Item = (usize, &SettingsPageItem)> {
@@ -1670,7 +1689,7 @@ impl SettingsWindow {
.id("settings-ui-page") .id("settings-ui-page")
.size_full() .size_full()
.overflow_y_scroll() .overflow_y_scroll()
.track_scroll(&self.scroll_handle); .track_scroll(&self.page_scroll_handle);
let items: Vec<_> = items.collect(); let items: Vec<_> = items.collect();
let items_len = items.len(); let items_len = items.len();
@@ -1785,7 +1804,7 @@ impl SettingsWindow {
.px_8() .px_8()
.bg(cx.theme().colors().editor_background) .bg(cx.theme().colors().editor_background)
.child(page_header) .child(page_header)
.vertical_scrollbar_for(self.scroll_handle.clone(), window, cx) .vertical_scrollbar_for(self.page_scroll_handle.clone(), window, cx)
.track_focus(&self.content_focus_handle.focus_handle(cx)) .track_focus(&self.content_focus_handle.focus_handle(cx))
.child( .child(
div() div()
@@ -2026,7 +2045,7 @@ impl Render for SettingsWindow {
{ {
this.open_and_scroll_to_navbar_entry(this.navbar_entry, window, cx); this.open_and_scroll_to_navbar_entry(this.navbar_entry, window, cx);
} else { } else {
this.focus_and_scroll_to_nav_entry(this.navbar_entry, window); this.focus_and_scroll_to_nav_entry(this.navbar_entry, window, cx);
} }
})) }))
.on_action( .on_action(
@@ -2474,11 +2493,11 @@ mod test {
search_bar: cx.new(|cx| Editor::single_line(window, cx)), search_bar: cx.new(|cx| Editor::single_line(window, cx)),
navbar_entry: selected_idx.expect("Must have a selected navbar entry"), navbar_entry: selected_idx.expect("Must have a selected navbar entry"),
navbar_entries: Vec::default(), navbar_entries: Vec::default(),
list_handle: UniformListScrollHandle::default(), navbar_scroll_handle: UniformListScrollHandle::default(),
search_matches: vec![], search_matches: vec![],
content_handles: vec![], content_handles: vec![],
search_task: None, search_task: None,
scroll_handle: ScrollHandle::new(), page_scroll_handle: ScrollHandle::new(),
focus_handle: cx.focus_handle(), focus_handle: cx.focus_handle(),
navbar_focus_handle: NonFocusableHandle::new( navbar_focus_handle: NonFocusableHandle::new(
NAVBAR_CONTAINER_TAB_INDEX, NAVBAR_CONTAINER_TAB_INDEX,