feat(settings): add on_page_change callback to Settings widget#2398
Draft
ralphschindler wants to merge 3 commits into
Draft
feat(settings): add on_page_change callback to Settings widget#2398ralphschindler wants to merge 3 commits into
ralphschindler wants to merge 3 commits into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Settingsstores its selected page inSettingsState, which ispub(super)and cannot be read by consumers. The only external control point isdefault_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_changecallback 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.
Screenshot
not applicable.
Break Changes
none.
How to Test
Checklist
cargo runfor story tests related to the changes.