Show colored swatch in CSS Code Hints

[marcelgerber]

(for updated screenshots scroll down)

I'd like to get this in Release 1.2 as well so the color support is even better (we did great progress this Release).
cc @redmunds

[redmunds]

@larz0 What do you think about this change?

In general it seems like it would be helpful. The "badges" in the color picker are square with rounded corners, so I think these badges should match (instead of being circles).

[marcelgerber]

Yeah, that's a change we can easily do anytime.

[larz0]

@redmunds yep you're right. @marcelgerber could we move the colors before the color names? Thanks for this!

[marcelgerber]

@larz0 Do you like this one?`

Notice that currentColor has noticeably less margin-left for obvious reasons - is that ok?

[redmunds]

@marcelgerber I'd prefer to see the text aligned. Can you put a transparent badge in place for currentColor, inherit, and transparent ?

[marcelgerber]

Changed again.

[larz0]

@marcelgerber could you make the swatch 12x12 with 2px border radius and use an inner shadow with 2px blur in rgba(0,0,0,0.24) for light code hints and 1px blur in rgba(255,255,255,0.1) for dark code hints?

[larz0]

Just to clarify, we can remove the borders.

[marcelgerber]

Yeah, I got that.
The dark shadow is a little too faint I guess, though (look for the black swatch):

(box-shadow: inset 0 0 1px rgba(255, 255, 255, 0.1);)

[larz0]

Are you using rgba(255,255,255,0.1) for the dark swatch?

[marcelgerber]

Yeah, I use box-shadow: inset 0 0 1px rgba(255, 255, 255, 0.1);

[larz0]

Could you try a higher opacity until you can barely see the shadow on back?

[marcelgerber]

Got it like this with a opacity of 0.24:

I think this is a good one. I mean, I got pretty good eyes and can see it, while others with worse eyes probably won't see it that well. But as black is pretty much the only color that has this issue and everyone should know what black looks like, I'm fine with that.

What do you think?

[larz0]

@marcelgerber ok sounds good.

[ingorichter]

I like it 👍. Ready to merge @larz0?

[le717]

I didn't know what you meant when you said "colored badge", but oh man this is sweet. @marcelgerber, you're on fire. :D

[marcelgerber]

In case you can think of a better naming, go say it.
"badge" is referenced pretty often in the code right now, so it would be great if we can improve wording there.

[larz0]

I'd suggest "swatch" instead of "badge" if the this code will only be used for colors and not value types.

[marcelgerber]

@larz0 Changed.
Btw, are you done with UI/UX review already?

[le717]

I think swatch works too, although when hear the word "swatch" I think of a color palette I can pick colors from. Could just be me though. :)

[larz0]

@marcelgerber UX review is done 👍 @le717 yep in Brackets' context we're picking colors from the code hint menu :)

[le717]

@larz0 You're right, and I still agree calling them a swatch works. :)

[redmunds]

@marcelgerber Can you think of any way to make this a function in utils/ColorUtils.js so it can be shared with other code hint providers?

[redmunds]

SVG code hints already have color name hinting, so it would be great if this PR also handles them.

[redmunds]

@marcelgerber This looks great. We really need to share as much of the code/markup/styles as possible to make it easy to implement in other hint providers, etc.

[sprintr]

I implemented color swatches in my svg-color-swatches branch. It is working fine.

I think the formatHints function should be unified since it is also being used in SVG code hints. The CSS from both extensions can also be unified in styles/brackets.less.

[marcelgerber]

Yeah, I'll try to do so later this day, but I can't guarantee anything for the JS part.

[marcelgerber]

@redmunds @sprintr Done. SVG Hints are in, but there's still a lot of duplicated code.
Notify me when you're done with review and I'll squash commits. Should I add tests for SVG code hints, too? (They'd probably look very similar to the CSS ones)

It looks like Code Hints could use some code cleanup either way - you, @redmunds, could possibly file a spin-off issue on that. I have only looked at CSS & SVG Code Hints, and looking at functions like formatHints, they basically look the same. I guess it's similar with HTML & JS Code Hints.

[redmunds]

Typo: falys

[redmunds]

Can probably use margin shorthand here: margin: 2px -8px 0 4px;

[redmunds]

Done with review. Thanks for the refactoring.

It looks like Code Hints could use some code cleanup...

I added issue #10452 . Let me know if I missed anything.

Should I add tests for SVG code hints, too? (They'd probably look very similar to the CSS ones)

It doesn't seem necessary to duplicate the tests for color filtering. Maybe just the formatting tests (which should ultimately be tested with refactoring in #10452).

[marcelgerber]

With formatting, you mean the tests that already exist in SVG Code Hints, right?

[redmunds]

With formatting, you mean the tests that already exist in SVG Code Hints, right?

I was referring to adding SVG Tests similar to these CSS tests:

• "should show color swatches for background-color"
• "should always include transparent and currentColor and they should not have a swatch, but class no-swatch-margin"
• "should remove class no-swatch-margin from transparent if it's the only one in the list"

[marcelgerber]

Pushed once again.

[redmunds]

@marcelgerber Looks good, so you can squash commits.

[marcelgerber]

@redmunds Done.

[redmunds]

Merging.

[le717]

Just tried this out: oh my yes. This is great. Good job @marcelgerber. 😄