Skip to content
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

Keep the last selected pivot active when navigating over profile pages #10047

Merged

Conversation

kovdu
Copy link
Contributor

@kovdu kovdu commented May 6, 2021

PR Checklist

Validation Steps Performed

  • When opening a profile page and selecting a random pivot, when then navigating away from the profile page to another item and navigating back to the same profile the last selected pivot was still active.
  • When opening a profile page and selecting a random pivot, when then navigating away from the profile page to a second profile the last selected pivot was immediately active on the second profile.
  • When selecting random pivots and navigating only between other profile pages the last selected pivot was always active on the opened profile page.

What felt a bit strange at first sight was:

  • open a profile page, select a random pivot
  • start adding a new profile
  • the last selected pivot is immediately active and not the "General" pivot what might be the obvious pivot to show when a user wants to add a new profile.

After playing a bit more with this this started to feel ok. Since as a user you might go to "Add new profile" immediately anyway instead of opening other profiles first.

Just explicitly highlighting this last point since maybe someone prefers that we need to make sure to always start with the 'General' pivot when adding a new profile and then this fix is not correct.

@ghost ghost added Area-Settings UI Anything specific to the SUI Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels May 6, 2021
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's basically what I had before. Thanks for the solid writeup!

@zadjii-msft
Copy link
Member

@msftbot make sure @carlos-zamora signs off on this

@ghost ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label May 6, 2021
@ghost
Copy link

ghost commented May 6, 2021

Hello @zadjii-msft!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I'll only merge this pull request if it's approved by @carlos-zamora

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@ghost ghost merged commit 0e8d5f2 into microsoft:main May 7, 2021
@ghost
Copy link

ghost commented May 25, 2021

🎉Windows Terminal Preview v1.9.1445.0 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings UI Anything specific to the SUI AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SettingsUI: keep "page" when changing profile
3 participants