Reorder the color chips grid in the color schemes page #14223

PankajBhojwani · 2022-10-14T21:58:10Z

XAML has an issue in windows 10 where Width="*" does not work properly
inside the DataTemplate for a ListView. Because of this, the color
scheme list view items looked very strange in windows 10 (see #14187).

Thanks to that, we thought up a few new designs for the color schemes
page and selected a new one that blends the color chips with a region
that shows the foreground and background color with the text preview.

Closes #14187

cinnamon-msft · 2022-10-17T18:21:41Z

So uh.. I'm not sure this is the best design choice. The most important info is the color scheme name, which should be the left-most item. Is there something else we can do to get around this bug?

DHowett · 2022-10-17T18:25:57Z

So uh.. I'm not sure this is the best design choice. The most important info is the color scheme name, which should be the left-most item. Is there something else we can do to get around this bug?

I would argue that the most important part of a color scheme is the colors. The name is secondary, and users will use whichever one has the colors they want.

Konsole chose to put the preview to the left of the name as well, in their appearance list.

DHowett · 2022-10-17T18:28:28Z

Perhaps we could experiment with putting the colors under the name? Windows loved a chonky list item.

carlos-zamora · 2022-10-17T18:28:40Z

So uh.. I'm not sure this is the best design choice. The most important info is the color scheme name, which should be the left-most item. Is there something else we can do to get around this bug?

The problem is caused by a XAML bug that only affects Windows 10. I remember seeing Pankaj try and bind to the ActualWidth to try and fix it, but that wasn't working either (unfortunately).

Like Dustin said, Konsole has the preview to the left so this isn't necessarily bad.

My main issue with it is the color chips because they're too big. I hate to say it, but maybe we should remove the color chips?

DHowett · 2022-10-17T19:06:16Z

should remove the color chips?

Hard no from me! They bring so much value to this page -- you can make a color scheme selection based on the look alone, without having to open one up and remember what it looked like.

DHowett · 2022-10-17T19:57:04Z

I've been doing some hacking. Note that these are not polished; centering and alignment issues are to be expected.

distinct name, vertical

Different roundings and spacings.

colorized name, "full width chip"

what if we went crazy?

DHowett · 2022-10-17T19:58:38Z

(Personally, I think I love "full width chip" #-1 with the colors on the left)

lhecker · 2022-10-18T15:46:55Z

I like this one:

The "full width chip" ones look a bit awkward due to the teeeny tiny padding around the text. Makes it hard to get a feeling for the brightness of the text background IMO.

Pankaj's image for instance has a better padding:

Another good option IMO would be to combine the color chips with margin (second image) with the large text background (first image).

zadjii · 2022-10-18T17:07:22Z

I kinda agree with Leonard, but I worry about schemes with bad fg/bg contrast (but then, that's kinda on them)

DHowett · 2022-10-18T19:15:55Z

Team seems to love this:

PankajBhojwani · 2022-10-18T22:38:06Z

Made the changes as per our discussion! Here's the screenshot:

PankajBhojwani · 2022-10-18T22:39:24Z

As a follow up, I'll be adding that treament to the color scheme dropdown in the profile->appearance page

DHowett · 2022-10-18T22:44:43Z

I kinda like the 8x8 color chips with Margin=1 from the screenshot above yours, as well as the top/left/bottom borders around the color chips being the same (rather than narrow on the left and tall on the top/bottom). What do you think?

This slightly violates the suggestion Leonard made where the colored box would take up 75% of the height of the cell. We were prototyping with a 48px cell height and a total height of.. 32?

PankajBhojwani · 2022-10-18T23:03:17Z

I prefer the larger spacing and larger color chips tbh! Makes it a lot easier on the eyes (I feel like I need to squint for the smaller chips/spacing one). I can make the left margin bigger to have all the margins be the same.

Yeah this new one takes more than 75% of the height but I like it for the improved visibility! Happy to change it if the consensus is we prefer the other one though

DHowett · 2022-10-18T23:06:23Z

Hmmmm. So, my attempt was with 8. What if we try 10 or 12? Down from 15, but still not as big as 15.

I'm worried that keeping the chips on the bigger side doesn't help @carlos-zamora / @cinnamon-msft's concern about the text being too far away from the left side.

What does it look like with Size 12 Padding 1?

PankajBhojwani · 2022-10-18T23:34:14Z

Here's the final look:

src/cascadia/TerminalSettingsEditor/ColorSchemes.xaml

DHowett · 2022-10-19T18:46:07Z

src/cascadia/TerminalSettingsEditor/ColorSchemes.xaml

+                                               Padding="10,0,10,0"
+                                               HorizontalAlignment="Center"
+                                               VerticalAlignment="Center"
+                                               AutomationProperties.AccessibilityView="Raw"


Now that this is the only copy of the color scheme name in the UI, we need to do one of two things.

We need to make the entire tree Raw and set AutomationProperties.Name="{x:Bind Name, Mode=OneWay}" on the root element in the DataTemplate (actually, this might be the right way to go)

or

this can't be Raw

I saw tactic 1 in use in another application, but I can't remember which. It could be a really good way to ensure that a11y tools see the entire UI element as a single thing named "Campbell" or "One Half Dark". Maybe there's even a way to set the AutomationProperties.ItemType to color scheme or something. @carlos-zamora cc.

The current implementation works as expected, actually. Looks like the list view is somehow applying the names to the list view item itself.

That said, the "default" tag isn't being read out. I don't think that's new though and I'm not sure how to fix it. You may need to bind the list view item like Dustin said in (1) above, but have the "name" be set to "<name> <defaultTag (if present)>". Alas, that can probably be a follow-up.

Relevant: #13891

carlos-zamora · 2022-10-26T00:01:21Z

Hmmmm. So, my attempt was with 8. What if we try 10 or 12? Down from 15, but still not as big as 15.

I'm worried that keeping the chips on the bigger side doesn't help @carlos-zamora / @cinnamon-msft's concern about the text being too far away from the left side.

What does it look like with Size 12 Padding 1?

I think it looks fine but I have to admit I don't have the eye for this kind of stuff. My concern about the text being too far away from the left side is resolved though.

carlos-zamora

Not blocking, but I have a few questions/comments.

Also, here's another thing I don't know about. If the user enters high contrast mode, we can't guarantee the text here is in high contrast. Is that ok? I'm concerned about the use case for a newly installed terminal. On one hand, it's nice to be see which ones have a high enough contrast and which ones don't. On the other, maybe the right thing to do is to show the names in a high-contrast format? idk.

src/cascadia/TerminalSettingsEditor/ColorSchemes.xaml

carlos-zamora · 2022-10-26T00:06:03Z

src/cascadia/TerminalSettingsEditor/ColorSchemes.xaml

+                                           HorizontalAlignment="Center"
+                                           VerticalAlignment="Center"
+                                           AutomationProperties.AccessibilityView="Raw"
+                                           FontFamily="Cascadia Code"


@DHowett did we ever figure out what happens if we can't find Cascadia Code? Should we care?

So, it falls back to Segoe UI or something. It's weird but not world-ending.

DHowett · 2022-10-28T21:48:08Z

src/cascadia/TerminalSettingsEditor/ColorSchemes.xaml

+                                           Padding="10,0,10,0"
+                                           HorizontalAlignment="Center"
+                                           VerticalAlignment="Center"
+                                           AutomationProperties.AccessibilityView="Raw"


I am still pretty weirded out about this. Why does this work for the accessibility tools? What are they seeing that offers them the actual name?

it looks like it's the ToString for the item itself... hmm

good for now but that probably explains why the default tag wasn't working

ghost · 2022-11-01T17:22:15Z

Hello @zadjii-msft!

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.

XAML has an issue in windows 10 where `Width="*"` does not work properly inside the `DataTemplate` for a `ListView`. Because of this, the color scheme list view items looked very strange in windows 10 (see #14187). Thanks to that, we thought up a few new designs for the color schemes page and selected a new one that blends the color chips with a region that shows the foreground and background color with the text preview. Closes #14187 (cherry picked from commit 23a02c5) Service-Card-Id: 86518549 Service-Version: 1.16

ghost · 2022-12-14T20:45:34Z

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

Handy links:

reorder color chip grid

e1c6965

ghost added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Oct 14, 2022

border background

dc7200f

update size, padding

f4b5d97

DHowett reviewed Oct 19, 2022

View reviewed changes

src/cascadia/TerminalSettingsEditor/ColorSchemes.xaml Outdated Show resolved Hide resolved

DHowett reviewed Oct 19, 2022

View reviewed changes

carlos-zamora added a commit that referenced this pull request Oct 25, 2022

PRE-MERGE #14223 Reorder the color chips grid in the color schemes page

7530309

move padding etc to grid itself, remove border

b18b5bf

lhecker approved these changes Oct 25, 2022

View reviewed changes

carlos-zamora reviewed Oct 26, 2022

View reviewed changes

comment

b9ec64a

DHowett reviewed Oct 28, 2022

View reviewed changes

DHowett approved these changes Oct 31, 2022

View reviewed changes

DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Oct 31, 2022

DHowett added this to To Cherry Pick in 1.16 Servicing Pipeline via automation Oct 31, 2022

zadjii-msft added AutoMerge Marked for automatic merge by the bot when requirements are met and removed AutoMerge Marked for automatic merge by the bot when requirements are met labels Nov 1, 2022

ghost merged commit 23a02c5 into main Nov 1, 2022

ghost deleted the dev/pabhoj/schemes_page_win10 branch November 1, 2022 17:22

DHowett moved this from To Cherry Pick to Cherry Picked in 1.16 Servicing Pipeline Dec 1, 2022

DHowett moved this from Cherry Picked to Shipped in 1.16 Servicing Pipeline Dec 1, 2022

ghost mentioned this pull request Dec 14, 2022

SUI color palettes alignment #14187

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorder the color chips grid in the color schemes page #14223

Reorder the color chips grid in the color schemes page #14223

PankajBhojwani commented Oct 14, 2022 •

edited by DHowett

cinnamon-msft commented Oct 17, 2022

DHowett commented Oct 17, 2022

DHowett commented Oct 17, 2022

carlos-zamora commented Oct 17, 2022

DHowett commented Oct 17, 2022

DHowett commented Oct 17, 2022

DHowett commented Oct 17, 2022 •

edited

lhecker commented Oct 18, 2022

zadjii commented Oct 18, 2022

DHowett commented Oct 18, 2022

PankajBhojwani commented Oct 18, 2022

PankajBhojwani commented Oct 18, 2022

DHowett commented Oct 18, 2022

PankajBhojwani commented Oct 18, 2022 •

edited

DHowett commented Oct 18, 2022

PankajBhojwani commented Oct 18, 2022

DHowett Oct 19, 2022

carlos-zamora Oct 25, 2022

carlos-zamora Oct 27, 2022

carlos-zamora commented Oct 26, 2022

carlos-zamora left a comment

carlos-zamora Oct 26, 2022

DHowett Oct 28, 2022

DHowett Oct 28, 2022

ghost commented Nov 1, 2022

ghost commented Dec 14, 2022

Reorder the color chips grid in the color schemes page #14223

Reorder the color chips grid in the color schemes page #14223

Conversation

PankajBhojwani commented Oct 14, 2022 • edited by DHowett

cinnamon-msft commented Oct 17, 2022

DHowett commented Oct 17, 2022

DHowett commented Oct 17, 2022

carlos-zamora commented Oct 17, 2022

DHowett commented Oct 17, 2022

DHowett commented Oct 17, 2022

distinct name, vertical

colorized name, "full width chip"

what if we went crazy?

DHowett commented Oct 17, 2022 • edited

lhecker commented Oct 18, 2022

zadjii commented Oct 18, 2022

DHowett commented Oct 18, 2022

PankajBhojwani commented Oct 18, 2022

PankajBhojwani commented Oct 18, 2022

DHowett commented Oct 18, 2022

PankajBhojwani commented Oct 18, 2022 • edited

DHowett commented Oct 18, 2022

PankajBhojwani commented Oct 18, 2022

DHowett Oct 19, 2022

Choose a reason for hiding this comment

carlos-zamora Oct 25, 2022

Choose a reason for hiding this comment

carlos-zamora Oct 27, 2022

Choose a reason for hiding this comment

carlos-zamora commented Oct 26, 2022

carlos-zamora left a comment

Choose a reason for hiding this comment

carlos-zamora Oct 26, 2022

Choose a reason for hiding this comment

DHowett Oct 28, 2022

Choose a reason for hiding this comment

DHowett Oct 28, 2022

Choose a reason for hiding this comment

ghost commented Nov 1, 2022

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.

ghost commented Dec 14, 2022

PankajBhojwani commented Oct 14, 2022 •

edited by DHowett

DHowett commented Oct 17, 2022 •

edited

PankajBhojwani commented Oct 18, 2022 •

edited

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.