firefox-devtools / ux

Firefox DevTools UX Community
Mozilla Public License 2.0
103 stars 21 forks source link

Unify search highlight colors across DevTools #14

Open violasong opened 5 years ago

violasong commented 5 years ago

We've got a lot of slightly different yellows, purples, and grays in our search highlight colors. In some cases (Style Editor, Scratch Pad) the highlight color makes the text difficult to read.

There may need to be a distinction between Cmd-G navigable search results (e.g. in Debugger) and places where the filter results rows are highlighted (e.g. Rules).

image

For reference

Other dev tools:

image

Firefox search:

image
Gabiskandar commented 5 years ago

I've created a yellow color study based on the current Nightly DevTools. It seems like a lot of the variation in color is to do with the various tones and opacities layering on top of the various colors (shades in the dark theme predominantly). I turned the browser toolbox on and have the CSS file open with the two themes styling so in the next step we can compare the stylesheet variables with the colors and tweak things in the browser to test and check the total number of hues and their application – this should be relatively easy so I can pick this up again in the next few days (possibly sooner).

If you could check over the study and see if you can see any obvious yellow shades I've missed (the titles aren't perfect 😉) @violasong?

MDT-color-study.pdf

fvsch commented 5 years ago

Looking at the Firefox in-page search highlight, I’m seeing:

What’s interesting is that the second highlight forces the highlighted text to white, to make sure it’s visible and stands out. This approach might be needed to achieve strong highlights in the dark theme, where a brown or dark green or dark orange background might be hard to spot, and a more vibrant background color would make the text impossible to read if it’s not forced to e.g. pure white.

Gabiskandar commented 5 years ago

@fvsch thanks! I think that approach is probably a good idea as the yellow isn't very noticeable in some cases, it just looks a bit murky, however yellow is the 'warning' color, so I presume we need to ensure we use a yellow color that whenever it is an actual warning (i.e. in the filter output) and think about the alternative highlight color when that's appropriate? Obviously taking into consideration the current blues and purples that are already in the dark theme – for example.

I'm also finding the colors are just 'slightly' off the Photon swatches, so that's definitely one thing that needs improvement as a minimum - even if it doesn't change tone to highlight it further. I will look at getting the purples and greens etc. documented also. More ideas, thoughts, and findings welcome!

violasong commented 5 years ago

Will look at this and respond soon! For now I just wanted to add that whatever unified yellow highlight results from this discussion could also be used to replace the (currently orange) DOM-changing color as discussed in this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1014629#c3

violasong commented 5 years ago

Wow, this color study is great, Gabrielle! I laugh/cried at the ridiculousness of it all :D. Totally agree about fixing the colors that are just slightly off Photon.

I'm not sure how you made the "page search" highlight happen, but that looks like it might be the same orange as the HTML-changing orange I just mentioned above? I think Yellow 70 could work as a background color for situations like DOM-changing even if it's a bit murky. Mainly, it will be great to get away from orange, since we try to reserve that color for foxes 🦊

Gabiskandar commented 5 years ago

Ha, thanks @violasong, there seems to be some slightly odd shades of peach going on, I am sure the original person(s) had fair reasoning to add them, but altogether it looks a little off. So, I delved into the toolbox and tried out some colors, I have taken some screenshots for you to peruse.

Yellow study continues:

I have focused on searching the CSS (text highlight), which personally I am finding a horror, partly due to the lighter background grey, the lighter row highlight, and the pink (named red in CSS) text color. All feedback welcome on this one in particular CC @fvsch - I do not think I have found anything close yet, so will move on to the other color studies and see if we stumble upon the answer in another occurrence - the colors will have to align somehow. MDT-search-css-file.pdf

Here is a focus on the page errors in the console, this was a much more pleasant journey, less hassle. I also attempted to remove the peachy coloring from the text (last example). MDT-page-errors.pdf

Sorry it's been a little slow, it's been a busy week, will try to tie up the yellows ASAP and move on to the other colors.

fvsch commented 5 years ago

For the warning logs in the console, I like the Yellow 70 background at 0.1 opacity. The current color is a bit too brown IMO so it would be a welcome change. Some of the other tests seem to have too low contrast (full opacity Yellow 70 and 80).

If we do change that background color it’s probably better to align the text color to a Photon Yellow shade (and away from the current peachy yellow), and likewise for the icon. I suggest a “Yellow 20” shade at #FFF899, though #FFFF99 makes sense too if you look at the Yellow scale in HSL (and how it goes away from red/brown when it gets lighter):

:root {
  --yellow-20: hsl(58, 100%, 80%);
  --yellow-50: hsl(55, 100%, 50%);
  --yellow-60: hsl(51, 100%, 42%);
  --yellow-70: hsl(46, 100%, 32%);
  --yellow-80: hsl(43, 100%, 22%);
  --yellow-90: hsl(39, 100%, 12%);
}

Quick test with different Photon Yellow variants for warning logs (top row, bottom row uses the current colors):

screen shot 2018-09-02 at 20 11 06
fvsch commented 5 years ago

For search highlights I don’t know, I’m just not sure that yellow works in those situations.

Definitely up for having more Photon Yellow shades for console warnings, DOM update highlighting and other kinds of "hey, something just happened here" highlightings. But for search, maybe we need something separate. It’s a different type of highlighting IMO.

violasong commented 5 years ago

Thanks Gabrielle for this continued color study and Florens for the explorations!

Re : Error warnings in console: Yes, I like the Yellow 70 .1 opacity background! For the text color, I agree that the current is too peachy but guess I found both #FFF899 and #FFFF99 to be a tad too lemony 😂. It's possible I'm just not used to it - I'll play around with this more soon.

Re: Highlight colors: we could do something like debugger's outline solution, but perhaps yellow shades instead of gray:

image

Re: changing magenta syntax color to light gray - I've been chatting about this with the debugger team and they've said they prefer the very colorful current palette because gray seems like "disabled." Of course it's still up for discussion but I guess I'd not change it as part of this issue. It's possible that magenta needs to be tweaked a bit.

fvsch commented 5 years ago

The Console warning colors were aligned with Photon Yellow.

I think we could focus on picking colors and styles for 3 highlights:

