Allow creating and editing unfocused appearances in the SUI #10317

PankajBhojwani · 2021-06-03T17:33:28Z

Summary of the Pull Request

Adds unfocused appearance creation/configuration in the SUI

There is now an 'Unfocused Appearance' section at the bottom of the 'Appearance' tab in a profile. There is a '+' button to create an unfocused appearance if one does not exist, or a delete button to delete the unfocused appearance if one exists (only one of these buttons is visible at a time).

PR Checklist

Closes #xxx
CLA signed. If not, go over here and sign the CLA
Tests added/passed
Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
Schema updated.
I work here

Validation Steps Performed

carlos-zamora

Looks like you're making good progress 😊

carlos-zamora · 2021-06-04T23:10:25Z

src/cascadia/TerminalSettingsEditor/Appearances.cpp

+            // make sure to send all the property changed events once here
+            // we do this in the case an old appearance was deleted and then a new one is created,
+            // the old settings need to be updated in xaml


Hmm... could we somehow detect something like "AppearanceChanged"?

That's what's happening here already! (Note that we only get here from the _ViewModelChanged event)

The thing is the Appearances object (not the view model) has some settings that xaml binds to (like CurrentColorScheme) that hang around even if the view model is deleted, then when a new view model is created we need to tell xaml that these settings may have changed

src/cascadia/TerminalSettingsModel/Profile.cpp

src/cascadia/TerminalSettingsEditor/Profiles.xaml

carlos-zamora · 2021-06-04T23:21:27Z

src/cascadia/TerminalSettingsEditor/Profiles.idl

+        void CreateUnfocusedAppearance();
+        void DeleteUnfocusedAppearance();
+


You can probably remove these two functions and just go through the profile instead, right? These don't feel like they should be a part of the navigation state.

Unfortunately we need some way to pass the list of color schemes and the window root to the profile view model for it to create an appearance view model. The PVM doesn't have the list of color schemes nor the window root, only the navigation state does.

…/pabhoj/sui_appearances

carlos-zamora

Mostly nits. I'd still like a demo of this in the teams chat before approving.

src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw

carlos-zamora · 2021-06-18T17:33:28Z

src/cascadia/TerminalSettingsEditor/Profiles.xaml

+                                    <Button.Resources>
+                                        <ResourceDictionary>
+                                            <ResourceDictionary.ThemeDictionaries>
+                                                <ResourceDictionary x:Key="Light">
+                                                    <SolidColorBrush x:Key="ButtonBackground"
+                                                                     Color="Firebrick" />
+                                                    <SolidColorBrush x:Key="ButtonBackgroundPointerOver"
+                                                                     Color="#C23232" />
+                                                    <SolidColorBrush x:Key="ButtonBackgroundPressed"
+                                                                     Color="#A21212" />
+                                                    <SolidColorBrush x:Key="ButtonForeground"
+                                                                     Color="White" />
+                                                    <SolidColorBrush x:Key="ButtonForegroundPointerOver"
+                                                                     Color="White" />
+                                                    <SolidColorBrush x:Key="ButtonForegroundPressed"
+                                                                     Color="White" />
+                                                </ResourceDictionary>
+                                                <ResourceDictionary x:Key="Dark">
+                                                    <SolidColorBrush x:Key="ButtonBackground"
+                                                                     Color="Firebrick" />
+                                                    <SolidColorBrush x:Key="ButtonBackgroundPointerOver"
+                                                                     Color="#C23232" />
+                                                    <SolidColorBrush x:Key="ButtonBackgroundPressed"
+                                                                     Color="#A21212" />
+                                                    <SolidColorBrush x:Key="ButtonForeground"
+                                                                     Color="White" />
+                                                    <SolidColorBrush x:Key="ButtonForegroundPointerOver"
+                                                                     Color="White" />
+                                                    <SolidColorBrush x:Key="ButtonForegroundPressed"
+                                                                     Color="White" />
+                                                </ResourceDictionary>
+                                                <ResourceDictionary x:Key="HighContrast">
+                                                    <SolidColorBrush x:Key="ButtonBackground"
+                                                                     Color="{ThemeResource SystemColorButtonFaceColor}" />
+                                                    <SolidColorBrush x:Key="ButtonBackgroundPointerOver"
+                                                                     Color="{ThemeResource SystemColorHighlightColor}" />
+                                                    <SolidColorBrush x:Key="ButtonBackgroundPressed"
+                                                                     Color="{ThemeResource SystemColorHighlightColor}" />
+                                                    <SolidColorBrush x:Key="ButtonForeground"
+                                                                     Color="{ThemeResource SystemColorButtonTextColor}" />
+                                                    <SolidColorBrush x:Key="ButtonForegroundPointerOver"
+                                                                     Color="{ThemeResource SystemColorHighlightTextColor}" />
+                                                    <SolidColorBrush x:Key="ButtonForegroundPressed"
+                                                                     Color="{ThemeResource SystemColorHighlightTextColor}" />
+                                                </ResourceDictionary>
+                                            </ResourceDictionary.ThemeDictionaries>
+                                        </ResourceDictionary>
+                                    </Button.Resources>


Seeing this gave me an idea: #10454

I'm open to updating this once a PR for that goes through!

src/cascadia/TerminalSettingsEditor/Profiles.cpp

carlos-zamora

Discussed offline. Try making the "Unfocused appearance" a title instead of a "subtitle" to see if that helps it pop more and help differentiate it from the subgroups. Other than that, looks great! :)

We'll iterate on the design. I'm sure the team'll give more feedback once it's merged to main and we get a build to selfhost.

DHowett · 2021-07-13T21:20:20Z

want to slap a til::feature on it? to make sure it only happens in preview?

PankajBhojwani · 2021-07-13T23:12:55Z

