From e4d65e46003265a0778f1c9017b2128c2870879b Mon Sep 17 00:00:00 2001 From: Ralph Schindler Date: Mon, 25 May 2026 08:35:45 -0500 Subject: [PATCH 1/3] feat(settings): add on_page_change callback to Settings widget Consumers that need to restore the last-visited page (e.g. persist it for the session) had no way to observe when the user navigated between pages, because SettingsState is pub(super). Add an optional Arc callback invoked whenever the user clicks a top-level page in the sidebar. The page index can then be stored externally and passed back via default_selected_index on the next render to restore the position. --- crates/ui/src/setting/settings.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/crates/ui/src/setting/settings.rs b/crates/ui/src/setting/settings.rs index 6ec2496bfc..495b3ee25e 100644 --- a/crates/ui/src/setting/settings.rs +++ b/crates/ui/src/setting/settings.rs @@ -11,6 +11,7 @@ use gpui::{ RenderOnce, StyleRefinement, Styled, Window, div, prelude::FluentBuilder as _, px, relative, }; use rust_i18n::t; +use std::sync::Arc; /// The settings structure containing multiple pages for app settings. /// @@ -34,6 +35,7 @@ pub struct Settings { sidebar_style: StyleRefinement, default_selected_index: SelectIndex, header_style: StyleRefinement, + on_page_change: Option>, } impl Settings { @@ -48,6 +50,7 @@ impl Settings { sidebar_style: StyleRefinement::default(), default_selected_index: SelectIndex::default(), header_style: StyleRefinement::default(), + on_page_change: None, } } @@ -95,6 +98,15 @@ impl Settings { self } + /// Set a callback invoked when the user selects a top-level page in the sidebar. + /// + /// The first argument is the zero-based page index. This is useful for persisting + /// the last-visited page so it can be restored via [`Self::default_selected_index`]. + pub fn on_page_change(mut self, f: impl Fn(usize, &mut App) + 'static) -> Self { + self.on_page_change = Some(Arc::new(f)); + self + } + fn filtered_pages(&self, query: &str, cx: &App) -> Vec { self.pages .iter() @@ -153,6 +165,7 @@ impl Settings { &self, state: &Entity, pages: &Vec, + on_page_change: Option>, _: &mut Window, cx: &mut App, ) -> impl IntoElement { @@ -182,6 +195,7 @@ impl Settings { .active(is_page_active) .on_click({ let state = state.clone(); + let on_page_change = on_page_change.clone(); move |_, _, cx| { state.update(cx, |state, cx| { state.selected_index = SelectIndex { @@ -189,7 +203,10 @@ impl Settings { ..Default::default() }; cx.notify(); - }) + }); + if let Some(f) = &on_page_change { + f(page_ix, cx); + } } }) .when(page.groups.len() > 1, |this| { @@ -288,7 +305,7 @@ impl RenderOnce for Settings { .child( resizable_panel() .size(self.sidebar_width) - .child(self.render_sidebar(&state, &filtered_pages, window, cx)), + .child(self.render_sidebar(&state, &filtered_pages, self.on_page_change.clone(), window, cx)), ) .child(resizable_panel().child(self.render_active_page( &state, From 017704751e38f1ae61e9a51f3089f4b52d5ed813 Mon Sep 17 00:00:00 2001 From: Ralph Schindler Date: Mon, 25 May 2026 09:47:47 -0500 Subject: [PATCH 2/3] test(settings): add builder test and use Rc for on_page_change callback Replace Arc with Rc to match the callback convention used elsewhere in the codebase (Button, Notification, setting fields). Add builder test covering the full Settings API including on_page_change. --- crates/ui/src/setting/settings.rs | 33 +++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/crates/ui/src/setting/settings.rs b/crates/ui/src/setting/settings.rs index 495b3ee25e..1133f1057a 100644 --- a/crates/ui/src/setting/settings.rs +++ b/crates/ui/src/setting/settings.rs @@ -11,7 +11,7 @@ use gpui::{ RenderOnce, StyleRefinement, Styled, Window, div, prelude::FluentBuilder as _, px, relative, }; use rust_i18n::t; -use std::sync::Arc; +use std::rc::Rc; /// The settings structure containing multiple pages for app settings. /// @@ -35,7 +35,7 @@ pub struct Settings { sidebar_style: StyleRefinement, default_selected_index: SelectIndex, header_style: StyleRefinement, - on_page_change: Option>, + on_page_change: Option>, } impl Settings { @@ -103,7 +103,7 @@ impl Settings { /// The first argument is the zero-based page index. This is useful for persisting /// the last-visited page so it can be restored via [`Self::default_selected_index`]. pub fn on_page_change(mut self, f: impl Fn(usize, &mut App) + 'static) -> Self { - self.on_page_change = Some(Arc::new(f)); + self.on_page_change = Some(Rc::new(f)); self } @@ -165,7 +165,7 @@ impl Settings { &self, state: &Entity, pages: &Vec, - on_page_change: Option>, + on_page_change: Option>, _: &mut Window, cx: &mut App, ) -> impl IntoElement { @@ -273,6 +273,31 @@ pub struct SelectIndex { pub group_ix: Option, } +#[cfg(test)] +mod tests { + use super::*; + use gpui::px; + + #[test] + fn test_settings_builder() { + let settings = Settings::new("test-settings") + .sidebar_width(px(200.0)) + .default_selected_index(SelectIndex { + page_ix: 1, + group_ix: None, + }) + .on_page_change(|_, _| {}) + .page(SettingPage::new("General")) + .page(SettingPage::new("Advanced")); + + assert_eq!(settings.sidebar_width, px(200.0)); + assert_eq!(settings.default_selected_index.page_ix, 1); + assert!(settings.default_selected_index.group_ix.is_none()); + assert!(settings.on_page_change.is_some()); + assert_eq!(settings.pages.len(), 2); + } +} + impl RenderOnce for Settings { fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { let state = window.use_keyed_state(self.id.clone(), cx, |window, cx| { From 74b4e2198301d3c75b3205d82440a82dc54104ec Mon Sep 17 00:00:00 2001 From: Ralph Schindler Date: Mon, 25 May 2026 10:05:43 -0500 Subject: [PATCH 3/3] story(settings): remember last-visited page across gallery navigation Store the last selected page index in SettingsStory and restore it via default_selected_index on each render, using the new on_page_change callback to keep it in sync. --- crates/story/src/stories/settings_story.rs | 13 +++++++++++ crates/ui/src/setting/settings.rs | 25 ---------------------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/crates/story/src/stories/settings_story.rs b/crates/story/src/stories/settings_story.rs index 252605ee75..9c8ba4f08d 100644 --- a/crates/story/src/stories/settings_story.rs +++ b/crates/story/src/stories/settings_story.rs @@ -61,6 +61,7 @@ pub struct SettingsStory { focus_handle: FocusHandle, group_variant: GroupBoxVariant, size: Size, + last_page_index: usize, } struct OpenURLSettingField { @@ -121,6 +122,7 @@ impl SettingsStory { focus_handle: cx.focus_handle(), group_variant: GroupBoxVariant::Outline, size: Size::default(), + last_page_index: 0, } } @@ -455,9 +457,20 @@ impl Focusable for SettingsStory { impl Render for SettingsStory { fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + let entity = cx.entity(); Settings::new("app-settings") .with_size(self.size) .with_group_variant(self.group_variant) + .default_selected_index(gpui_component::setting::SelectIndex { + page_ix: self.last_page_index, + group_ix: None, + }) + .on_page_change(move |ix, cx| { + entity.update(cx, |this, cx| { + this.last_page_index = ix; + cx.notify(); + }); + }) .pages(self.setting_pages(window, cx)) } } diff --git a/crates/ui/src/setting/settings.rs b/crates/ui/src/setting/settings.rs index 1133f1057a..86eb4b9c05 100644 --- a/crates/ui/src/setting/settings.rs +++ b/crates/ui/src/setting/settings.rs @@ -273,31 +273,6 @@ pub struct SelectIndex { pub group_ix: Option, } -#[cfg(test)] -mod tests { - use super::*; - use gpui::px; - - #[test] - fn test_settings_builder() { - let settings = Settings::new("test-settings") - .sidebar_width(px(200.0)) - .default_selected_index(SelectIndex { - page_ix: 1, - group_ix: None, - }) - .on_page_change(|_, _| {}) - .page(SettingPage::new("General")) - .page(SettingPage::new("Advanced")); - - assert_eq!(settings.sidebar_width, px(200.0)); - assert_eq!(settings.default_selected_index.page_ix, 1); - assert!(settings.default_selected_index.group_ix.is_none()); - assert!(settings.on_page_change.is_some()); - assert_eq!(settings.pages.len(), 2); - } -} - impl RenderOnce for Settings { fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { let state = window.use_keyed_state(self.id.clone(), cx, |window, cx| {