mrlacey / CSInlineColorViz

See samples of the colors you use within your C# code.
https://marketplace.visualstudio.com/items?itemName=MattLaceyLtd.CSInlineColorViz
MIT License
21 stars 2 forks source link

Feature request: Add a color picker on click on adornment #11

Open LaraSQP opened 1 year ago

LaraSQP commented 1 year ago

Popping a color picker is almost trivial.

But to hook it up to the adornment is beyond my skills.

Still, I must ask because this extension is very useful, and adding this functionality would be a boon.

Thank you.

mrlacey commented 1 year ago

There are already a number of other extensions that offer a color picker and the ability to change colors. Have you tried them? Are there any compatibility issues?

I don't want to reinvent the wheel when there are already solutions out there.

LaraSQP commented 1 year ago

I know of this one:

https://shemeerns.wordpress.com/plugins/color-picker/

The problem is that the adornment works poorly (or not at all) and it is quite a heavy extension.

What I am looking for is exactly what yours provides plus the color picker popup. If you know of an extension like that, please let me know.

Or if you can tell me how to respond to click events on the adornment of your extension, I will try to code it myself (fingers crossed).

Thank you.

mrlacey commented 1 year ago

This one looks pretty light weight: https://marketplace.visualstudio.com/items?itemName=NikolaMSFT.InlineColorPickerX64Preview

LaraSQP commented 1 year ago

I'll try that one again.

Thank you.

LaraSQP commented 1 year ago

Darn.

Inline Color Picker does not work with named colors and the support for C# is spotty.

Nonetheless, its code is on GitHub. I might be able to port some of it to your extension.

Thanks again for your help.

mrlacey commented 1 year ago

I can see named colors as an input, but should they also be supported as an output?

If wanting to support named colors as an output it will also be necessary to add support for categorizing named colors by their type. At the moment, I just treat all named colors the same and let the compiler warn if something is invalid. As an example, if you specified ConsoleColor.HotPink you would get a "HotPink" color indicator displayed even though this isn't a valid ConsoleColor (but is a valid System.Drawing.Color). I think it's ok to handle invalid input as if it's valid, but in terms of outputting code, it should always be valid. There might be similar considerations needed for other conversions of formatting and namespaces.

If going from a named color, how should the output code be structured? With FromRgb? With a ColorConverter passing the Hex value? WIll it depend on project type?

If going from FromRgb() and then selecting a color with an alpha channel value, should it also convert to FromArgb?

The hard (time-consuming) part isn't going to be displaying the UI. It's going to be working out all the different scenarios and building appropriate tests.

LaraSQP commented 1 year ago

I've come to the same conclusion after going through the code a few times. I will see what I can do. It's not likely I can get it done but I am going to give it a shot.

mrlacey commented 1 year ago

Let's start by documenting all the possible inputs

LaraSQP commented 1 year ago

I will restrict myself first to the inputs that the extension is already handling and try to "reply back" in kind.

mrlacey commented 1 year ago

Having gone through the RegExs

Supported inputs:

Not yet supported inputs:

Some comments from a quick look through the code:

LaraSQP commented 1 year ago

Darn, that's quick and great. We'll look at it this weekend.

And, yeah, I am not considering the InlineColorPicker XAML. Like you said, too complex.

mrlacey commented 1 year ago

Also need to consider if/when/how changes should be made to the source. Should changes be automatically applied? Do they need to be confirmed? Does the dialog/popup need to display a cancel option not to make any changes? Source changes also need to integrate with the undo stack.

There are also many options for displaying a color spectrum and options for darkness and opacity. I'm not sure which I prefer. Is there an objective "best" option? What are the pros and cons of the different options?

LaraSQP commented 1 year ago

I'm going to go with the simplest option in all cases.

  1. A cancel is a no-go. Otherwise, update.

  2. There are essentially only two types of colors: named or picker. Whatever is in the user's code will be returned.

  3. The user's code must be turned from a string to a color (i.e., the string must have "meaning"). But the replacement on the user's code is always the replacement of a string for another string (i.e., "meaning" is irrelevant). This approach might also take care of the undo stack.

I think I'll code this mess on a form to avoid the hassle of debugging an extension. Once it's done, it can be plugged in.

mrlacey commented 1 year ago

I'm going to go with the simplest option in all cases.

  1. A cancel is a no-go. Otherwise, update.

