-
Notifications
You must be signed in to change notification settings - Fork 8.6k
Back Button in Settings #18986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Back Button in Settings #18986
Conversation
This comment has been minimized.
This comment has been minimized.
320d04a
to
33add60
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this! Downloaded the branch and played around with it for a while.
I noticed that the back button only appears if we're in a subpage. I'd prefer that we have it always be visible, but we change the enabled/disabled state appropriately.
On a bigger scale though, I'd expect the back button to take you back through the history of previous pages. Being able to go back to the main pages isn't entirely the right experience. Especially since the extensions page now has ways to navigate to other profile pages. Besides, we can already go back to the parent page by clicking on the breadcrumb item.
I was trying to keep it consistent to Windows UI but just realised you are totally right, 'Settings' always has the back button visible, will set to visible and just use Enable/Disable
Thanks good point, i'll do a refactor to try and do this, need to wrap my head around of if there's a 'history' or something i can utilize here to get the right tag :)
This was discussed in original issue, the reasoning that you would still have a back button is mainly consistency with what windows ui does (the settings example in issue is great). |
When navigating to a nested page e.g. Advanced Options, the Back Button appears for the navigation list now, this follows windows UI, see Windows Settings for a example
33add60
to
f29d2f7
Compare
Going to convert back to draft as I am very lost on how to keep breadcrumbs currently, need to somehow retrieve 'tag' from the Backstack but this information is not pushed |
Going to close this PR as this approach will not work; I believe you would need to modify the data that is being pushed onto the Backstack so that you can maintain what tags are needed to calculate the breadcrumbs. Alternatively i guess you could copy the breadcrumbs to a variable and save that in the backstack but I feel like this is not very memory efficient |
However it ends up shaking out, we're excited to see! :) |
Summary of the Pull Request
This PR makes it so that if you are in a 'nested' context e.g. Advanced Settings, a back button will appear, this is consistent with Windows UI (see Settings and go to a nested page and see in the top left a back button will appear).
References and Relevant Issues
#18658
Detailed Description of the Pull Request / Additional comments
This is done by enabling the native back button in the navigation list IFF the breadcrumbs length is >1 (i.e nested context).
Validation Steps Performed
PR Checklist