966640 - The right margin of the side menu widget should match the new splitter color on the dark theme

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Victor Porof [:vporof][:vp]]

#222426 is not #000

Comment 1

[Victor Porof [:vporof][:vp]]

Attachment: widget-colors2.patch

Comment 2

[Brian Grinstead [:bgrins]]

Comment on attachment 8369080 [details] [diff] [review]
widget-colors2.patch

Review of attachment 8369080 [details] [diff] [review]:
-----------------------------------------------------------------

The splitter color change looks good, but I'm not sure if there is a reason why the selectionTextColor variable has been removed

::: browser/themes/shared/devtools/widgets.inc.css
@@ +418,5 @@
>  }
>  
>  .theme-dark .side-menu-widget-item.selected {
>    background-color: #1d4f73; /* Select Highlight Blue */
> +  color: #f5f7fa; /* Light foreground text */

Is there a reason why this variable has been removed and replaced with the color in 4 places here?

Comment 3

[Victor Porof [:vporof][:vp]]

(In reply to Brian Grinstead [:bgrins] from comment #2)
> Comment on attachment 8369080 [details] [diff] [review]

It's inconsistently used in that particular widget, at least. This "light foreground text" color is used both as @smw_selectionTextColor@ and as #f5f7fa. We should just pick one. Since we're using standard colors now from the wiki, I thought I'd remove it and follow what every other css file does.

I can do this in a different bug if you want, or in a different way.

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to Victor Porof [:vp] from comment #3)
> (In reply to Brian Grinstead [:bgrins] from comment #2)
> > Comment on attachment 8369080 [details] [diff] [review]
> 
> It's inconsistently used in that particular widget, at least. This "light
> foreground text" color is used both as @smw_selectionTextColor@ and as
> #f5f7fa. We should just pick one. Since we're using standard colors now from
> the wiki, I thought I'd remove it and follow what every other css file does.
> 
> I can do this in a different bug if you want, or in a different way.

That's fine.  We are going to have to do a big find/replace at some point anyway when we switch to CSS variables or switch to new theme colors, so I don't really mind having duplicated colors around as long as they are used consistently

Comment 5

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/integration/fx-team/rev/fb7a29f77890

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/fb7a29f77890