So will there be a dedicated cancel button? Or will (soft?) dismiss of the picker without specifically choosing a new value be considered a cancel? Relatedly, will there be a button to apply the picked color?

  1. There are essentially only two types of colors: named or picker. Whatever is in the user's code will be returned.

Does this imply you're considering/suggesting two different types of picker?

Is the version with named colors necessary? What does it add that intellisense doesn't provide?

Also, does this mean you're not considering supporting RGB to RGBA changes?

  1. The user's code must be turned from a string to a color (i.e., the string must have "meaning"). But the replacement on the user's code is always the replacement of a string for another string (i.e., "meaning" is irrelevant). This approach might also take care of the undo stack.

We already have the code to turn a named value into hex and the adornment will only be shown if it's something that can be parsed/understood as a color. As the picker will only be available when a known color exists (and adornment shown), we already have a System.Windows.Media.Color instance.

We're going to need to go from an instance of a Color, a set of RegEx matches, and a SnapshotSpan. Then, if given a new color (however this is defined) work out what the replacement text should be. I'd start by looking at adding tests for this.

Integration with the undo stack must be added as part of modifying the code in the editor. It's straight forward but must be added explicitly. I mentioned this earlier so I don't forget.

LaraSQP commented 1 year ago

The ColorDialog has a "close" button on the titlebar. That's the "cancel".

A simple dialog popup with a listview can be used for the named colors. That will provide the (color) preview that intellisense doesn't provide and that forces the user to constantly look up a color table somewhere else (like the property dropdown in the designer).

I do not want to worry about the A in ARGB now. Whatever the user had, it's fine as it is. Alpha channels, in any case, do not preview well in my experience.

The extension parses fine. The only thing there is to add a flag that states what string to return (named, ARGB/RGB, 0x/#) to paste in place of the existing selection.

LaraSQP commented 1 year ago

Turns out my current setup is not hooked to debug extensions and it isn't mine to mess with. I'm out.

I did write the code to capture a mouse click in ColorAdornment which would call an Action passed via the constructor from ColorAdornmentTagger. This might make it possible to reach the adornmentCache in IntraTextAdornmentTagger (with the chosen new color) to then update it in the editor.

Or this could just be complete nonsense. I can't test it.

Apologies for being of no help whatsoever.

mrlacey commented 1 year ago

Well, I can get something to show when I double-click on certain color adorners.

inlinecs-picker-poc1

I know the original text, it's location, and what type of picker to display (list of named colors--first, full ARGB picker--second)

LaraSQP commented 1 year ago

That would certainly do.

mrlacey commented 1 year ago

Well, I can get something to show when I double-click on certain color adorners.

inlinecs-picker-poc1

I know the original text, it's location, and what type of picker to display (list of named colors--first, full ARGB picker--second)

I haven't forgotten this. I just can't get the popup to correctly support light dismiss and scrolling the contents of the popup gets confused with the scrolling of the page and it makes for a horrible experience that I certainly wouldn't want to use. Back to the drawing board....

LaraSQP commented 1 year ago

Honestly, "light dismiss" is more of a quirk. If it is modal, that's fine too. After all, it is invoked via a double-click.

Also, you can simplify by mixing in some WinForms which would give you access to the ColorDialog and that would deal with the non-named colors. For the named and system colors, you can just use a listview and dump the enum KnownColor with a preview (ForeColor ^ 0xffffff, BackColor would do). That's how I did it, sorting by hue ls.OrderByDescending( color => color.GetHue() ).ThenBy( o => o.R * 3 + o.G * 2 + o.B * 1 )

There must be WPF alternatives too.

mrlacey commented 9 months ago

I haven't forgotten about this. Going for a simple (well, relatively simple) modal dialog option. Here's an early look at changing a named color:

csinlinecolor-dialog

Double-click the colored square to open the dialog. Select from the list sorted alphabetically or by color. Cancel by closing the dialog without clicking on a color.

LaraSQP commented 9 months ago

That looks great.

Are you using WPF for this?

If so, you can use the Extended WPF Toolkit to also include a ColorPicker:

https://github.com/xceedsoftware/wpftoolkit/wiki/ColorPicker

ColorPicker_color_picker_standard

ColorPicker_color_picker_advanced

mrlacey commented 9 months ago

Yes, this is with WPF. The Xceed WPFToolkit ColorCanvas could be useful for some of the other options.