Rejuvenate the color schemes page in the SUI to follow Win 11 style guidelines

[PankajBhojwani]

trafficstars

Update the color schemes page with the new Win 11 style

• [ ] Closes #9775

Detailed Description of the Pull Request / Additional comments

• With the new layout, we no longer need an 'enter rename mode' button, so all that logic has been removed

Validation Steps Performed

Screenshots below

[PankajBhojwani]

Screenshots: Deleted because these were outdated, see below for the newer ones

[DHowett]

So, I've got some feelings about this new Color Schemes page. I think we might have gone "too hard" on Expanders, to the point where the page has lost coherence. It's a page for editing color schemes, it should not take so many clicks to edit a color scheme, you know? Hiding the various types of color, which are all kinda the same setting at the end of the day, in different groups... it rubs me the wrong way.

In addition to that, Add New and Rename getting their own areas, separate from the scheme, feels like they were just bolted on as an afterthought.

I want us to stop and reconsider this page, and how it flows. The preview on top, the scheme selector (which is a single setting that controls the contents of the page), and then the colors might be the wrong order in which to tackle it.

If we had a ground-up redo, what would our color scheme editor look like? How would we want the user to flow through it?

[zadjii-msft]

If we had a ground-up redo, what would our color scheme editor look like? How would we want the user to flow through it?

<bad idea time>

It's a giant list of all the schemes as expando buttons. Clicking them takes you to a subpage that's basically the same thing we have today, but with a preview?

Or they're all expanders with the 20 buttons and renamer nested in the expanders, all on the main page

[cinnamon-msft]

I think I agree with Dustin here (even though I think I designed this a while back?). I'm wondering how we can make it Windows 11-like but not remove usability. I think seeing all the colors at once is definitely a must have. I can marinate on this and mock up something new. 😄

[cinnamon-msft]

Okay actually I just checked out the Colors page in Windows Settings, and it opened with the Accent color expander already opened. Maybe we can just have the Terminal colors and System colors expanders open by default, so there isn't an extra click and you can see the colors instantly?

[PankajBhojwani]

Screenshots:

[zadjii-msft]

if the user deleted the scheme with the keyboard, shouldn't we yeet focus back to the list? also, do we throw focus anywhere else for any reason?

We should probably follow #11971 (or just generally make sure that still works). In that one, we were always yeeting back to the list, esp if the next scheme selected would be a default scheme that's not editable

[zadjii-msft]

disclaimer: I am not a UX designer

As a side thought, should we have little arrows at the right of these "expanders" that lead to the edit page?

They wouldn't say edit (except in a tooltip), but they would intuitively lead me to think there's more for that scheme if I follow that arrow.

I don't think we should get rid of the "edit" button (as another highly visible, intentional button), but just a thought.

[DHowett]

I don't know if we should put buttons inside the thing. We may want to consider double-click pushing on the Edit page, but... what do we do in the future when inbox schemes can't be edited (only copied)?

[PankajBhojwani]

if the user deleted the scheme with the keyboard, shouldn't we yeet focus back to the list? also, do we throw focus anywhere else for any reason?

Focus gets sent back to the list now, and we don't throw focus anywhere else for any reason

are these readable by screen readers?

Only the name of the color scheme gets read out by narrator as the keyboard focus moves through the list view items

[PankajBhojwani]

are these readable by screen readers?

Oh wait you meant the preview window in the edit page, keyboard focus cannot land on it so nope, screen readers can't read it

[github-actions[bot]]

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view or the :scroll:action log for details.

Unrecognized words (12)

CLA
dcompile
decrst
Devops
nowait
STDIO
STGM
TLDP
Unintense
uwa
Wikipedia
WORKITEM

Previously acknowledged words that are now absent

ansicode appconsult askubuntu aspx azuredevopspodcast azurewebsites biblioscape bitsavers charlespetzold Checkin ckuehl cla cliutils codeproject colororacle condev Consolescreen cppreference css cxcy DCompile debolden deconstructed DECRST DECRSTS decstandar devblogs devicefamily devops dostips DWMWA dwriteglyphrundescriptionclustermap dxp easyrgb ecma edu errno fabricbot FARPROC fcb fdopen fixterms freedesktop GETKEYSTATE guardxfg HOWTO HPAINTBUFFER HPROPSHEETPAGE htm iconify ietf intptr inwap ipa issuecomment kayla kovidgoyal leonerd linkid LLVM locstudio lparen LPCHARSETINFO manpage MAPVIRTUALKEY mscorlib MSDL mspartners myignite ned newcursor nlength NOWAIT ocf opensource openxmlformats osfhandle pdp PENDTASKMSG pgorepro pgort PGU php Poli PPORT PSMALL rapidtables RDONLY rdpartysource redistributable referencesource restrictedcapabilities Rexx rfc robertelder rosettacode rparen runasradio scm SOURCESDIRECTORY stackoverflow stdio stgm techcommunity technet testmddefinition Timeline timelines tldp toolbars Toolset ucdxml unintense universaltest UWA UWAs uwaterloo uwspace viewtopic VKKEYSCAN vstudio wddmconrenderer wdx wfdopen wfstream wikia wikipedia windev windowsdeveloper winfx wline wlinestream workitem WResult xfg Xzn ycombinator

Some files were automatically ignored

These sample patterns would exclude them:

^\Qsamples/ConPTY/EchoCon/EchoCon/EchoCon.vcxproj.filters\E$
^\Qsrc/host/exe/Host.EXE.vcxproj.filters\E$
^\Qsrc/host/ft_host/chafa.txt\E$
^\Qsrc/tools/closetest/CloseTest.vcxproj.filters\E$

You should consider adding them to:

.github/actions/spelling/excludes.txt

File matching is via Perl regular expressions.

To check these files, more of their words need to be in the dictionary than not. You can use patterns.txt to exclude portions, add items to the dictionary (e.g. by adding them to allow.txt), or fix typos.

To accept :heavy_check_mark: these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the [email protected]:microsoft/terminal.git repository on the dev/pabhoj/sui_schemes_page branch (:information_source: how do I use this?):

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect/alphabet.txt
.github/actions/spelling/expect/expect.txt
.github/actions/spelling/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect/df671377d575c8178a9ca2ba29be55f6d29d714f.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
(cat '.github/actions/spelling/excludes.txt' - 2> /dev/null <<EOF
$should_exclude_patterns
EOF
) |grep .|
sort -f |
uniq > '.github/actions/spelling/excludes.txt.temp' &&
mv '.github/actions/spelling/excludes.txt.temp' '.github/actions/spelling/excludes.txt'
}

comment_json=$(mktemp)
curl -L -s -S \
-H "Content-Type: application/json" \
"https://api.github.com/repos/microsoft/terminal/issues/comments/1199780160" > "$comment_json"
comment_body=$(mktemp)
jq -r ".body // empty" "$comment_json" | tr -d "\\r" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")

patch_add=$(perl -e '$/=undef; $_=<>; if (m{Unrecognized words[^<]*</summary>\n*```\n*([^<]*)```\n*</details>$}m) { print "$1" } elsif (m{Unrecognized words[^<]*\n\n((?:\w.*\n)+)\n}m) { print "$1" };' < "$comment_body")

