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

Add Base layer back into the SUI as "Defaults" #10430

Closed
cinnamon-msft opened this issue Jun 15, 2021 · 2 comments · Fixed by #10588
Closed

Add Base layer back into the SUI as "Defaults" #10430

cinnamon-msft opened this issue Jun 15, 2021 · 2 comments · Fixed by #10588
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. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@cinnamon-msft
Copy link
Contributor

cinnamon-msft commented Jun 15, 2021

Description of the new feature/enhancement

We'd like to add Base layer back into the settings UI along with an Extensions page to help avoid the conflicts between profiles.defaults and JSON fragment extensions.

The Extensions page will take some time to design, but we'd like to get started with getting Base layer back into the UI, so we'll track the progress on the Extensions page in another issue.

For now, let's add Base layer back into the settings UI with the following improvements:

  • Change the name Base layer to Defaults
  • Add the Defaults page back into the settings UI
  • Add the reset buttons back into the profile pages
  • Collect data on which settings are applied using Defaults
  • Collect data on how often the profile page reset buttons are used

Related: #9539

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. Area-Settings UI Anything specific to the SUI labels Jun 15, 2021
@cinnamon-msft cinnamon-msft added this to the Terminal v1.10 milestone Jun 15, 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 Jun 15, 2021
@DHowett DHowett self-assigned this Jun 15, 2021
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 15, 2021
@ghost ghost added the In-PR This issue has a related PR label Jul 8, 2021
@ghost ghost closed this as completed in #10588 Jul 9, 2021
ghost pushed a commit that referenced this issue Jul 9, 2021
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)
@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 Jul 9, 2021
@ghost
Copy link

ghost commented Jul 14, 2021

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

Handy links:

@ghost
Copy link

ghost commented Oct 20, 2021

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

Handy links:

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. 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.

2 participants