Represent Inheritance in Settings UI #8804
Labels
Area-Settings UI
Anything specific to the SUI
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Product-Terminal
The new Windows Terminal.
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Comments
cinnamon-msft
added
Issue-Bug
ghost
added
Needs-Tag-Fix
|
The same error occurs if you used to have a font face in your Base layer, then remove it. Then if you create a new profile and don't set its font, the error occurs again because |
|
This is all caused by This is really a case of us needing to implement #8269, so that users can explicitly clear/inherit a setting. |
carlos-zamora
added
Product-Terminal
|
Carlos, repurpose this issue to be the per-field inheritance implementation for 8269. |
carlos-zamora
changed the title
carlos-zamora
removed
the
Needs-Triage
This was referenced Jan 21, 2021
ghost
pushed a commit
that referenced
this issue
Feb 8, 2021
## Summary of the Pull Request Introduces the `SettingContainer`. `SettingContainer` is used to wrap a setting in the settings UI and provide the following functionality: - a reset button next to the header - tooltips and automation properties for the setting being wrapped - a comment stating if you are currently overriding a setting ## References [Spec - Inheritance in Settings UI](https://github.com/microsoft/terminal/blob/main/doc/specs/%231564%20-%20Settings%20UI/cascading-settings.md) #8804 - removes the ambiguity of leaving a setting blank #6800 - Settings UI Epic #8899 - Automation properties for Settings UI #8768 - Keyboard Navigation ## PR Checklist * [X] Closes #8804 ## Detailed Description of the Pull Request / Additional comments A few highlights in this PR: - CommonResources.xaml: - we need to merge the SettingContainerStyle.xaml in there. Otherwise, XAML doesn't merge these files properly and can't apply the template. - Profiles.cpp: - view model checks if the starting directory and background image were reset, to determine which value to show when unchecking the special value - `Profiles::OnNavigatedTo()` needs a property changed handler to update its own "Current<Setting>" and update the UI properly - Profiles.xaml: - basically wrapped all of the settings we want to be inheritable in there - `Binding` is used instead of `x:Bind` in some places because `x:Bind` can't find the parent `SettingContainer` and gives you a compiler error. - Resources.resw: - had to set the "HeaderText" and "HelpText" on each setting container. Does a decent localization burden, unfortunately. - `SettingContainer` files - This operates by creating a template and applying that template over other settings. This allows you to inject the existing controls inside of this. This means that we need to provide our UIElements names and access/modify them via `OnApplyTemplate` - We had to remove the header from each individual control, and have `SettingContainer` be in charge of it. This allows us to add the reset button in there. - Due to the problem mentioned earlier about CommonResources.xaml, we can't reference anything from CommonResources.xaml. - Using `DependencyProperty` to let us set a few properties in the XML files. Particularly, `Has<Setting>` and `Clear<Setting>` are what do all the heavy lifting of interacting with the inheritance model. ## Demo  ## Validation Steps Performed - Verified correct binding behavior with the following generic setting controls: - radio buttons - toggle switch - text block - slider - settings with browse buttons - the background image alignment control - controls with special check boxes (starting directory and background image) ## Next Steps - The automation properties have been verified using NVDA. This is a part of resolving #8899. - The override text is currently "Overrides a setting". According to #8269, we actually want to add a hyperlink in there that navigates to the parent profile object. This will be a follow-up task as it requires settings model changes.
ghost
added
the
Resolution-Fix-Committed
ghost
pushed a commit
that referenced
this issue
Feb 19, 2021
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
|
🎉This issue was addressed in #8919, which has now been successfully released as Handy links: |
This issue was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Original issues name: Base layer is ignored when profile setting is deleted in SUI
The following bug report is a particular example of inheritance not being represented properly in the Settings UI. In order to fix this, #8269 needs to be implemented.
Environment
Steps to reproduce
Apply a setting in Base layer, then remove the same setting from a specific profile. For example, set the Base layer font face to Cascadia Code and remove Cascadia Code PL from the font face section of PowerShell.
Note that I didn't test other settings with Base layer, so this may be the only one I'm not entirely sure.
Expected behavior
PowerShell should use Cascadia Code because it's defined in Base layer.
Actual behavior
Terminal shows an error saying
"" is not a font face, using Consolas instead.and Cascadia Code is ignored from the Base layer.The text was updated successfully, but these errors were encountered: