Add support for per-profile tab colors #7162

zadjii-msft · 2020-08-03T16:35:54Z

Summary of the Pull Request

This PR adds support for per-profile tab colors, in accordance with #7134. This adds a single tabColor property, that when set, specifies the background color for profile's tab. This color can be overridden by the color picker, and clearing the color with the color picker will revert to this default color set for the tab.

References

Seriously go read Add a spec for per-profile tab colors #7134
Theming is covered in Add comprehensive XAML "theming" functionality #3327 & Add a XAML Theming spec #5772

PR Checklist

Closes settings.json should provide tab color as part of a scheme and/or profile, with an optional way to introduce variant for multiple instances of the same profile #1337
I work here
Tests added/passed
Requires documentation to be updated

Detailed Description of the Pull Request / Additional comments

Seriously go read Add a spec for per-profile tab colors #7134

Validation Steps Performed

Played with setting this color, both on launch and via hot-reload

… hot-reload the color

zadjii-msft

Hey these are some obvious mistakes I made

src/cascadia/TerminalApp/Profile.cpp

src/cascadia/TerminalApp/Tab.cpp

src/cascadia/TerminalApp/TerminalPage.cpp

…rofile-tab-color-impl

carlos-zamora · 2020-08-03T22:54:17Z

src/cascadia/TerminalApp/Tab.cpp

+        // Color                |             | Set by
+        // -------------------- | --          | --
+        // Runtime Color        | _optional_  | Color Picker / `setTabColor` action
+        // Control Tab Color    | _optional_  | Profile's `tabColor`, or a color set by VT


"or a color set by VT"

Shouldn't this be in the line above with "Runtime Color"?

Oh so I'm thinking that this operates more like the tab title, which can be set by VT, but the VT title can also be overridden temporarily at runtime the tab renamer.

Also note that I'm not sure that there is a VT sequence for setting the tab color, that's purely a hypothetical

Also note that I'm not sure that there is a VT sequence for setting the tab color, that's purely a hypothetical

One could argue setting the title to a text string with a VT-sequence to set the background color could reasonably be interpreted as a request / intent to set the tab (or titlebar if tabs are disabled )color.

src/cascadia/TerminalApp/Tab.cpp

src/types/inc/utils.hpp

src/inc/til/coalesce.h

DHowett

I'm confused about why TerminalSettings/Terminal/Core need to know the app-provided color. It seems like we're using TS as a go-between so two parts of TerminalApp can talk to eachother...

…rofile-tab-color-impl

zadjii-msft · 2020-08-06T22:09:24Z

@DHowett and I discussed this and he's more okay with it now that we've considered the fact that the color needs to be associated with each individual control, not just per-tab. It's more like and extension of titlebar state:

titlebar text
titlebar color
titlebar icon

etc.

…rofile-tab-color-impl # Conflicts: # src/cascadia/UnitTests_TerminalCore/MockTermSettings.h

carlos-zamora · 2020-08-07T16:38:45Z

src/cascadia/TerminalCore/ICoreSettings.idl

@@ -34,6 +34,8 @@ namespace Microsoft.Terminal.TerminalControl
        String WordDelimiters;

        Boolean ForceVTInput;
+
+        Windows.Foundation.IReference<UInt32> TabColor;


Shouldn't this be an IControlSetting instead of an ICoreSetting?

So, I was imagining this as just another thing that's similar to the title. If VT can modify the "title color" of the terminal, then that's something that's owned by the Core, not the control. It's the same way title "text" works now.

ghost · 2020-08-26T17:03:39Z

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

Handy links:

zadjii-msft added 2 commits August 3, 2020 10:46

This works to initialize the tabs with the current color, but doesn't…

9d771f5

… hot-reload the color

damn hot-reloading is slick as heck

fb20323

ghost added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal. labels Aug 3, 2020

zadjii-msft commented Aug 3, 2020

View reviewed changes

src/cascadia/TerminalApp/Profile.cpp Outdated Show resolved Hide resolved

src/cascadia/TerminalApp/Tab.cpp Outdated Show resolved Hide resolved

