CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Show colored swatch in CSS Code Hints #10425
Conversation
@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). |
Yeah, that's a change we can easily do anytime. |
@redmunds yep you're right. @marcelgerber could we move the colors before the color names? Thanks for this! |
@larz0 Do you like this one?` Notice that |
@marcelgerber I'd prefer to see the text aligned. Can you put a transparent badge in place for |
@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? |
Just to clarify, we can remove the borders. |
Are you using rgba(255,255,255,0.1) for the dark swatch? |
Yeah, I use |
Could you try a higher opacity until you can barely see the shadow on back? |
@marcelgerber ok sounds good. |
I like it 👍. Ready to merge @larz0? |
I didn't know what you meant when you said "colored badge", but oh man this is sweet. @marcelgerber, you're on fire. :D |
In case you can think of a better naming, go say it. |
I'd suggest "swatch" instead of "badge" if the this code will only be used for colors and not value types. |
@larz0 Changed. |
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. :) |
@marcelgerber UX review is done 👍 @le717 yep in Brackets' context we're picking colors from the code hint menu :) |
@larz0 You're right, and I still agree calling them a swatch works. :) |
$hintObj.addClass("left-margin"); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SVG code hints already have color name hinting, so it would be great if this PR also handles them.
@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. |
I implemented color swatches in my svg-color-swatches branch. It is working fine. I think the |
Yeah, I'll try to do so later this day, but I can't guarantee anything for the JS part. |
@redmunds @sprintr Done. SVG Hints are in, but there's still a lot of duplicated code. 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 |
expect(provider.hasHints(testEditor, implicitChar)).toBe(true); | ||
var hintsObj = provider.getHints(); | ||
expect(hintsObj).toBeTruthy(); | ||
return extractHintList(hintsObj.hints); // return just the array of hints | ||
// return just the array of hints of returnWholeObj is falys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: falys
float: left; | ||
margin-top: 2px; | ||
margin-left: 4px; | ||
margin-right: -8px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can probably use margin
shorthand here: margin: 2px -8px 0 4px;
Done with review. Thanks for the refactoring.
I added issue #10452 . Let me know if I missed anything.
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). |
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:
|
Pushed once again. |
@marcelgerber Looks good, so you can squash commits. |
748b884
to
549be0a
Compare
@redmunds Done. |
Merging. |
Show colored swatch in CSS Code Hints
Just tried this out: oh my yes. This is great. Good job @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