search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

[CLOSED] Show colored swatch in CSS Code Hints #9222

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by MarcelGerber Wednesday Jan 21, 2015 at 13:45 GMT Originally opened as https://github.com/adobe/brackets/pull/10425

(for updated screenshots scroll down) image image

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

MarcelGerber included the following code: https://github.com/adobe/brackets/pull/10425/commits

core-ai-bot commented 3 years ago

Comment by redmunds Wednesday Jan 21, 2015 at 15:23 GMT

@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).

core-ai-bot commented 3 years ago

Comment by MarcelGerber Wednesday Jan 21, 2015 at 15:39 GMT

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

core-ai-bot commented 3 years ago

Comment by larz0 Wednesday Jan 21, 2015 at 17:55 GMT

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

core-ai-bot commented 3 years ago

Comment by MarcelGerber Wednesday Jan 21, 2015 at 18:33 GMT

@larz0 Do you like this one?`

image

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

core-ai-bot commented 3 years ago

Comment by redmunds Wednesday Jan 21, 2015 at 19:14 GMT

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

core-ai-bot commented 3 years ago

Comment by MarcelGerber Wednesday Jan 21, 2015 at 19:33 GMT

Changed again.

image

core-ai-bot commented 3 years ago

Comment by larz0 Wednesday Jan 21, 2015 at 19:57 GMT

@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?

core-ai-bot commented 3 years ago

Comment by larz0 Wednesday Jan 21, 2015 at 20:10 GMT

Just to clarify, we can remove the borders.

core-ai-bot commented 3 years ago

Comment by MarcelGerber Wednesday Jan 21, 2015 at 20:11 GMT

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

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

core-ai-bot commented 3 years ago

Comment by larz0 Wednesday Jan 21, 2015 at 20:12 GMT

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

core-ai-bot commented 3 years ago

Comment by MarcelGerber Wednesday Jan 21, 2015 at 20:13 GMT

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

core-ai-bot commented 3 years ago

Comment by larz0 Wednesday Jan 21, 2015 at 20:14 GMT

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

core-ai-bot commented 3 years ago

Comment by MarcelGerber Wednesday Jan 21, 2015 at 20:19 GMT

Got it like this with a opacity of 0.24: image

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?

core-ai-bot commented 3 years ago

Comment by larz0 Wednesday Jan 21, 2015 at 20:20 GMT

@MarcelGerber ok sounds good.

core-ai-bot commented 3 years ago

Comment by ingorichter Wednesday Jan 21, 2015 at 23:29 GMT

I like it :+1:. Ready to merge@larz0?

core-ai-bot commented 3 years ago

Comment by le717 Thursday Jan 22, 2015 at 03:11 GMT

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

core-ai-bot commented 3 years ago

Comment by MarcelGerber Thursday Jan 22, 2015 at 05:50 GMT

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.

core-ai-bot commented 3 years ago

Comment by larz0 Thursday Jan 22, 2015 at 16:24 GMT

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

core-ai-bot commented 3 years ago

Comment by MarcelGerber Thursday Jan 22, 2015 at 17:03 GMT

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

core-ai-bot commented 3 years ago

Comment by le717 Thursday Jan 22, 2015 at 17:30 GMT

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. :)

core-ai-bot commented 3 years ago

Comment by larz0 Thursday Jan 22, 2015 at 20:49 GMT

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

core-ai-bot commented 3 years ago

Comment by le717 Thursday Jan 22, 2015 at 23:30 GMT

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

core-ai-bot commented 3 years ago

Comment by redmunds Friday Jan 23, 2015 at 00:43 GMT

@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.

core-ai-bot commented 3 years ago

Comment by sprintr Friday Jan 23, 2015 at 05:19 GMT

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.

core-ai-bot commented 3 years ago

Comment by MarcelGerber Friday Jan 23, 2015 at 05:39 GMT

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

core-ai-bot commented 3 years ago

Comment by MarcelGerber Friday Jan 23, 2015 at 16:13 GMT

@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.

core-ai-bot commented 3 years ago

Comment by redmunds Friday Jan 23, 2015 at 17:43 GMT

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).

core-ai-bot commented 3 years ago

Comment by MarcelGerber Friday Jan 23, 2015 at 17:44 GMT

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

core-ai-bot commented 3 years ago

Comment by redmunds Friday Jan 23, 2015 at 17:48 GMT

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:

core-ai-bot commented 3 years ago

Comment by MarcelGerber Friday Jan 23, 2015 at 18:19 GMT

Pushed once again.