src/cascadia/TerminalApp/TerminalPage.cpp Outdated Show resolved Hide resolved

zadjii-msft added 4 commits August 3, 2020 11:44

nits from things I forgot before submitting the PR

6949387

fix the compilation of the unittests

daf7e63

Merge remote-tracking branch 'origin/master' into dev/migrie/f/1337-p…

26bfa90

…rofile-tab-color-impl

all that just for a _test to pass_?

969c807

carlos-zamora reviewed Aug 4, 2020

View reviewed changes

miniksa reviewed Aug 4, 2020

View reviewed changes

src/cascadia/TerminalApp/Tab.cpp Outdated Show resolved Hide resolved

miniksa reviewed Aug 4, 2020

View reviewed changes

src/types/inc/utils.hpp Outdated Show resolved Hide resolved

Eh let's just move this to it's own file now, that's easier

0b84333

zadjii-msft assigned miniksa Aug 4, 2020

miniksa reviewed Aug 6, 2020

View reviewed changes

src/inc/til/coalesce.h Outdated Show resolved Hide resolved

DHowett requested changes Aug 6, 2020

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 6, 2020

zadjii-msft added 2 commits August 6, 2020 16:45

Rename this to be _simpler_

0edc1d8

Merge remote-tracking branch 'origin/master' into dev/migrie/f/1337-p…

4f57e73

…rofile-tab-color-impl

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 6, 2020

zadjii-msft requested a review from DHowett August 6, 2020 22:09

Merge remote-tracking branch 'origin/master' into dev/migrie/f/1337-p…

3414336

…rofile-tab-color-impl # Conflicts: # src/cascadia/UnitTests_TerminalCore/MockTermSettings.h

carlos-zamora reviewed Aug 7, 2020

View reviewed changes

carlos-zamora approved these changes Aug 7, 2020

View reviewed changes

zadjii-msft assigned DHowett Aug 7, 2020

DHowett approved these changes Aug 7, 2020

View reviewed changes

DHowett merged commit 4e0f313 into master Aug 7, 2020

DHowett deleted the dev/migrie/f/1337-profile-tab-color-impl branch August 7, 2020 23:07

zadjii-msft mentioned this pull request Aug 19, 2020

Panes should have editable titles #7290

Open

cinnamon-msft mentioned this pull request Aug 20, 2020

Document 1.3 release features MicrosoftDocs/terminal#117

Closed

14 tasks

ghost mentioned this pull request Aug 26, 2020

settings.json should provide tab color as part of a scheme and/or profile, with an optional way to introduce variant for multiple instances of the same profile #1337

Closed

ghost mentioned this pull request May 28, 2021

Add an Appearances xaml object and AppearanceViewModel to TSE #10066

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for per-profile tab colors #7162

Add support for per-profile tab colors #7162

zadjii-msft commented Aug 3, 2020 •

edited

Loading

zadjii-msft left a comment

carlos-zamora Aug 3, 2020

zadjii-msft Aug 4, 2020

zadjii-msft Aug 4, 2020

jantari Aug 27, 2020

DHowett left a comment

zadjii-msft commented Aug 6, 2020

carlos-zamora Aug 7, 2020

zadjii-msft Aug 7, 2020

ghost commented Aug 26, 2020

Add support for per-profile tab colors #7162

Add support for per-profile tab colors #7162

Conversation

zadjii-msft commented Aug 3, 2020 • edited Loading

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

zadjii-msft left a comment

Choose a reason for hiding this comment

carlos-zamora Aug 3, 2020

Choose a reason for hiding this comment

zadjii-msft Aug 4, 2020

Choose a reason for hiding this comment

zadjii-msft Aug 4, 2020

Choose a reason for hiding this comment

jantari Aug 27, 2020

Choose a reason for hiding this comment

DHowett left a comment

Choose a reason for hiding this comment

zadjii-msft commented Aug 6, 2020

carlos-zamora Aug 7, 2020

Choose a reason for hiding this comment

zadjii-msft Aug 7, 2020

Choose a reason for hiding this comment

ghost commented Aug 26, 2020

zadjii-msft commented Aug 3, 2020 •

edited

Loading