1158204 - Improve highlighted search strings in debugger and styles

Status: NEW

(DevTools :: General, defect, P2)

Description

[Dane MacMillan [:danemacmillan]]

The color contrast chosen for highlighted strings when searching in the debugger tab make it nearly impossible to read.

http://i.imgur.com/ES31d5m.png

The color contrast in the styles tab are better, but they should still be improved.

http://i.imgur.com/8VY2scS.png

Comment 1

[Sebastian Zartner [:sebo]]

The search highlighting should generally be unified, i.e. the text and background colors should always be the same.
There was a similar issue for Firebug including mockups, which could serve as an example.[1]

Sebastian

[1] https://github.com/firebug/firebug/issues/3113

Comment 2

[shivang nagaria]

I like to work on this task, please give me some pointers so that I can start working on this task.

Comment 3

[Brian Grinstead [:bgrins]]

It appears that the issue with the debugger is that the editor window isn't focused so we are bumping into Bug 706209 and being unable to style the selection on inactive windows.  Not sure what the best approach is for this - we've discussed integrating the debugger search into the editor, but I'm unsure how that would work with cross-file searching.

The Style Editor is easier to deal with because we are using the CodeMirror search plugin which gives us a .cm-searching class that we can modify easily.

Helen, do you have a suggestion for what a search match in the Style Editor should look like in the light / dark theme (currently http://i.imgur.com/8VY2scS.png)?  If we still want a yellow-like background color then I guess we could add a theme-yellow option to https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors and then use the appropriate one per-theme (along with a consistent dark text color).

Comment 4

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: Screen Shot 2016-01-11 at 9.09.46 AM.png

So I think we would gain a lot just by making the background colors a little more neutral. (See screenshot—I went with some blue-ish grays playing off of the line-highlight color.)

Also made sure that the new colors pass accessibility.

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to Helen V. Holmes (:helenvholmes) (:✨) from comment #4)
> Created attachment 8706367 [details]
> Screen Shot 2016-01-11 at 9.09.46 AM.png
> 
> So I think we would gain a lot just by making the background colors a little
> more neutral. (See screenshot—I went with some blue-ish grays playing off of
> the line-highlight color.)
> 
> Also made sure that the new colors pass accessibility.

Thanks!  Should it use this same color for the light theme or a different one?

Comment 6

[Brian Grinstead [:bgrins]]

Also, we could change the text color for matched text if that makes it easier for accessibility

Comment 7

[Brian Grinstead [:bgrins]]

Note: to implement changes, we'll want to add a rule for `.CodeMirror .cm-searching { }` around here: https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/light-theme.css#378 and here: https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/dark-theme.css#379

Comment 8

[Helen V. Holmes (:helenvholmes) (:✨)]

(In reply to Brian Grinstead [:bgrins] from comment #5)
> Thanks!  Should it use this same color for the light theme or a different
> one?

I don't think we should use the same colors—I'll attach another mockup with updates for the light theme.

Comment 9

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: dark-theme.png

Comment 10

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: light-theme.png

I ended up tweaking the light theme just a little to pass AA, I think the colors were fine as-is for the most part (unlike the dark theme). I also updated the dark theme a bit, on one of the combos I was getting a large-text AA pass that failed once I set it to Menlo 10pt, which it looks like is what we're using in the editor (so that's fixed now too).

Comment 11

[Brian Grinstead [:bgrins]]

(In reply to Helen V. Holmes (:helenvholmes) (:✨) from comment #10)
> Created attachment 8706918 [details]
> light-theme.png
> 
> I ended up tweaking the light theme just a little to pass AA, I think the
> colors were fine as-is for the most part (unlike the dark theme). I also
> updated the dark theme a bit, on one of the combos I was getting a
> large-text AA pass that failed once I set it to Menlo 10pt, which it looks
> like is what we're using in the editor (so that's fixed now too).

What do you think about changing the text color for selected text along with the background color?  So it could be a very dark (near black) text color with the yellow background.  This would guarantee we always pass the accessibility guidelines even if the syntax highlighting colors change.  And we could possibly use a similar scheme for dark theme too (tweaking the yellow to match the theme if needed).

Comment 12

[Helen V. Holmes (:helenvholmes) (:✨)]

(In reply to Brian Grinstead [:bgrins] from comment #11)
> What do you think about changing the text color for selected text along with
> the background color?  So it could be a very dark (near black) text color
> with the yellow background.  This would guarantee we always pass the
> accessibility guidelines even if the syntax highlighting colors change.  And
> we could possibly use a similar scheme for dark theme too (tweaking the
> yellow to match the theme if needed).

I'm down for changing the text color to something very dark for the light theme (makes sense to me). I wouldn't use the yellow background for the dark theme though (I would recommend doing the same thing just the inverse for the light theme)—I suspect the yellow will look as equally garish (if not more so) than what we're coming from.