Skip to content

Bugfix: navigate to new profile ref on settings reload#8835

Merged
1 commit merged intomainfrom
dev/cazamor/sui/bugfix-save-profile-settings
Jan 21, 2021
Merged

Bugfix: navigate to new profile ref on settings reload#8835
1 commit merged intomainfrom
dev/cazamor/sui/bugfix-save-profile-settings

Conversation

@carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Jan 20, 2021

Upon a settings reload, we would select the correct navigation item for
a profile, but navigate to the old one. As a result, you would navigate
to the old page that points to a dead profile object. This would make it
appear like you did not discard/save the changes.

This bugfix navigates to the newly created profile, ensuring that your
changes are actually applied to the settings model's clone in use.

References

#8773 - Introduced the bug
#6800 - Settings UI Epic

Validation Steps Performed

  • Navigate to "powershell" profile
  • edit "tab title" value
  • discard changes

Before: changes would persist unless you discarded changes again
Now: changes are discarded

Also verified expected behavior occurs when you click "save" instead of
"discard"

@carlos-zamora carlos-zamora added Product-Terminal The new Windows Terminal. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Area-SettingsUI Anything specific to the SUI labels Jan 20, 2021
@carlos-zamora carlos-zamora added this to the Terminal v1.6 milestone Jan 20, 2021
@carlos-zamora carlos-zamora requested a review from DHowett January 20, 2021 23:44
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

Ugh.

@carlos-zamora
Copy link
Member Author

@msftbot merge this in 5 minutes

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

ghost commented Jan 21, 2021

Hello @carlos-zamora!

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 won't merge this pull request until after the UTC date Thu, 21 Jan 2021 00:54:22 GMT, which is in 5 minutes

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".

@ghost ghost merged commit 3b53c69 into main Jan 21, 2021
@ghost ghost deleted the dev/cazamor/sui/bugfix-save-profile-settings branch January 21, 2021 00:55
mpela81 pushed a commit to mpela81/terminal that referenced this pull request Jan 28, 2021
Upon a settings reload, we would select the correct navigation item for
a profile, but navigate to the old one. As a result, you would navigate
to the old page that points to a dead profile object. This would make it
appear like you did not discard/save the changes.

This bugfix navigates to the newly created profile, ensuring that your
changes are actually applied to the settings model's clone in use.

## References
microsoft#8773 - Introduced the bug
microsoft#6800 - Settings UI Epic

## Validation Steps Performed
- Navigate to "powershell" profile
- edit "tab title" value
- discard changes

Before: changes would persist unless you discarded changes again
Now: changes are discarded

Also verified expected behavior occurs when you click "save" instead of
"discard"
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-SettingsUI Anything specific to the SUI AutoMerge Marked for automatic merge by the bot when requirements are met Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Terminal The new Windows Terminal.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants