Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove base layer from settings UI #9539

Closed
cinnamon-msft opened this issue Mar 18, 2021 · 19 comments · Fixed by #9655
Closed

Remove base layer from settings UI #9539

cinnamon-msft opened this issue Mar 18, 2021 · 19 comments · Fixed by #9655
Assignees
Labels
Area-Settings UI Anything specific to the SUI Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Priority-0 Bugs that we consider release-blocking/recall-class (P0) 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
Copy link
Contributor

cinnamon-msft commented Mar 18, 2021

Description of the new feature/enhancement

After discussing with the team, we believe it makes sense to keep base layer as a JSON setting only and see if people request later to be able to apply settings to all profiles with the UI.

We need to remove the base layer page as well as the reset button tooltip functionality that says "Reset to base layer value". We can't remove the reset button entirely because there's a tooltip for when settings override JSON fragments.

image

Additional explanation of why we're doing this

Proposed technical implementation details (optional)

@cinnamon-msft cinnamon-msft added Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Area-Settings UI Anything specific to the SUI labels Mar 18, 2021
@cinnamon-msft cinnamon-msft added this to the Terminal v1.8 milestone Mar 18, 2021
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Mar 18, 2021
@WSLUser
Copy link
Contributor

WSLUser commented Mar 18, 2021

I use the base layer to control the default font for all profiles as well as color scheme. I'm sure I'm not the only one.

@DHowett
Copy link
Member

DHowett commented Mar 18, 2021

@WSLUser you can continue using the JSON file for that. We've had hours of meetings as a team about this, and we're still trying to figure out the best way to expose it to users or whether to expose it at all.

If you've got a unique perspective, we'd love to hear it! 😄

@DHowett
Copy link
Member

DHowett commented Mar 18, 2021

@cinnamon-msft it would benefit our community if you can summarize why exactly we're doing this, so we can point people at the decision in the future. WDYT?

@WSLUser
Copy link
Contributor

WSLUser commented Mar 18, 2021

If I'm trying to only use the Settings UI, I prefer changing a number of defaults that apply to all profiles. Then in each profile, I might make a particular tweak such as adding a background image (which I already did with the json prior to the UI) or an icon for a custom profile. For example, I want Windows Powershell to use "Campbell Powershell" for the color scheme instead of "Campbell". But for all my other profiles, I actually prefer "Tango Dark". \

Here's what I have showing in the json under "defaults":

            "historySize": 12000,
            "rowstoscroll": "1000",
            "colorScheme": "Tango Dark",
            "fontFace": "Cascadia Code",
            "bellStyle": "all",
            "closeOnExit": "always"

@jack775544
Copy link

jack775544 commented Mar 18, 2021

I personally do most of my changes in the base layer, things like theme, font size and other settings like bell config are things I like to have synced across all my different profiles.

Generally I have only really found myself configuring individual profiles for editing the command line of that profile and not much else.

EDIT: To extend on this slightly, I find myself not really using the general tab in the base layer but I do use the other 2 tabs quite a lot.

@Nacimota
Copy link
Contributor

Yep, I'm setting most of my profile settings in the base layer. I haven't fiddled with them much in the settings UI, but only because that feature is fairly recent. I think I'd much prefer to make font changes and so forth in the UI than in JSON if possible.

Can you tell us what the specific reasons were for hiding the base layer? The only issue I can potentially see is in the wording itself; perhaps "defaults" or "all profiles" is clearer than "base layer", but it seems like there's plenty of reason to keep it around in the UI if possible.

@cinnamon-msft
Copy link
Contributor Author

Base layer presents some conflicts when it comes to the JSON fragments and potentially future extensions. Settings applied in Base layer will override settings that come from JSON fragments, which may not be the desired result.

For example, if Ubuntu ships a JSON fragment in their package that uses the Ubuntu Mono font and a purple color scheme, people may want those settings applied, but there's no escape hatch from Base layer. The same goes for if someone wanted to download someone else's configuration in the form of a JSON fragment, Base layer would override any corresponding settings.

