Polish OpenSettings action for Settings UI and Profile page navigation on refresh #8670
Conversation
This file contains 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
This reverts commit 1174aab.
|
Ugh! 1174aab still caused the crash. Surprisingly, "Discard Changes" works fine, but "Apply" causes the out-of-bounds crash. :/ |
zadjii-msft
approved these changes
Jan 4, 2021
DHowett
reviewed
Jan 4, 2021
| @@ -363,4 +363,7 @@ | |||
| <data name="BreakIntoDebuggerCommandKey" xml:space="preserve"> | |||
| <value>Break into the debugger</value> | |||
| </data> | |||
| </root> | |||
| <data name="OpenSettingsUICommandKey" xml:space="preserve"> | |||
| <value>Open Settings UI</value> | |||
There was a problem hiding this comment.
nit: maybe just.. Open Settings
UI is a term we use, not a term our community uses.
DHowett
reviewed
Jan 4, 2021
src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw
Outdated
Show resolved
Hide resolved
DHowett
approved these changes
Jan 4, 2021
DHowett
added
the
AutoMerge
|
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
mpela81
pushed a commit
to mpela81/terminal
that referenced
this pull request
Jan 28, 2021
…n on refresh (microsoft#8670) Performs a number of minor bugfixes related to the Settings UI: - b5370a1 Dropdown bug: - the dropdown would display the keybinding for the first `openSettings` found. So it would accidentally present and bind the one for the Settings UI. - 91eb49e autogenerated name for opening Settings UI: - the Settings UI keybinding would display "open settings file". This was updated to say "Open Settings UI". - 1cadbf4 Profile Page navigation crash: - the selected item off of a MUX navigation view returns a MUX NavViewItem (as opposed to WUX) - dd2f3e5 Hookup delete for Profile page navigation: - missed a spot where we were manually navigating to the Profile page. So it wasn't hooked up properly - 9fea6de Properly cast NavViewItem tags - When we update the NavigationView's menu items, we were casting the tags to `Model::Profile` instead of `Editor::ProfileViewModel`. ## References microsoft#6800 - Settings UI epic Fixes the following bug: > - [ ] JSON change --> crash > - open SUI --> open JSON --> edit retro effects in JSON --> save file --> cry because the app crashed ## Additional comments This was a part of some manual testing I performed on the Settings UI. More intricate bugs are being reported on microsoft#6800 and will be fixed in their own PR.
Closed
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.
Summary of the Pull Request
Performs a number of minor bugfixes related to the Settings UI:
openSettingsfound. So it would accidentally present and bind the one for the Settings UI.Model::Profileinstead ofEditor::ProfileViewModel.References
#6800 - Settings UI epic
Fixes the following bug:
Additional comments
This was a part of some manual testing I performed on the Settings UI. More intricate bugs are being reported on #6800 and will be fixed in their own PR.