Settings UI inheritance spec #8269

cinnamon-msft · 2020-11-14T00:41:22Z

Summary of the Pull Request

Spec on how we display profile inheritance inside the settings UI.

References

#1564 - Settings UI

carlos-zamora · 2020-11-16T17:47:46Z

doc/specs/#1564 - Settings UI/cascading-settings.md

+
+Each dropdown has either "inherit" or "custom". If the user selects "custom", the original control will appear (i.e. a color picker for colors or a number picker for integers).
+
+This option was not chosen because it added too much overhead for changing a setting. For example, if you wanted to enable acrylic, you'd have to click the dropdown, select custom, watch the checkbox appear, and then select the checkbox.


I meant more like if a dropdown doesn't make sense for the setting, replace all options with a "Custom" control. So the settings would look something like this:

Boolean Settings:

Enabled + Disabled + Inherit

We have so few values to choose from, that it makes sense to consolidate them into one control.

Acrylic Opacity

Custom + Inherit

if Custom, Slider control appears to allow you to select a value

Thought: maybe instead of "appears", we just enable, disable it? That way you can always see the value that is inherited?

This also fixes the color picker pitfall you mentioned above. The overall concept is that if there's too many values to enumerate in a dropdown, replace the list of values with "Custom".

doc/specs/#1564 - Settings UI/cascading-settings.md

carlos-zamora · 2020-11-16T18:03:30Z

doc/specs/#1564 - Settings UI/cascading-settings.md

+
+The Add new profile button in the navigation menu would take you to a new page. This page will have radio buttons listing your profiles along with a default settings option. The user can choose to either duplicate a profile or create a new one from the default settings. Once the user makes a selection, the settings UI will take them to their new profile page. The fields on that profile page will be filled according to which profile selection the user made.
+
+![Add new profile](./add-new-profile.png)


I'm a bit confused by your mockup here:

What's the purpose of the Save/Reset buttons?

The three buttons on the page seem a bit weird to me. Like, the idea is that you can only do one of the three right? It's not clear to me that they're separate actions? Idk. Maybe adding a text description on each action or a tooltip might alleviate this problem.

I was thinking the design would be more like...

There's a "Confirm" button (like the "Save" button) that universally says "I am done with this page, let's move on".

When selecting a profile to duplicate from, it can get messy if people have a lot of profiles. Remember, this will include all of your profiles. Including the hidden ones... which tends to relate to how many dynamic profiles you have haha.

doc/specs/#1564 - Settings UI/cascading-settings.md

DHowett · 2020-11-16T20:17:05Z

So, my major issue with "copy settings to" or "duplicate" or "copy settings from" is that we don't have profile inheritance. It's a whole separate spec, and putting it in the UI suggests that we'll need to figure out our data model for inheritance long before we actually ship SUI.

Note about the "subtitle" option:

Is there a version where we only display the text on the overrides? Everything else will display as though it was set "here", but when you edit it it will get an overriden+reset label.

That way, only the five or so settings that people usually set actually display the subtitle. Everything else remains un-cluttered!

doc/specs/#1564 - Settings UI/cascading-settings.md

ghost · 2020-12-11T18:37:09Z

Hello @cinnamon-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

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 (`@msftbot`) and give me an instruction to get started! Learn more here.

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 #6800 Closes #1564 Closes #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>

Spec on how we display profile inheritance inside the settings UI. [Markdown View](https://github.com/microsoft/terminal/blob/dev/cazamor/spec/sui-inheritance/doc/specs/%231564%20-%20Settings%20UI/cascading-settings.md) ## References microsoft#1564 - Settings UI

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 microsoft#6720, microsoft#8269 Follow-up work in microsoft#6800 Closes microsoft#1564 Closes microsoft#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>

## 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 ![Inheritance Demo](https://user-images.githubusercontent.com/11050425/106192086-92a56680-6160-11eb-838c-4ec0beb54965.gif) ## 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.

carlos-zamora and others added 4 commits November 10, 2020 16:50

Introduce Design Doc for Cascading Settings UI

f22552d

added images

1c500d4

add remaining scenarios

c99e97b

I can't spell at midnight

c4e08f4

cinnamon-msft marked this pull request as ready for review November 16, 2020 17:34

carlos-zamora reviewed Nov 16, 2020

View reviewed changes

cinnamon-msft added 2 commits November 19, 2020 09:56

add changes from discussion

7569184

remove comments

c9e8014

carlos-zamora approved these changes Nov 19, 2020

View reviewed changes

zadjii-msft self-assigned this Nov 20, 2020

zadjii-msft approved these changes Dec 1, 2020

View reviewed changes

zadjii-msft removed their assignment Dec 1, 2020

zadjii-msft added Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs Area-Settings UI Anything specific to the SUI labels Dec 1, 2020

carlos-zamora mentioned this pull request Dec 3, 2020

Implement the Settings UI #8048

Merged

small nit

86f8ac9

miniksa reviewed Dec 10, 2020

View reviewed changes

doc/specs/#1564 - Settings UI/cascading-settings.md Show resolved Hide resolved

doc/specs/#1564 - Settings UI/cascading-settings.md Show resolved Hide resolved

doc/specs/#1564 - Settings UI/cascading-settings.md Show resolved Hide resolved

minor changes

f8d72b4

miniksa approved these changes Dec 10, 2020

View reviewed changes

carlos-zamora mentioned this pull request Dec 11, 2020

Epic: Settings UI Completion #6800

Open

25 tasks

cinnamon-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Dec 11, 2020

carlos-zamora merged commit ba8f385 into main Dec 11, 2020

carlos-zamora deleted the dev/cazamor/spec/sui-inheritance branch December 11, 2020 18:43

This was referenced Jan 12, 2021

Missing Settings UI Settings #8755

Closed

Polish Settings UI: Profile page #8764

Closed

Represent Inheritance in Settings UI #8804

Closed

carlos-zamora mentioned this pull request Jan 28, 2021

Represent inheritance in Settings UI #8919

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Settings UI inheritance spec #8269

Settings UI inheritance spec #8269

cinnamon-msft commented Nov 14, 2020 •

edited by carlos-zamora

Loading

carlos-zamora Nov 16, 2020

carlos-zamora Nov 16, 2020

DHowett commented Nov 16, 2020

ghost commented Dec 11, 2020


		Each dropdown has either "inherit" or "custom". If the user selects "custom", the original control will appear (i.e. a color picker for colors or a number picker for integers).

		This option was not chosen because it added too much overhead for changing a setting. For example, if you wanted to enable acrylic, you'd have to click the dropdown, select custom, watch the checkbox appear, and then select the checkbox.


		The Add new profile button in the navigation menu would take you to a new page. This page will have radio buttons listing your profiles along with a default settings option. The user can choose to either duplicate a profile or create a new one from the default settings. Once the user makes a selection, the settings UI will take them to their new profile page. The fields on that profile page will be filled according to which profile selection the user made.

		![Add new profile](./add-new-profile.png)

Settings UI inheritance spec #8269

Settings UI inheritance spec #8269

Conversation

cinnamon-msft commented Nov 14, 2020 • edited by carlos-zamora Loading

Summary of the Pull Request

References

carlos-zamora Nov 16, 2020

Choose a reason for hiding this comment

carlos-zamora Nov 16, 2020

Choose a reason for hiding this comment

DHowett commented Nov 16, 2020

ghost commented Dec 11, 2020

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 (@msftbot) and give me an instruction to get started! Learn more here.

cinnamon-msft commented Nov 14, 2020 •

edited by carlos-zamora

Loading

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 (`@msftbot`) and give me an instruction to get started! Learn more here.