Maybe we could keep yellow for the DOM change highlight (it's a kind of "warning" that flashes briefly), and go for green for the different search/filtering highlights.

fvsch commented 5 years ago

Re: Highlight colors: we could do something like debugger's outline solution, but perhaps yellow shades instead of gray

Debbugger issue 6744 shows this is problematic, because the highlight can be made of several small elements. So we should probably avoid that style.

A. Here’s a test with Green 50 and black text for currently focused search result, and Magenta 70 and white text for other search results:

screenshot

I think this works well for the Debugger, but it would be inconsistent with the Inspector's highlights. One issue with the Inspector's search highlights is that they are often bigger and can highlight many lines. Currently they're some shade of light yellow, which I slightly tweaked here by using Yellow 50 at 40% opacity:

screenshot

(As a side note, I changed the search input's orange border to Yellow 60. We should probably file a bug for that.)

Having a lot of Magenta here might be excessive:

inspector screenshot

So if we want to keep a yellow highlight for highlighting several search results, here are two options for the Debugger (which could work in the Style Editor as well):

B. Yellow 50 40% opacity and original text color; Green 50 and black text for the current search result.

screenshot

C. Yellow 50 40% opacity and original text color; Green 60 and white text for the current search result.

screenshot

If the 40% opacity is a bit weak (for the short highlights in the debugger), we could push it to 50% or 60%. Stronger opacity like 60% or more might make things a bit strong in the Inspector (which has full-line highlights), so we probably have to compromise to a middle ground if we want to keep the same color in both places.

/cc @mcroud @violasong for review.

darkwing commented 5 years ago

Once a decision is made on this, please ping https://github.com/devtools-html/debugger.html/issues/6744 to let us know the decision. Thank you!

violasong commented 5 years ago

Thanks for the work on this. I really like the idea of Yellow 50 40% for all highlights, but I can see how that gets lost a bit in Debugger. Maybe it's worth doing 40% for line match, but 60% for word match.

Green for current match is great - matches main Firefox too. I think a lighter green would look a little nicer — how about black on Green 50 at 40% opacity?

DPX-designer commented 5 years ago

Echoing a similar opinion here that the green (current match) and yellow (all matches) combination works well. The black text on green succeeds greater accessibility contrast than the white. Good stuff!

violasong commented 5 years ago

A tiny mockup of how .4 green50 looks

image
darkwing commented 5 years ago

Could we get a final answer here? It's blocking a few debugger bugs. Thank you!

violasong commented 5 years ago

Thanks for the nudge! Let's move forward with the following:

Active highlight: light: .3 opacity green-50; dark: .2 opacity green-50 with 1px border: green-50

Non-active highlights: light: .4 opacity yellow-50 dark .2 opacity yellow-50 with 1px border: yellow-60

image image
fvsch commented 5 years ago

@violasong Main issue here is that we can't do borders for highlights in the source code, because a match can be made of several elements. In your example, it would be something like:

<mark>(</mark>
<mark>'eot/FiraSan</mark>
...
<mark>format</mark>
<mark>(</mark>
...

So we end up drawing borders around each element, as in debugger issue #6744. This means we end up with borders in the middle of the matched string.

And CSS selectors are not powerful enough to help us fake a continuous border.

Now, if we could get the highlighted fragments to have classes like:

Then we could work around this issue and paint our borders. @darkwing Would that be doable, or do we need to stick to background/text colors only?

violasong commented 5 years ago

Ah! I was basing assumptions off of the current outline style, and forgot there was a need to change this. It was just tough to change the background color for dark mode without ruining legibility.

I think one thing that needs to happen here is lightening the color of the indigo and purple syntax color so they're brighter, like the green.

fvsch commented 5 years ago

Trying to make all syntax colors compatible with those background highlights would be a lot of work (and the risk of regressing some of those contrasts later on would be high). So I suggest we just force the text color to black or white, like in some of my suggestions in https://github.com/devtools-html/ux/issues/14#issuecomment-445764570

We probably want the search highlights to pop out, I thing it's okay if we lose some syntax hints in the process.

violasong commented 5 years ago

Ideally we choose syntax colors bright enough that a deep highlight would work with all of them, but to unblock this I think the white text on dark green/yellow sounds like a good way to go - will defer to you on this @fvsch, if you're interested :)

digitarald commented 4 years ago

New search UI in Network panel is also affected by this. Could we try to close this bug with some conclusion?

janodvarko commented 4 years ago

Just for the record, there is also: https://bugzilla.mozilla.org/show_bug.cgi?id=1582702

lloan commented 4 years ago

Has there been a decision for the preferred search highlight moving forward? Based on the input on this bug thread, are we moving forward with this look for the search highlighting?

image

lloan commented 4 years ago

Phabricator patch - https://phabricator.services.mozilla.com/D47519

Light: image

Dark: image

fvsch commented 4 years ago

I'm afraid that yellow might not be perceptible for some users, especially compared to the current high contrast brown.

On a white background, #fff69b has a contrast of 1.11 which is very low. A tad more vibrant like #fff266 might work better (1.15 contrast; hard to reach any contrast above 1.5 with yellow on white) for more users, but would not meet accessibility requirements (my understanding of WCAG 2.1 is that we should have 3:1 contrast here).

The outlined style for the Dark theme works well enough but we can't generalize it to code search because CodeMirror splits text into many spans, and a matched string may be composed of several spans; when we add borders, the results are pretty bad.

We could use a border-bottom though, that could help with accessibility too (especially in high contrast mode).

Black text, Yellow 50 background, Yellow 60 border:
Screen Shot 2019-09-28 at 11 21 57

Black text, #fff266 background, Yellow 60 border: Screen Shot 2019-09-28 at 11 23 28

fvsch commented 4 years ago

We're using #fff697 (kind of like a "Yellow 30") for flashing backgrounds on updates in the Inspector, and could probably darken it a bit to a #fff266 (kind of a "Yellow 40").

lloan commented 4 years ago

Got it. So @fvsch have we decided for sure what we want to use? And if we're going to update the variable --theme-contrast-background to reflect that? Just for reference, I'll include the a link to D47519.

digitarald commented 4 years ago

I like @fvsch 's underline+background style, given the extra contrast it adds. @violasong , any comments here from your side our should we take this and run with it?

violasong commented 4 years ago

I like the underline idea! Seems like a great solution to the dark mode problem.

The yellow background in your examples do look a little strong to me. I'd still like to preserve the syntax coloring in places like Rules, and I feel it would look overpowering when mixed with those colors. Maybe the existing #fff69b would be accessible with the added border?

fvsch commented 4 years ago

Edit: updated spec at https://github.com/firefox-devtools/ux/issues/14#issuecomment-542877666

Converging on the existing yellow highlight (used in Inspector Markup and Rules) and the bottom border style:

Element Variable Light theme Dark theme
Text --theme-contrast-color #000000 #ffffff
Background --theme-contrast-background #fff697 #605913
Border --theme-contrast-border Yellow 60 Yellow 65
Light Dark

Update: on selected rows which are white-on-blue, using the yellow highlights can be jarring. In that case I recommend using the selection colors but inverted:

project-search-result-after

violasong commented 4 years ago

Thanks - looks good to me!

fvsch commented 4 years ago

Color tweaks and semi-transparent yellow background

Revised colors, including a semi-opaque background variant to be used in code editors (mostly because CodeMirror paints selection styles behind the highlighted text's background, and we need to combine both to keep the "currently selected search result" visible).

Element Variable Light theme Dark theme
Text --theme-contrast-color #000000 #ffffff
Background w/ Alpha --theme-contrast-background-alpha Yellow 50 at 40% opacity Yellow 50 at 15% opacity
Background (solid) --theme-contrast-background #fff699 #4f4b1f
Border --theme-contrast-border Yellow 60 Yellow 65
contrast-background-alpha

Style for "currently selected" search results

When completing a search in a context where users can navigation between results (e.g. with Cmd+G and Cmd+Shift+G on macOS), the currently selected or "active" match should be green, possibly reusing colors from: https://github.com/firefox-devtools/ux/issues/14#issuecomment-459541389

Right now we are limited by the way CodeMirror handles this use case, so we're keeping the combination of text selection style + semi-transparent yellow highlight mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1588796#c3

But the goal, if we can remove those technical blockers, is to have a much more noticeable style style for the active match.