Additionally, we're not certain majority of users will want to apply their own layers to settings. Windows Terminal just shipped inside Windows in the Insider Program this week and we are going to see a large uptick in users. Keeping just profile customizations in the settings UI presents the simplest form of applying profile settings without a layering concept.

Lastly, designing the Base layer page along with the reset buttons on the other profile pages has been challenging and we aren't confident it's the most elegant way to display and explain layering. We are always open to new designs on how to represent this, but there are still the challenges above that we'd have to work with.

We see the benefit of having Base layer in the settings UI as being able to apply settings to multiple profiles at once. We have designed other ways that we could do this in the UI without a layering concept. Those designs can be found in this spec and I've added the images here for reference.

We feel that using Base layer is more of an advanced scenario, which is why we're keeping it as a JSON-only setting at the moment. There is always the option that we add Base layer back to the UI in the future once the settings UI is more polished and if there is a great enough demand for it, we just feel it's the safest option to keep it in settings.json for now.

Duplicate profile
image

Copy settings to... button
image
image

@DHowett
Copy link
Member

DHowett commented Mar 19, 2021

In the interest of community transparency, I'll copy my dissenting opinion here 😄

Base layer presents some conflicts when it comes to the JSON fragments and potentially future extensions. Settings applied in Base layer will override settings that come from JSON fragments, which may not be the desired result.

This feels rather like "throwing the baby out with the bath water". We have 30 things that base layer works very well for or makes very easy, but just one thing that we've introduced that might be confusing until users think about it a little more (and that's assuming that people even have fragments ... which we should get data on when fragments go to Stable!)

I only begrudgingly accept that we should hide this feature while we figure out the best way to expose it. Options presented included removing support for it entirely, as well, which most folks weren't in line with.

Open for random thoughts on how fragments could be made to work with Base Layer

There are two logical fragment locations today, "system" and "user". We could just put all user fragments above base layer, and all system fragments below. This solves the problem for Joe S. who just wants to copy a terminal profile from ~ ~ the internet ~ ~ and have it work while still allowing user settings to "win" over Canonical's font or Python's background color.

I come from the school of "the user's settings are the most important thing", and my approach reflects that. The user said "I want my font to be Cascadia Mono," so it would be deeply and incorrectibly wrong for us to allow an application to say "no, you'll take MY settings and LIVE WITH IT."

@htcfreek
Copy link

For me it would make sense to disable all controls overridden by baselayer and show a warning near each of them.

@ghost ghost added the In-PR This issue has a related PR label Mar 29, 2021
@ghost ghost closed this as completed in #9655 Mar 30, 2021
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Mar 30, 2021
ghost pushed a commit that referenced this issue Mar 30, 2021
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
DHowett pushed a commit that referenced this issue Apr 2, 2021
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.

Closes #9539

(cherry picked from commit 19fb9b2)
@vefatica
Copy link

vefatica commented Apr 2, 2021

What's the status of this? I'm against it. I'd rather not have to go to settings.json for the most basic of settings. I wouldn't mind if, in the UI, "Base layer" were renamed "Defaults".

@DHowett
Copy link
Member

DHowett commented Apr 2, 2021

Status right now is "collecting feedback". We want to get this right when we first ship it in Stable, because Stable is the version of Terminal that goes into Windows itself.

@ghost
Copy link

ghost commented Apr 14, 2021

🎉This issue was addressed in #9655, which has now been successfully released as Windows Terminal v1.7.1033.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Apr 14, 2021

🎉This issue was addressed in #9655, which has now been successfully released as Windows Terminal Preview v1.8.1032.0.:tada:

Handy links:

@vefatica
Copy link

I miss it already (1.8.1032.0).

@zadjii-msft
Copy link
Member

"I miss it already" feels a lot like just commenting "+1". This is a thread that's looking for specific user feedback, and proposals on ways to deal with the aforementioned complexities. Any concrete suggestions?

@vefatica
Copy link

I doubt I can be of much help. I don't understand JSON fragments or the problems associated with them. Can't you just put "Defaults" in the UI and NOT have them override JSON fragments? IMO there's too much important and global stuff in "defaults" to make the user jump through hoops to get at it.

@davea1
Copy link

davea1 commented Apr 19, 2021

This feels rather like "throwing the baby out with the bath water". We have 30 things that base layer works very well for or makes very easy, but just one thing that we've introduced that might be confusing until users think about it a little more (and that's assuming that people even have fragments ... which we should get data on when fragments go to Stable!)

I feel quite strongly that the Global stuff absolutely needs to be accessible from the Settings UI and before WT becomes Stable and part of Windows.

Being an advanced user myself (Software Developer), this doesn't truly impede my ability to configure WT, including the Global settings, the way I want. However, I thought the ultimate goal here should be to make this tool accessible to anyone who might want to use it.

Imagine a scenario where one who has primarily been a 'GUI' user now wants to begin to explore the power of a CLI. It makes sense to encourage them to try out the Windows Terminal since it could have obvious benefits.

User asks: "How do I set the default font face/size for all sessions? I don't see this anywhere in the settings."
Reply: "It's simple. For global/default settings, you just need to edit the JSON file."
Period of long dead silence follows.
User eventually replies with: "The what??? I have no idea what that means. Shouldn't such a setting typically be a standard thing and therefore configurable with all the other options? Doesn't other similar software work this way?"
Reply: "True. The Command Prompt and other terminal products similar to WT do typically have such settings surfaced via the GUI."
User: "Oh OK. Well, let me know when this product is ready for normal users."

At this point in this hypothetical scenario, I believe it is clear that we have truly FAILED, unless the goal is for WT to only be useful for advanced users.

Sorry, I don't really know how to solve the issue, but I would have truly thought that temporarily disabling JSON fragments (advanced functionality) would have been the obvious choice here rather than "throwing the baby out with the bath water" as Dustin so eloquently stated in reference to completely ripping the Global settings (basic functionality) from the UI.

On a more positive note, WT is awesome!!! Keep up the amazing work WT team!!!

@davea1
Copy link

davea1 commented Apr 20, 2021

"I miss it already" feels a lot like just commenting "+1". This is a thread that's looking for specific user feedback, and proposals on ways to deal with the aforementioned complexities. Any concrete suggestions?

After thinking about this issue for a while, I think that most would expect any of their explicit user settings to always override any JSON fragment settings unless instructed otherwise, possibly via a new option that would flip this behavior.

@davea1
Copy link

davea1 commented Apr 21, 2021

In the interest of full transparency, here are the default settings that are (currently) important to me:

        "defaults":
        {
            // Put settings here that you want to apply to all profiles.
            "bellStyle": "none",
            "fontSize": 9,
            "padding": "0, 0, 0, 0"
        },

While I do see the benefit of optionally allowing users to copy/duplicate/set values across one/more/all profiles via the UI, I would never want this to fully replace the existence of default settings. From my perspective, having a set of global defaults with only specific values overridden as desired within individual WT profiles results in a much cleaner JSON settings file compared to requiring excessive duplication of 'default' settings across all profiles.

I realize it was already stated that:

Options presented included removing support for it entirely, as well, which most folks weren't in line with.

I just wanted to make it clear that removing support for default settings is NOT something that I would ever be in favor of.

In regard to the precedence hierarchy of the various settings 'layers', I would imagine most users would expect this as default behavior:

user profiles > user defaults > JSON fragments > built-in defaults

and optionally some mechanism/option, either globally or on a per-profile basis, that would reorder the precedence to something like this:

JSON fragments > user profiles > user defaults > built-in defaults

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings UI Anything specific to the SUI Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants