Skip to content

feat(settings): add on_page_change callback to Settings widget#2398

Draft
ralphschindler wants to merge 3 commits into
longbridge:mainfrom
ralphschindler:feat/settings-on-page-change
Draft

feat(settings): add on_page_change callback to Settings widget#2398
ralphschindler wants to merge 3 commits into
longbridge:mainfrom
ralphschindler:feat/settings-on-page-change

Conversation

@ralphschindler
Copy link
Copy Markdown

@ralphschindler ralphschindler commented May 25, 2026

Description

Settings stores its selected page in SettingsState, which is pub(super) and cannot be read by consumers. The only external control point is default_selected_index, which only applies on first initialization.

This makes it impossible to implement patterns like restoring the last-visited settings page when a user navigates away and returns — a common need for any app that lazily shows/hides the settings view.

This PR adds an optional on_page_change callback that fires whenever the user selects a top-level page in the sidebar:

Note: I opted to test this in the story since it did not appear unit style tests were favored in the repo, specifically this settings/ directory. I can add a test (back) in settings/ if that is preferable.

Settings::new("my-settings")
    .default_selected_index(SelectIndex { page_ix: last_page, group_ix: None })
    .on_page_change(move |page_ix, cx| {
        // store page_ix externally; pass it back via default_selected_index on next render
    })

Screenshot

not applicable.

Break Changes

none.

How to Test

  1. Create a Settings widget with .on_page_change(|ix, _cx| println!("page: {ix}"))
  2. Click between pages in the sidebar — callback should fire with the correct zero-based index
  3. Confirm existing Settings usage without .on_page_change compiles and behaves identically

Checklist

  • I have read the CONTRIBUTING document and followed the guidelines.
  • Reviewed the changes in this PR and confirmed AI generated code (If any) is accurate.
  • Passed cargo run for story tests related to the changes.
  • Tested macOS, Windows and Linux platforms performance (if the change is platform-specific)

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<dyn Fn(usize, &mut App)> 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.
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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant