Add nullable colors and improve Profile.Icon in settings UI by carlos-zamora · Pull Request #17870 · microsoft/terminal

carlos-zamora · 2024-09-06T19:33:21Z

Summary of the Pull Request

Adds some pre-existing settings ($profile.foreground, $profile.background, $profile.selectionBackground, $profile.cursorColor) to the settings UI. This was accomplished by introducing a new control: NullableColorPicker. This control allows the user to pick a color from the color scheme, set the color to null, and select a color from an advanced color picker.

Improves the UI for the Profile.Icon setting by adding an "Icon Type" combo box. This allows the user to pick from multiple options:

None: sets the icon to "none" which is interpreted as no icon
Built-in Icon: presents a combo box that enumerates the Segoe MDL 2 assets
Emoji: presents a text box with a hint to open the emoji picker
File: presents a text box to input the path of the image to use

Additionally, the rendered icon is displayed in the setting container. If "none", "none" is presented to the user (localized).

References and Relevant Issues

#10000

Detailed Description of the Pull Request / Additional comments

NullableColorPicker control
- includes a built-in NullColorButton to set the current value to null
- includes a "More colors..." button to display an advanced color picker
- uses data templates on data templates (data templates squared?) to convert the current color scheme into a grid of color chips
- color chips display a checkmark (similar to Windows settings personalization). This automatically updates its color to stay compliant with color contrast.
- color chips are added to a list so we can (un)check them when a new color is selected
SettingsContainer changes
- Forked ExpanderSettingContainerStyle to allow for a custom preview template. This way, we can display the current value in the expander and we're not just limited to text.
- changed type of CurrentValue property from String to IInspectable
- added CurrentValueTemplate property to control how to display the current value
Miscellaneous:
- Added a few converters (BooleanToVisibility, ColorToString, ColorToBrush)
- Added NameWithHexCode to ColorTableEntry to expose a color as Red #RRGGBB (used for tooltips and a11y)
- Added ForegroundPreview (and equivalent for other colors) to AppearanceViewModel to deduce the color that will be used

Validation Steps Performed

a11y pass (NVDA, keyboard)
set the color to one of the color chips
set the color to null
set the color to a value from the integrated color picker
control updates properly when a new color scheme is selected
control updates properly when a color scheme has multiple colors of the same value

Follow-ups

[A11y] Screen readers don't read expander's preview text
Add Tab Color to settings UI
Update CursorColor preview to display #FFFFFF as "invert"
Use Leonard's font picker UI, with the Segoe icon picker, so that you can filter the list

src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp

src/cascadia/UIHelpers/Converters.h

src/cascadia/TerminalSettingsEditor/TerminalColorConverters.cpp

DHowett · 2024-10-03T23:49:11Z

src/cascadia/TerminalSettingsEditor/TerminalColorConverters.cpp

+    Windows::Foundation::IInspectable ColorToBrushConverter::Convert(Windows::Foundation::IInspectable const& value, Windows::UI::Xaml::Interop::TypeName const& /*targetType*/, Windows::Foundation::IInspectable const& /*parameter*/, hstring const& /*language*/)
+    {
+        const auto color = value.as<Windows::UI::Color>();
+        return Microsoft::Terminal::UI::Converters::ColorToBrush(color);


fwiw if we're adding a static global converter function, we don't need a converter object!

Omg, yeah, I hate it. I'd rather directly use mtu:Converters.ColorToBrush(<param>) in the XML, but the problem is here:

 Fill="{Binding Converter={StaticResource ColorToBrushConverter}}"

We don't have a Path set because we want to convert the object itself. Since there isn't a path, there isn't a way to target the object itself (from what I know, at least). Fill="{Binding mtu:Converters.ColorToBrush}" silently fails (no rendered Rectangle)!

Also, if you're curious, we can't use x:Bind because XamlCompiler error WMC1119: This Xaml file must have a code-behind class to use {x:Bind}. See http://go.microsoft.com/fwlink/?LinkID=532920&clcid=0x409 for more information.

Also also, in case you're extra curious, I wasn't able to figure out how to define the DataType as IReference<Core::Color> because of the IReference part.

So that's why this part's implemented like this 🫤

UGH. Thanks.

src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp

src/cascadia/UIHelpers/Converters.cpp

DHowett

18/26

src/cascadia/TerminalSettingsEditor/Appearances.cpp

src/cascadia/TerminalSettingsEditor/Appearances.xaml

src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml

DHowett · 2024-10-11T22:48:36Z

src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml

+
+        <ContentPresenter Grid.Column="0"
+                          Content="{x:Bind ColorSchemeVM, Mode=OneWay}"
+                          ContentTemplate="{StaticResource ColorSchemeTemplate}" />


📝 ColorSchemeTemplate?

## Summary of the Pull Request Improves the UI for the Profile.Icon setting by adding an "Icon Type" combo box. This allows the user to pick from multiple options: - None: sets the icon to "none" which is interpreted as no icon - Built-in Icon: presents a combo box that enumerates the Segoe MDL 2 assets - Emoji: presents a text box with a hint to open the emoji picker - File: presents a text box to input the path of the image to use Additionally, the rendered icon is displayed in the setting container. If "none", "none" is presented to the user (localized). ✅ Verified as accessible using Accessibility Insights #10000

carlos-zamora · 2024-11-06T19:46:32Z

Alright! Made some minor changes to fix the focus issue, text box issue, theme issue (on Win10), and layout concerns. Here's a storyboard of the slightly new design:

Changes:

Moved color scheme back to the top
Removed text boxes from color picker. IMO this is fine as the hex value is presented in the Expander's header.
- I do still miss having the text box though. What if I add a textbox inside the expander? That'll act as a live preview for the hex value too, and allow users to copy-paste the value directly? It may look a little weird though :/
Combined override settings and color scheme setting into one expander
- Weird idea? Maybe. But there are plenty of examples of expanders having full controls in the header. This groups the settings together, but keeps them all near the top (by the live preview too).

carlos-zamora · 2024-11-19T22:17:11Z

Feedback from Bug Bash (11/19)

Nit: "Color" --> "Cursor Color"
Foreground/Background/Selection background don't get updated when the selected scheme changes
The Icon editor doesn't update when I click the revert button (!)
Icon/ColorScheme SettingContainer has wrong indentation in header (No repro, probably a bad merge)
Nit, probably a puntable thought: can we do Leonard's font picker thing, with the Segoe icon picker, so that you can filter the list? (i may be overindexed on filterable list UIs these days) (Moved as a follow-up)

carlos-zamora · 2024-11-21T17:58:45Z

Note to reviewer

The Profile.Icon changes all come from #17965 which merged into this PR. The PR title and description have been appropriately updated.

Some changes have been made to that area since it merged (just this one commit really): e46ed40

DHowett · 2024-12-11T21:19:24Z

(Code format seems upset)

DHowett

Excellent work!

src/cascadia/TerminalSettingsModel/Profile.cpp

carlos-zamora added 9 commits August 26, 2024 16:15

Implement ColorRef and ColorPickerViewModel

4fe421c

[broken] Add nullable color expanders to SUI

15922b7

Add previews for nullable color expanders

153d97e

[WIP] Add NullableColorPicker (hook up chips)

e3e0b93

temp commit 1

c3d2e98

temp commit 2

629ed07

almost done!

1d87e46

polish!

6e41d9a

code format

fd01fa3

This comment was marked as outdated.

Sign in to view

carlos-zamora mentioned this pull request Sep 18, 2024

Epic: Settings UI v2 #10000

Open

66 tasks

lhecker reviewed Sep 24, 2024

View reviewed changes

better color comparison

6dcc91a

lhecker approved these changes Sep 30, 2024

View reviewed changes

DHowett reviewed Oct 3, 2024

View reviewed changes

src/cascadia/UIHelpers/Converters.h Outdated Show resolved Hide resolved

DHowett reviewed Oct 3, 2024

View reviewed changes

src/cascadia/TerminalSettingsEditor/TerminalColorConverters.cpp Outdated Show resolved Hide resolved

DHowett requested changes Oct 3, 2024

View reviewed changes

microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Oct 3, 2024

carlos-zamora commented Oct 4, 2024

View reviewed changes

src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp Outdated Show resolved Hide resolved

microsoft-github-policy-service bot removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Oct 4, 2024

apply feedback; ensure accessibility

15a460f

carlos-zamora assigned DHowett Oct 10, 2024

DHowett reviewed Oct 11, 2024

View reviewed changes

src/cascadia/UIHelpers/Converters.cpp Outdated Show resolved Hide resolved

DHowett reviewed Oct 11, 2024

View reviewed changes

src/cascadia/TerminalSettingsEditor/Appearances.cpp Outdated Show resolved Hide resolved

DHowett requested changes Oct 11, 2024

View reviewed changes

microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Oct 11, 2024

carlos-zamora assigned carlos-zamora and unassigned DHowett Oct 14, 2024

microsoft-github-policy-service bot removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Oct 14, 2024

carlos-zamora added 2 commits October 28, 2024 12:53

apply feedback

8285dfb

Merge branch 'main' into dev/cazamor/SUI/nullable-color-picker

230b9bc

carlos-zamora assigned carlos-zamora and unassigned carlos-zamora Oct 28, 2024

carlos-zamora added 2 commits November 6, 2024 10:43

Remove broken text boxes from content dialog

3a40272

Rearrange page; add expander for color overrides

c3ad287

carlos-zamora assigned DHowett and unassigned carlos-zamora Nov 7, 2024

carlos-zamora added 2 commits November 20, 2024 13:16

Update color overrides when scheme changes

9df6b03

Fix revert button for Icon

e46ed40

carlos-zamora changed the title ~~Add nullable colors to settings UI~~ Add nullable colors and improve Profile.Icon in settings UI Nov 21, 2024

carlos-zamora added 2 commits December 4, 2024 11:36

Merge branch 'main' into dev/cazamor/SUI/nullable-color-picker

fa1eb46

Merge branch 'main' into dev/cazamor/SUI/nullable-color-picker

08d3d18

DHowett approved these changes Dec 11, 2024

View reviewed changes

src/cascadia/TerminalSettingsModel/Profile.cpp Show resolved Hide resolved

DHowett assigned carlos-zamora and unassigned DHowett Dec 12, 2024

code format + document Icon

54e0560

carlos-zamora mentioned this pull request Dec 12, 2024

SUI Nullable Colors & Icon control Follow-ups #18318

Open

9 tasks

carlos-zamora removed their assignment Dec 12, 2024

carlos-zamora enabled auto-merge (squash) December 12, 2024 19:35

DHowett approved these changes Dec 12, 2024

View reviewed changes

carlos-zamora merged commit 5132f9c into main Dec 12, 2024

carlos-zamora deleted the dev/cazamor/SUI/nullable-color-picker branch December 12, 2024 20:30

DHowett mentioned this pull request Jun 10, 2025

Add nullable colors to Settings UI #9189

Closed

2 tasks

Conversation

carlos-zamora commented Sep 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of the Pull Request

References and Relevant Issues

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Follow-ups

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DHowett Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

carlos-zamora Oct 9, 2024

Choose a reason for hiding this comment

Uh oh!

DHowett Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DHowett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DHowett Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

carlos-zamora commented Nov 6, 2024

Uh oh!

carlos-zamora commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Feedback from Bug Bash (11/19)

Uh oh!

carlos-zamora commented Nov 21, 2024

Note to reviewer

Uh oh!

DHowett commented Dec 11, 2024

Uh oh!

DHowett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

carlos-zamora commented Sep 6, 2024 •

edited

Loading

carlos-zamora commented Nov 19, 2024 •

edited

Loading