Re-enables the delete button for generated profiles in the settings UI.
Additionally fixes "Startup Profiles" to only list active profiles.
Profiles are considered deleted if they're absent from settings.json, but their
GUID has been encountered before. Or in other words, from a user's perspective:
Generated profiles are added to the settings.json automatically only once.
Thus if the user chooses to delete the profile (e.g. using the delete button)
they aren't re-added automatically and thus appear to have been deleted.
Meanwhile those generated profiles are actually only marked as "hidden"
as well as "deleted", but still exist in internal profile lists.
The "hidden" attribute hides them from all existing menus. The "deleted" one
hides them from the settings UI and prevents them from being written to disk.
It would've been preferrable of course to just not generate and
add deleted profile to internal profile lists in the first place.
But this would've required far more wide-reaching changes.
The settings UI for instance requires a list of _all_ profiles in order to
allow a user to re-create previously deleted profiles. Such an approach was
attempted but discarded because of it's current complexity overhead.
## References
* Part of #9997
* A sequel to 5d36e5d
## PR Checklist
* [x] Closes#10960
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* "Startup Profiles" doesn't list deleted profiles ✔️
* Manually removing an item from settings.json removes the profile ✔️
* Removing cmd.exe and saving doesn't create empty objects (#10960) ✔️
* "Add a new profile" lists deleted profiles ✔️
* "Duplicate" recreates previously deleted profiles ✔️
* Profiles are always created with GUIDs ✔️
## 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
This pull request brings back the "Base Layer" page, now renamed to
"Defaults", and the "Reset to inherited value" buttons. The scope of
inheritance for which buttons will display has been widened.
The button will be visible in the following cases:
The user has set a setting for the current profile, and it overrides...
1. ... something in profiles.defaults.
2. ... something in a Fragment Extension profile.
3. ... something from a Dynamic Profile Generator.
4. ... something from the compiled-in defaults.
Compared to the original implementation of reset arrows, cases (1), (3)
and (4) are new. Rationale:
(1) The user can see a setting on the Defaults page, and they need a way
to reset back to it.
(3) Dynamic profiles are not meaningfully different from fragments, and
users may need a way to reset back to the default value generated
for WSL or PowerShell.
(4) The user can see a setting on the Defaults page, **BUT** they are
not the one who created it. They *still* need a way to get back to
it.
To support this, I've introduced another origin tag, "User", and renamed
"Custom" to "None". Due to the way origin/override detection works¹, we
cannot otherwise disambiguate between settings that came from the user
and settings that came from the compiled-in defaults.
Changes were required in TerminalSettings such that we could construct a
settings object with a profile that does not have a GUID. In making this
change, I fixed a bit of silliness where we took a profile, extracted
its guid, and used that guid to look up the same profile object. Oops.
I also fixed the PropertyChanged notifier to include the
XxxOverrideSource property.
The presence of the page and the reset arrows is restricted to
Preview- or Dev-branded builds. Stable builds will retain their current
behavior.
¹ `XxxOverrideSource` returns the profile *above* the current profile
that holds a value for setting `Xxx`. When the value is the
compiled-in value, `XxxOverrideSource` will be `null`. Since it's
supposed to be the profile above the current profile, it will also be
`null` if the profile contains a setting at this layer.
In short, `null` means "user specified" *or* "compiled in". Oops.
Fixes#10430
Validation
----------
* [x] Tested Release build to make sure it's mostly arrow-free (apart from fragments)
## Summary of the Pull Request
Adds a feature flag `Feature_EditableActionsPage` that controls whether the Actions page in the Settings UI is read-only vs editable. The editable version is disabled for `Release` builds and enabled everywhere else (i.e. Dev, Preview, etc...).
Validated using `<stage>` `AlwaysEnabled` and `AlwaysDisabled`.
## References
#6900 - Actions Page Epic
## PR Checklist
Closes#10578
## Summary of the Pull Request
This commit fixes various race conditions regarding the settings UI. It's unsafe to write to class members from background threads without acquiring mutexes or yielding to the main thread first.
By changing the settings reload code path to yield to the main thread early, we're able to cut down on code complexity and unsafe member accesses.
## PR Checklist
* [x] Closes#9273
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* Settings UI reloads without crashing ✔️
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/issues/8881
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.
## Detailed Description of the Pull Request / Additional comments
* Preserve profile GUID upon item Tag creation.
* Use this GUID rather than the current profile GUID to select an item
upon settings update.
So even if the profile was renamed and the GUID has changed,
the GUID in the tag remains unchanged and can be found
upon discarding.
This reverts commit a3a2a4102d.
#10046 causes a crash on save. MainPage::UpdateSettings() is unable to update the navigation view's selected item due to an "incorrect parameter". This is particularly strange to see because #10046 only modifies the navigation view's header, not the menu items themselves. Reverting this change fixes that crash (verified).
Reopens#9694
## Summary of the Pull Request
This PR lays the foundation for a new Actions page in the Settings UI as designed in #6900. The Actions page now leverages the `ActionMap` to display all of the key bindings and allow the user to modify the associated key chord or delete the key binding entirely.
## References
#9621 - ActionMap
#9926 - ActionMap serialization
#9428 - ActionMap Spec
#6900 - Actions page
#9427 - Actions page design doc
## Detailed Description of the Pull Request / Additional comments
### Settings Model Changes
- `Command::Copy()` now copies the `ActionAndArgs`
- `ActionMap::RebindKeys()` handles changing the key chord of a key binding. If a conflict occurs, the conflicting key chord is overwritten.
- `ActionMap::DeleteKeyBinding()` "deletes" a key binding by binding "unbound" to the given key chord.
- `ActionMap::KeyBindings()` presents another view (similar to `NameMap`) of the `ActionMap`. It specifically presents a map of key chords to commands. It is generated similar to how `NameMap` is generated.
### Editor Changes
- `Actions.xaml` is mainly split into two parts:
- `ListView` (as before) holds the list of key bindings. We _could_ explore the idea of an items repeater, but the `ListView` seems to provide some niceties with regards to navigating the list via the key board (though none are selectable).
- `DataTemplate` is used to represent each key binding inside the `ListView`. This is tricky because it is bound to a `KeyBindingViewModel` which must provide _all_ context necessary to modify the UI and the settings model. We cannot use names to target UI elements inside this template, so we must make the view model smart and force updates to the UI via changes in the view model.
- `KeyBindingViewModel` is a view model object that controls the UI and the settings model.
There are a number of TODOs in Actions.cpp will be long-term follow-ups and would be nice to have. This includes...
- a binary search by name on `Actions::KeyBindingList`
- presenting an error when the provided key chord is invalid.
## Demo
## Summary of the Pull Request
Adds the profile icons to the page header. I had to manually create the header, and manually bind it to the `Icon` and `Content` of each `NavViewItem`.
It's important that each `NavViewItem`'s icon is set as an `IconSource`, so that we can bind to it. If it's just a plain old `FontIcon`, then we can't re-use it.
Additionally, I removed the manual sizing of all font icons to font size 12. That would make font icons _tiny_ in the header. Now, they'll properly re-use the size of the `NavigationViewTitleHeaderContentControlTextStyle` in the nav view header. This involved also manually making the icons smaller on the `AddProfile` page and in the `CommandPalette`.
As per usual, images are in Teams
## PR Checklist
* [x] Closes#9694
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* Checked (bitmap|font) icons in tabs
* (bitmap|font) icons in the flyout
* (bitmap|font) icons in command palette
* (bitmap|font) icons in the nav view
* (bitmap|font) icons in the header
* (bitmap|font) icons in the add profile page
- Whenever we add a new profile setting from now on we have to update
`Profile::CopySettings` _and_ `CascadiaSettings::DuplicateProfile` 👎
Notes from bug bash (checked bugs have been resolved):
- [ ] The duplicate list can be very long if you have profiles
- [x] DH: "Create new" seems too vague. "New empty profile" or something
seems a little clearer to me.
- [x] There is no deduplication counter for name
- [x] Crash when your settings file is corrupt and we had to fall back
to the defaults and you duplicate a profile
- [x] Crash due to #10003
## PR Checklist
* [x] Closes#9121
This is required to maintain the modality of the dialogs, which we lost
when we moved from Pickers to IFileDialog. The HWND hosting Window API
we dreamed up is incompatible with IModalDialog, because IModalDialog
requires the HWND immediately upon `Show`. We're smuggling it in a
uint64, as is tradition.
zadjii-msft noticed this in #9760.
Removes base layer (aka profiles.defaults) from the Settings UI. `SettingContainer` was also updated to not present a revert arrow when overriding a base layer value.
The new experience is now as follows:
- The revert arrow will only appear if you are overriding a value from a fragment extension.
- Users are still able to fully interact with `profiles.defaults` in their settings.json. Doing so still propagates those changes to their profiles as normal. In this case, the Settings UI presents the base layer value as the one that you selected.
#6800 - Settings UI Epic
Closes#9539
This was the only thing blocking me from signing off on #9224 in 1.7.
! CHANGE WARNING !
If we bind to `T.S.M.Command`s in XAML, then the compiler gets _very
angry_ at us. It generates two different versions of
`GetReferenceTypeMember_Icon` in `XamlTypeInfo.g.cpp`. Presumably
because there's an Icon on a NavViewItem and an Icon on a Command. We
don't really know why. Fortunately, the fix is "rename Command::Icon" to
"Command::IconPath". It's dumb, but it works. Thanks for the help with
that one Carlos ☺️
Unblocks #9224
This PR adds improved override message generation for inheritance in
SUI. The settings model now has an `OriginTag` to be able to denote
where a `Profile` came from. This tag is used in the `SettingContainer`
to generate a more specific override message.
## References
#6800 - SUI Epic
#8919 - SUI Inheritance PR
#8804 - SUI Inheritance (old issue)
## Detailed Description of the Pull Request / Additional comments
- **Terminal Settings Model**
- Introduced `PROJECTED_SETTING` as a macro to more easily declare the
functions for each setting
- Introduced `<setting>OverrideSource` which finds the `Profile` that
has \<setting\> defined
- Introduced `OriginTag Profile::Origin {Custom, InBox, Generated}` to
trace where a profile came from
- `DefaultProfileUtils` creates profiles for profile generators. So
that now sets the `Origin` tag to `Generated`
- `CascadiaSettings::LoadDefaults()` tags all profiles created as
`InBox`.
- The view model had to ingest the API change to be able to interact
with `<setting>OverrideSource`
- **Override Message Generation**
- The reset button now has a more specific tooltip
- The reset button now only appears if base layer is being overridden
- We use the settings model changes to determine the message to
display for the target
## Validation Steps Performed
Tested the following cases:
- overrides nothing (inherited setting)
- overrides value inherited from...
- base layer
- a profile generator
- in-box profile
- global settings should not have this feature
- 0b0dbdf Makes the browse buttons center vertically aligned
- This is now made possible by #8919. The "center" used to include the height of the header. Now that it's separated, the center is solely calculated to be the text box.
- Closes#8764
- 0288f06 Fix keyboard navigation focus for color schemes rename button
- Enter/Esc when in the scheme renamer now focuses the combo box
- Keyboard-invoking accept/cancel button focuses the rename button
- References #8765 and #8768
- d5ef552 Cyclical tab navigation
- now, if you try to tab past the save button, you cycle back to the beginning of the navigation view
- this is consistent with the xaml controls gallery
- References #8768
- a613b08 AutomationProperties for Save, Reset, and open json buttons
- References #8899
This reverts the revert in #8838.
The problem was that the `Profile` in the singleton nav state would be
updated before the binding fired, so we'd end up modifying the _new_
profile, because both the old page and the new page would be pointing at
the _new_ profile already.
Instead of using a singleton instance of the profile nav state, we'll
create a new one each time. The new nav state attempt to steal the
selected pivot from the last instance of the nav state, if the profiles
are the same. This means that
This means that we won't end up modifying the new profile. The old
page's nav state will still have the old profile, so it'll still end up
modifying the old `ProfileViewModel`.
## PR Checklist
* [x] I work here
* [x] Tested manually
* [x] Fixes the first point in #8769, again
This adds a "Name" text box to the Profile page in the Settings UI.
Any changes to the name/icon are propagated to the relevant
NavigationViewItem.
Base Layer does not have a "Name".
## References
#6800 - Settings UI Epic
## Summary of the Pull Request
This fixes a bug where renaming/deleting a color scheme would not update profiles that referenced it.
This also adds detection for renaming a color scheme to a name that is already in use, and adds appropriate UI for that.
## References
#6800 - Settings UI Epic
## PR Checklist
* [X] Closes#8756
## Detailed Description of the Pull Request / Additional comments
`Model::CascadiaSettings` was updated to have a `UpdateColorSchemeReferences()` function that updates all profiles referencing the newly renamed color scheme.
`Editor::ColorSchemesPageNavigationState` now takes and exposes a `Model::CascadiaSettings`.
When a color scheme is renamed or deleted, we use `CascadiaSettings` to update our list of color schemes appropriately, then call `UpdateColorSchemeReferences()` to update the profiles.
The tricky part is that `Profile` does not store a direct reference to `ColorScheme`, but rather the name of the color scheme. See [this tread](https://github.com/microsoft/terminal/issues/8756#issuecomment-760375027) for a discussion on this topic.
## Validation Steps Performed
Repro steps from #8756 when renaming/deleting a referenced color scheme.
## Demo
This reverts commit a7d7362b95 introduced by #8803.
Reverting this commit fixes#8836 at the expense of the profile page
remembering the last Pivot selection. The
## References
#6800 - Settings UI Epic
#8803 maintained a `ProfilePageNavigationState` in `MainPage` to remember
the pivot position. However, the two-way binding on the TextBoxes
now seem to happen too late (after the navigation occurs),
resulting in the text being applied to the wrong profile. In other
words, the sequence of events probably looks something like this:
1. user types text (_state.profile = old profile)
2. user moves to new profile
3. navigation completes (_state.profile = new profile)
4. textbox two-way binding fires, setting _state.profile.WHATEVER = value
## Validation Steps Performed
Performed repro sets from #8836. Bug no longer occurs.
Reopens#8769Closes#8836
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"
Removes the visibility hack in `UpdateSettings` where we were hiding
Profile menu items instead of removing them. This hack was removed using
`ReplaceAll`. For an unknown reason, calling `Remove()` would result in
an out-of-bounds error in XAML code.
The "Discard" button would improperly refresh the Settings UI. Both of
the bugs were caused by holding a reference to a hidden menu item then
trying to set the `SelectedItem` to that menu item.
Additionally, 9283375 adds a check for the selected item in
`SettingsNav_ItemInvoked()`. This prevents navigation to an already
selected item. This was the heuristic used by the XAML Controls Gallery.
References #6800 - Settings UI Epic
## Validation Steps Performed
(Repeated for each menu item)
1. Select the menu item
2. click "Discard changes"
3. Verify navigated to same page
Also performed repro steps for #8747 and #8748.
Closes#8747Closes#8748
This PR Makes sure that after you save the settings, we stay on the same part of the profiles pivot. We do this by having a singleton `ProfilesNavigationState`, a bit like the color scheme one in #8799. Hence why this PR is targeting the other.
## PR Checklist
* [x] I work here
* [x] Tested manually
* [x] Fixes the first point in #8769
## Summary of the Pull Request
This PR fixes two of the components of #8765.
> * [ ] Edit a color scheme -> Hit 'apply' -> the selected color scheme resets to the first color scheme in the list (instead of the one just edited)
This was fixed by storing the navigation state as a singleton in MainPage, and having the color schemes page update the selected scheme on that singleton. That way, a subsequent navigation to the schemes page could re-use the existing state.
> * [ ] The buttons turn gray on rollover covering up what color I'm looking at (I have dark mode)
This one was tricky. We're binding the resource for this button, to the color the button is bound to. We're also running a converter on that color, as to change the alpha slightly. This allows us to still have visual feedback on pointerover, without obscuring the color entirely.
## PR Checklist
* [x] I work here
* [x] Tested manually
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
#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 #6800 and will be fixed in
their own PR.
Introduces the following UI controls to the ColorSchemes page:
- "Add new" button
- next to dropdown selector
- adds a new color scheme named ("Color Scheme #" where # is the number of color schemes you have)
- "Rename" Button
- next to the selector
- replaces the ComboBox with a TextBox and the accept/cancel buttons appear
- "Delete" button
- bottom of the page
- opens flyout, when confirmed, deletes the current color scheme and selects another one
This also adds a Delete button to the Profiles page. The Hide checkbox was moved above the Delete button.
## References
#1564 - Settings UI
#6800 - Settings UI Completion Epic
## Detailed Description of the Pull Request / Additional comments
**Color Schemes:**
- Deleting a color scheme selects another one from the list available
- Rename replaces the combobox with a textbox to allow editing
- The Add New button creates a new color scheme named "Color Scheme X" where X is the number of schemes defined
- In-box color schemes cannot be deleted
**Profile:**
- Deleting a profile selects another one from the list available
- the rename button does not exist (yet), because it needs a modification to the NavigationView's Header Template
- The delete button is disabled for in-box profiles (CMD and Windows Powershell) and dynamic profiles
## Validation Steps Performed
**Color Schemes - Add New**
✅ Creates a new color scheme named "Color Scheme X" (X being the number of color schemes)
✅ The new color scheme can be renamed/deleted/modified
**Color Schemes - Rename**
✅ You cannot rename an in-box color scheme
✅ The rename button has a tooltip
✅ Clicking the rename button replaces the combobox with a textbox
✅ Accept --> changes name
✅ Cancel --> does not change the name
✅ accepting/cancelling the rename operation updates the combo box appropriately
**Color Schemes - Delete**
✅ Clicking delete produces a flyout to confirm deletion
✅ Deleting a color scheme removes it from the list and select the one under it
✅ Deleting the last color scheme selects the last available color scheme after it's deleted
✅ In-box color schemes have the delete button disabled, and a disclaimer appears next to it
**Profile- Delete**
✅ Base layer presents a disclaimer at the top, and hides the delete button
✅ Dynamic and in-box profiles disable the delete button and show the appropriate disclaimer next to the disabled button
✅ Clicking delete produces a flyout to confirm deletion
✅ Regular profiles have a delete button that is styled appropriately
✅ Clicking the delete profile button opens a content dialog. Confirmation deletes the profile and navigates to the profile indexed under it (deleting the last one redirects to the last one)
## Demo
Refer to this post [here](https://github.com/microsoft/terminal/pull/8403#issuecomment-747545651.
Confirmation flyout demo: https://github.com/microsoft/terminal/pull/8403#issuecomment-747657842
This commit introduces the terminal settings editor (to wit: the
Settings UI) as a standalone project. This project, and this commit, is
the result of two and a half months of work.
TSE started as a hackathon project in the Microsoft 2020 Hackathon, and
from there it's grown to be a bona-fide graphical settings editor.
There is a lot of xaml data binding in here, a number of views and a
number of view models, and a bunch of paradigms that we've been
reviewing and testing out and designing and refining.
Specified in #6720, #8269
Follow-up work in #6800Closes#1564Closes#8048 (PR)
Co-authored-by: Carlos Zamora <carlos.zamora@microsoft.com>
Co-authored-by: Kayla Cinnamon <cinnamon@microsoft.com>
Co-authored-by: Alberto Medina Gutierrez <almedina@microsoft.com>
Co-authored-by: John Grandle <jograndl@microsoft.com>
Co-authored-by: xerootg <xerootg@users.noreply.github.com>
Co-authored-by: Scott <sarmiger1@gmail.com>
Co-authored-by: Vineeth Thomas Alex <vineeththomasalex@gmail.com>
Co-authored-by: Leon Liang <lelian@microsoft.com>
Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
Signed-off-by: Dustin L. Howett <duhowett@microsoft.com>