should_exclude_patterns=$(perl -e '$/=undef;
$_=<>;
exit unless s{(?:You should consider excluding directory paths|You should consider adding them to).*}{}s;
s{.*These sample patterns would exclude them:}{}s;
s{.*\`\`\`([^`]*)\`\`\`.*}{$1}m;
print' < "$comment_body" | grep . || true)

update_files
rm $comment_body
git add -u

Available dictionaries could cover words not in the dictionary

This includes both expected items (2856) from .github/actions/spelling/expect/alphabet.txt .github/actions/spelling/expect/expect.txt .github/actions/spelling/expect/web.txt and unrecognized words (12)

cspell:filetypes/filetypes.txt (337) covers 40 of them cspell:django/django.txt (2342) covers 23 of them cspell:html/html.txt (542) covers 22 of them cspell:aws/aws.txt (1485) covers 20 of them cspell:fullstack/fullstack.txt (181) covers 19 of them

Consider adding them using (in .github/workflows/spelling2.yml):

      with:
        extra_dictionaries:
          cspell:filetypes/filetypes.txt
          cspell:django/django.txt
          cspell:html/html.txt
          cspell:aws/aws.txt
          cspell:fullstack/fullstack.txt

To stop checking additional dictionaries, add:

      with:
        check_extra_dictionaries: ''

Warnings (2)

See the :open_file_folder: files view or the :scroll:action log for details.

:information_source: Warnings	Count
:information_source: large-file	1
:information_source: noisy-file	3

See :information_source: Event descriptions for more information.

:pencil2: Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

:warning: The command is written for posix shells. If it doesn't work for you, you can manually add (one word per line) / remove items to expect.txt and the excludes.txt files.

If the listed items are:

• ... misspelled, then please correct them instead of using the command.
• ... names, please add them to .github/actions/spelling/allow/names.txt.
• ... APIs, you can add them to a file in .github/actions/spelling/allow/.
• ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
• ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

:microscope: You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. :wink:

:clamp: If the flagged items are false positives

If items relate to a ...

• binary file (or some other file you wouldn't want to check at all).

  Please add a file path to the excludes.txt file matching the containing file.

  File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

  ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

• well-formed pattern.

  If you can write a pattern that would match it, try adding it to the patterns.txt file.

  Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

  Note that patterns can't match multiline strings.

[zadjii-msft]

I think we can now say that this PR will close out #9775, yea?

[PankajBhojwani]

I think we can now say that this PR will close out https://github.com/microsoft/terminal/issues/9775, yea?

yes

[zadjii-msft]

Yeet and fix in post?

[DHowett]

Does this actually close 9775? Even if the checkbox is unchecked, saying Closes xxx will close issue xxx when the PR merges. :)

[PankajBhojwani]

Does this actually close 9775?

I think it does... I suppose we could wait on feedback before actually closing it though?

[carlos-zamora]

Notes from bug bash (8/2):

• [x] remove big gap at bottom of CS page
• [x] add disclaimer for why "Delete" button is disabled
• [x] update "rename color scheme" disclaimer to only say "cannot be renamed"
• [ ] NIT: clicking "Save" shows the expander animation. It'd be nice if the animation wasn't shown
• [x] (offline) keyboard navigation issue
• [x] NEW text preview doesn't align with color grid

[DHowett]

If we're cracking the seal on this we might want to consider:

1. making double click open up the color scheme, as it is equivalent to Enter in many contexts
2. Somehow resolving the visual margin issue of having a nested "expander" looking thing inside the list
3. Maybe we should not start with one selected; otherwise, it looks like that is the "default" one (which could lead to the same confusion we have had in the past over selecting a scheme and having it not apply to profiles?)

[carlos-zamora]

1. making double click open up the color scheme, as it is equivalent to Enter in many contexts

XAML doesn't seem to provide a way to detect a double click 🙃. We should be able to add logic to detect it ourselves (similar to ControlInteractivity), but I'm gonna say we should do that as a follow-up since it's a bit more complex.

2. Somehow resolving the visual margin issue of having a nested "expander" looking thing inside the list

Writing down some notes so that we have a log of what I've tried. I initially took this as "make the selector pill appear inside the container". That was relatively easy, actually. I leveraged ListView.ItemContainerStyle to set the Background to ExpanderHeaderBackground and that made it look right... at first. The problem was that I then had to go down the rabbit hole of styling the hover/pressed state. I was able to borrow the resources from the Navigator (i.e. the "Advanced" navigator in a profile page), but was left to figure out (1) what color to use to make the selected item stand out and (2) how to change the border color (because in light theme, the border becomes more important, but ListViewItem has no key for border customization).

So I've given up on the approach above (at least for this PR), and am now removing the container entirely to make it look more like the Actions page (except with the selector pill on the left).'

3. Maybe we should not start with one selected; otherwise, it looks like that is the "default" one (which could lead to the same confusion we have had in the past over selecting a scheme and having it not apply to profiles?)

Fixed

[carlos-zamora]

Screenshots for 6e6c0fd:

[carlos-zamora]

This now includes #13839 (and any propagated changes)

Ready for review!

[DHowett]

I removed this from the body. Kayla's experience suggests the opposite.

• [ ] Closes #9775

[DHowett]

Whoops, i clicked Approve instead of collapsing the first file I read

[carlos-zamora]

Screenshots for 5a6ab36

[ghost]

Hello @carlos-zamora!

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.

[carlos-zamora]

Huh, I guess the bot's broken today. Merging!

[ghost]

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

Handy links:

• Release Notes
• Store Download