want to slap a til::feature on it? to make sure it only happens in preview?

Good idea, done!

ghost · 2021-07-13T23:13:11Z

Hello @PankajBhojwani!

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.

mdtauk · 2021-07-13T23:26:21Z

What about a pivot within the "Appearance" section, with Focused and Unfocused?

DHowett · 2021-07-14T00:14:32Z

src/cascadia/TerminalSettingsEditor/Profiles.cpp

+
+    bool ProfileViewModel::EditableUnfocusedAppearance()
+    {
+        if constexpr (Feature_EditableUnfocusedAppearance::IsEnabled())


This is the same as "return Feature_x::IsEnabled()"

Curious: isn't this technically faster because of the if constexpr? I suppose straight up returning IsEnabled() wouldn't be much slower haha, unless it's inline/constexpr? idk much about this stuff hahaha

No. The compiler is smart enough to optimize...

if (true) { return true; } return false;

into

return true;

on its own.

ghost · 2021-08-31T17:14:21Z

🎉Windows Terminal Preview v1.11.2421.0 has been released which incorporates this pull request.:tada:

Handy links:

PankajBhojwani added 22 commits April 30, 2021 06:48

initial

b923b4b

hm

85a6392

cursor shape/height mostly work

d750d56

remove navigated to

ad35c70

Color scheme ish

5920408

background image stuff ish

7eea902

bg image

f5078b8

Remove can delete appearance for now

f6cf9dd

format

0800d64

dependency callback

81a8b8b

font stuff

f179248

format

c8ee89f

comment

62db2fa

deduplicate file picker

5f047a1

Remove color scheme from profiles

6c7d997

abstract

3c3fede

font lists back in profile, ref to profile

05666f2

format

ef71e80

conflict

3dca0dd

conflict, updates from merge main

e384b00

format, rearrange

8e604ba

conflict

9ac1407

carlos-zamora requested changes Jun 4, 2021

View reviewed changes

ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jun 4, 2021

PankajBhojwani marked this pull request as ready for review June 11, 2021 21:02

PankajBhojwani added 4 commits June 15, 2021 16:07

% sign changes

901d4bb

Merge branch 'main' of https://github.com/microsoft/terminal into dev…

7e0264d

…/pabhoj/sui_appearances

move file/image picker to utils.h/cpp

f5b5bb9

tabstop false

fbfdb2d

PankajBhojwani added 8 commits June 21, 2021 14:47

create a new unfocused appearance

6aab43a

delete unfocused appearance

3e97289

hide font things for unfocused

8631907

dont need this

0d8649c

border, send settings update notifications once upon creation

10dda55

dont need if

fd71d30

remove expander, buttons instead

8b8cf55

format

9a6d580

PankajBhojwani force-pushed the dev/pabhoj/sui_unfocused_appearance branch from 07cf8f2 to 9a6d580 Compare June 21, 2021 22:08

carlos-zamora requested changes Jun 22, 2021

View reviewed changes

ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jun 22, 2021

Base automatically changed from dev/pabhoj/sui_appearances to main July 9, 2021 20:44

PankajBhojwani added 3 commits July 13, 2021 13:51

conflit

5c4f5d6

localize

eb9cb96

Format

b6698e6

carlos-zamora approved these changes Jul 13, 2021

View reviewed changes

til feature

38f761d

PankajBhojwani added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jul 13, 2021

ghost merged commit d13c37c into main Jul 13, 2021

ghost deleted the dev/pabhoj/sui_unfocused_appearance branch July 13, 2021 23:33

DHowett reviewed Jul 14, 2021

View reviewed changes

jelonmusk mentioned this pull request Aug 31, 2021

Delete button Styling moved to CommonResource.xaml ; Issue #10454 #11088

Closed

jolsen71 mentioned this pull request Aug 31, 2021

Add support for Acrylic and Opacity in "Unfocused Appearance" #11092

Closed

LuigiImVector mentioned this pull request Jan 12, 2022

Investigate: AcrylicBlur always on in an inactive window #7158

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow creating and editing unfocused appearances in the SUI #10317

Allow creating and editing unfocused appearances in the SUI #10317

PankajBhojwani commented Jun 3, 2021 •

edited

Loading

carlos-zamora left a comment

carlos-zamora Jun 4, 2021

PankajBhojwani Jun 7, 2021

carlos-zamora Jun 4, 2021

PankajBhojwani Jun 7, 2021

carlos-zamora left a comment

carlos-zamora Jun 18, 2021

PankajBhojwani Jun 23, 2021

carlos-zamora left a comment

DHowett commented Jul 13, 2021

PankajBhojwani commented Jul 13, 2021

ghost commented Jul 13, 2021

mdtauk commented Jul 13, 2021

DHowett Jul 14, 2021

carlos-zamora Jul 14, 2021

DHowett Jul 14, 2021 •

edited

Loading

ghost commented Aug 31, 2021

		void CreateUnfocusedAppearance();
		void DeleteUnfocusedAppearance();

Allow creating and editing unfocused appearances in the SUI #10317

Allow creating and editing unfocused appearances in the SUI #10317

Conversation

PankajBhojwani commented Jun 3, 2021 • edited Loading

Summary of the Pull Request

PR Checklist

Validation Steps Performed

carlos-zamora left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlos-zamora left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlos-zamora left a comment

Choose a reason for hiding this comment

DHowett commented Jul 13, 2021

PankajBhojwani commented Jul 13, 2021

ghost commented Jul 13, 2021

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.

mdtauk commented Jul 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DHowett Jul 14, 2021 • edited Loading

Choose a reason for hiding this comment

ghost commented Aug 31, 2021

PankajBhojwani commented Jun 3, 2021 •

edited

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.

DHowett Jul 14, 2021 •

edited

Loading