Fixes #2800 - Color picker (supporting hsl, hsv and rgb)

[tznind]

Here is an updated ColorPicker WIP that supports HSL, HSV and RGB.

Requires a lot of tidy up and tests but this is the outline. I will investigate integrating with #2900 which has lots of stuff on colors and fallback colors (e.g. Ansi).

I studied how it works in Paint.Net and saw the following:

• Hue bar never changes, it always shows full rainbow
• Saturation and Lightness are dependent on Hue and their counterpart. The bar shows what the color would change to if you clicked in that cell.

For RGB the value rendered on each line indicates 'the color that will result if moving the cursor to the position on the bar'. This is why the colors of G and B change as you adjust R.

I think it is a really nice behaviour but happy to take suggestions and/or discuss approach.

I've learned a lot about color models!

Paint.Net Color Picker

ColorPicker2

Fixes

• Fixes Revamp ColorPicker and Color Scenarios in UICatalog #2800

Proposed Changes/Todos

• When bar area is black triangle should turn grey so it is still visible
• Tests
• Scenario
• Events

Pull Request checklist:

• I've named my PR in the form of "Fixes #issue. Terse description."
• My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
• My code follows the Terminal.Gui library design guidelines
• I ran dotnet test before commit
• I have made corresponding changes to the API documentation (using /// style comments)
• My changes generate no new warnings
• I have checked my code and corrected any poor grammar or misspellings
• I conducted basic QA to assure all features are working

[tig]

This looks glorious!

[dodexahedron]

I like.

One thing I had on my personal to-do list was to actually just cherry pick the code from that ColorHelper library that we actually consume, since it's also MIT license, to avoid a whole library dependency for just a few methods and supporting types to make it happen. That seems to me like a nice thing to do for users. Also saves a non-trivial chunk of bytes for the package as well as work for the trimmer, which was actually what prompted me to consider adding that work item.

It also has the (IMO) important impact of not coupling a reference to Terminal.Gui to a reference to that library. Why does that matter? Because, if we cause a third-party dependency to be pulled in, consumers implicitly get access to that library in their project. If we were to switch the dependency or implement something ourselves or anything else that affected that, it could break people's applications until and unless they then manually reference that library after such a change were made to TG. And if they're doing strong naming and such, that might make upgrades on their end harder, as well.

If you wanna go for it, feel free. Otherwise, I'll just keep it as a low-priority item to address during beta/pre-release of TG. All good either way.

Could do it as a git submodule, but I've never loved that workflow and it seems to steepen the learning curve for getting going with a project, making it harder for people who aren't black belts in git-kwon-do to grok.

So, my intent is to preserve the original namespaces and formatting of the copied code and everything and tell the analyzers to buzz off for that code, so it doesn't add any warnings to our build and doesn't get reformatted. The default build configurations would be set up to build it that way. As part of the configuration I'm adding in #3558, I was planning to make a sort of "fully loaded" design time configuration, which would basically exclude that code and pull in the actual package of the same tagged version as the copied code, instead, so anyone actually working on enhancing features of TG would have access to the full library dependency and everything it offers, to make it just a matter of then copying over whatever new stuff gets used, before merge time, for the default configurations.

I actually vaguely remember seeing a Roslyn package out there that automates that last part at build time, even. I'll look into that whenever I get to it.

ALTERNATIVELY, we could just configure the build to trim that dependency specifically and not include the actual transitive build dependency in the package. At least I'm pretty sure there's a combination of incantations in msbuild to make that happen. If not, oh well. The other ways do it, just with slightly more XML. 🤷‍♂️

[tznind]

@tig the side effect of moving color names to a shared resources file is that all other strings are considered colors e.g. 'Save as'

I have added a test to show this.

[Fact]
public void TestColorNames ()
{
    var colors = new W3CColors ();
    Assert.Contains ("Aquamarine", colors.GetColorNames ());
    // Currently fails
    Assert.DoesNotContain ("Save as",colors.GetColorNames ());
}

How do you want to handle that? should there be a seperate resource file for the colors or is it possible to label them somehow so to tell the difference?

[tig]

The bug is in here:

public static IEnumerable<string> GetW3CColorNames ()
    {
        foreach (DictionaryEntry entry in _resourceManager.GetResourceSet (CultureInfo.CurrentCulture, true, true)!)
        {
            if (entry.Value is string colorName)
            {
                yield return colorName;
            }
        }
    }
```

Should be

```cs
public static IEnumerable<string> GetW3CColorNames ()
    {
        foreach (DictionaryEntry entry in _resourceManager.GetResourceSet (CultureInfo.CurrentCulture, true, true)!)
        {
            if (entry.Value is string colorName && colorName.StartsWith(`#`))
            {
                yield return colorName;
            }
        }
    }
```

(or similar)

[tznind]

Ok, we now have old ColorPicker back in again as ColorPicker16 and ColorPickers scenario lets you switch between the two.

I also fixed a bug in the rendering of ColorPicker16 and finished off the W3C color names which should make it easier to select a known color.

[tig]

I submitted another PR with some tweaks.

Can you also please:

• Revert AdornmentEditor etc... to use ColorPicker16
• Fix all warnings

Thanks.

[tig]

Is it possible to update the color name in the picker automatically when the sliders are set to a color that matches?

[dodexahedron]

It may be worth noting that named colors are environment-dependent, and may not be what their names say, depending on terminal, platform, termcaps, and environment variables like $LS_COLORS.

None of which are our problem/fault, but perhaps worth noting at least in docs and in XmlDoc comments around this functionality.

Also, that's relevant to the Color class and associated helpers, which therefore can also pick a potentially unexpected named color if provided numerical values, since it's going to pick them based on standard definitions.

[tznind]

It may be worth noting that named colors are environment-dependent, and may not be what their names say

For this color picker (true color) the colors are the w3c color names not the console colors

I added an interface for the color namer so in future if someone wants non w3c colors/names they can.

[dodexahedron]

Gotcha. Sounds great. 👍

[tznind]

Is it possible to update the color name in the picker automatically when the sliders are set to a color that matches?

Yes this is already how it works, however notice that each movement of the triangle moves multiple units in the color space. For example with RGB if bar width is 100 console characters then each cell is 25.5 units of color space.

You cannot use mouse to 're-select' the 127 value because it maps between cells, we can exactly select 125 so getting to the IndianRed example works

[tznind]

@tig I have restored the old ColorPicker16 for all Scenarios except the LineDrawing and ColorPickers scenarios (although the pickers one does both).

I think I got all the warnings just spelling of 'viewport' and a warning about async tests as well as naming rule violation which is on ResourceManager _resourceManager but when fixing IDE1006 there resharper wants to rename with the underscore so maybe some conflicting rules there.

Let me know if I missed any warnings or other things to do to this PR.

[tig]

Lovely work Thomas. Thank youl