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.

Sign up for GitHub

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

Jump to bottom

Settings UI: Current nav item is reset when discarding/saving on a profile nav item #10617

Closed
mimvdb opened this issue Jul 11, 2021 · 2 comments · Fixed by #10618
Closed

Settings UI: Current nav item is reset when discarding/saving on a profile nav item #10617

mimvdb opened this issue Jul 11, 2021 · 2 comments · Fixed by #10618
Labels
Area-Settings UI Anything specific to the SUI Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@mimvdb
Copy link
Contributor

mimvdb commented Jul 11, 2021

Windows Terminal version (or Windows build number)

Main branch 2021-07-11

Other Software

No response

Steps to reproduce

Go to settings, click on a profile (e.g. Windows PowerShell), click "Discard changes"

Expected Behavior

Nothing

Actual Behavior

Navigates to the top most nav item (Startup)
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jul 11, 2021
@mimvdb mimvdb mentioned this issue Jul 11, 2021
Merged
6 tasks
@ghost ghost added the In-PR This issue has a related PR label Jul 11, 2021
@skyline75489 skyline75489 added Area-Settings UI Anything specific to the SUI Product-Terminal The new Windows Terminal. labels Jul 11, 2021
@carlos-zamora carlos-zamora added Issue-Bug It either shouldn't be doing this or needs an investigation. and removed Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jul 12, 2021
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Jul 12, 2021
lhecker pushed a commit that referenced this issue Jul 12, 2021
@mimvdb
Include profile nav menu items to consider for retaining position (#1…
ef8ba20 
…0618)

## Summary of the Pull Request

When discarding or saving settings, the current navigation should be retained.

## References

Issue introduced by #10390

## PR Checklist

* [x] Closes #10617 
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

## Detailed Description of the Pull Request / Additional comments

`menuItemsSTL` is filled with all _non_ profile navItems, then `menuItemsSTL` fills `menuItems`, then the profile navItems are added to `menuItems`. So to include the profile nav items in the iteration, `menuItems` needs to be used

## Validation Steps Performed

Spam discard and save buttons
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jul 12, 2021
DHowett pushed a commit that referenced this issue Jul 12, 2021
@mimvdb @DHowett
Include profile nav menu items to consider for retaining position (#1…
dc77f8c 
…0618)

## Summary of the Pull Request

When discarding or saving settings, the current navigation should be retained.

## References

Issue introduced by #10390

## PR Checklist

* [x] Closes #10617
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

## Detailed Description of the Pull Request / Additional comments

`menuItemsSTL` is filled with all _non_ profile navItems, then `menuItemsSTL` fills `menuItems`, then the profile navItems are added to `menuItems`. So to include the profile nav items in the iteration, `menuItems` needs to be used

## Validation Steps Performed

Spam discard and save buttons

(cherry picked from commit ef8ba20)
@ghost
Copy link

ghost commented Jul 14, 2021

🎉This issue was addressed in #10618, which has now been successfully released as Windows Terminal v1.9.1942.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Jul 14, 2021

🎉This issue was addressed in #10618, which has now been successfully released as Windows Terminal Preview v1.10.1933.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Settings UI Anything specific to the SUI Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Include profile nav menu items to consider for retaining position mimvdb/terminal
3 participants
@mimvdb @skyline75489 @carlos-zamora