Closed Bug 1342002 Opened 7 years ago Closed 7 years ago

∟of Rule view looks lighter than font color in dark theme

Categories

(DevTools :: Inspector: Rules, defect, P3)

Product:
DevTools
Component:
Inspector: Rules
Version:
54 Branch
Type:
defect

Tracking

(firefox54 fixed, firefox55 fixed)

Status:
VERIFIED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: nachtigall, Assigned: rahulc)

References

Details

Attachments

(2 files, 1 obsolete file)

L---icon-color.png
6.48 KB, image/png
Details
Bug 1342002 - Border color of overriden items in Rule view looks lighter than font color in dark theme.
59 bytes, text/x-review-board-request
gl
: review+
gchang
: approval-mozilla-aurora+
Details 
I very much like the Overridden Indicator of Bug 1321791

One Nitpick:

AR:
In the Dark Theme (have not checked Light or Firebug Theme) the color of ∟ icon is var(--theme-highlight-gray) which is rgb(169, 186, 203). This is more subtle (mathematically) than the font color which is var(--theme-highlight-purple) which is rgb(188, 184, 219). However, the font is anti-aliased where the icon is straight lines. At least to my eyes the ∟ icon looks more white than the font color. The ∟ stands out a bit too much from of the rest. Especially since it already has indentation.

ER:
The ∟ icon should be much more subtle. That is, in the Dark Theme it should not be that white, but even more greyish.
Pinging Patrick. This caught my eyes but of course YMMV :)
Flags: needinfo?(pbrosset)
Flags: needinfo?(rahulch95)
Assignee: nobody → rahulch95
Status: NEW → ASSIGNED
Flags: needinfo?(rahulch95)
Priority: -- → P3
Attached image L---icon-color.png — 
Uh, I attached a screenshot (sorry, should have done immediately, but forgot)

STR:
1. Open the screenshot in a Firefox tab
2. Open Devtools and the Eyedropper
3. As you see anti-aliased font color something like #BCB8DB or #AF8E81, whereas the straight ∟ icon is #E9F4FE

BTW, the ⯈ icon is #888B93. And I think the ∟ maybe be the same grey, or even more greyish (subtle).
Clearing the needinfo flag since the bug was assigned since. I agree with the bug report, the color of the icon is too bright in the dark theme.
Flags: needinfo?(pbrosset)
Comment on attachment 8844645 [details]
Bug 1342002 - Border color of overriden items in Rule view looks lighter than font color in dark theme.

https://reviewboard.mozilla.org/r/117996/#review119928

::: devtools/client/themes/rules.css:7
(Diff revision 1)
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  /* CSS Variables specific to this panel that aren't defined by the themes */
>  .theme-light {
>    --rule-highlight-background-color: #ffee99;

I think I would prefer to see the gray color being using in the light theme. You can define a new variable for the light and dark theme that refers to the color variables. 

I would call it --rule-overridden-item-border-color or something along those lines.
Attachment #8844645 - Flags: review?(gl)
Comment on attachment 8844645 [details]
Bug 1342002 - Border color of overriden items in Rule view looks lighter than font color in dark theme.

https://reviewboard.mozilla.org/r/117996/#review119930

::: commit-message-3d341:1
(Diff revision 1)
> +Bug 1342002 - of Rule view looks lighter than font color in dark theme. r?gl

Commit message needs to be fixed.
Comment on attachment 8844645 [details]
Bug 1342002 - Border color of overriden items in Rule view looks lighter than font color in dark theme.

https://reviewboard.mozilla.org/r/117996/#review119930

> Commit message needs to be fixed.

I tried adding the character "∟" to the commit message in git, however `git mozreview push` had a problem dealing with with it - and `git log` didn't seem to have incorrect characters instead of the "∟" character.
Open to recommendations.
Comment on attachment 8844645 [details]
Bug 1342002 - Border color of overriden items in Rule view looks lighter than font color in dark theme.

https://reviewboard.mozilla.org/r/117996/#review120532

::: devtools/client/themes/rules.css:8
(Diff revision 2)
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  /* CSS Variables specific to this panel that aren't defined by the themes */
>  .theme-light {
>    --rule-highlight-background-color: #ffee99;
> +  --rule-overridden-item-border-color: #667380;;

NIT: double ;

::: devtools/client/themes/rules.css:8
(Diff revision 2)
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  /* CSS Variables specific to this panel that aren't defined by the themes */
>  .theme-light {
>    --rule-highlight-background-color: #ffee99;
> +  --rule-overridden-item-border-color: #667380;;

You can simply do:
--rule-overridden-item-border-color: var(--theme-content-color3)

::: devtools/client/themes/rules.css:13
(Diff revision 2)
> +  --rule-overridden-item-border-color: #667380;;
>  }
>  
>  .theme-dark {
>    --rule-highlight-background-color: #594724;
> +  --rule-overridden-item-border-color: #a9bacb;

Refer to the variable instead of hardcoding the value. If we ever change the theme color variable, we would populate all these changes for free.

::: devtools/client/themes/rules.css:20
(Diff revision 2)
>  
>  .theme-firebug {
>    --rule-highlight-background-color: #ffee99;
>    --rule-property-name: darkgreen;
>    --rule-property-value: darkblue;
> +  --rule-overridden-item-border-color: #8fa1b2;

Same as above.
Attachment #8844645 - Flags: review?(gl)
Attachment #8844645 - Attachment is obsolete: true
Attachment #8844645 - Flags: review?(gl)
Comment on attachment 8846866 [details]
Bug 1342002 - Border color of overriden items in Rule view looks lighter than font color in dark theme.

https://reviewboard.mozilla.org/r/119864/#review121974
Attachment #8846866 - Flags: review?(gl) → review+
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bb938116a71
Border color of overriden items in Rule view looks lighter than font color in dark theme. r=gl
Needinfo'ing myself since this needs to be uplifted
Flags: needinfo?(gl)
https://hg.mozilla.org/mozilla-central/rev/2bb938116a71
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
I have successfully reproduced this issue on firefox nightly according to(2017-02-23)

Fixing bug is verified on Latest Nightly-- Build ID: (20170402030202),User Agent: Mozilla/5.0 (Windows NT 10.0; rv:55.0) Gecko/20100101 Firefox/55.0

Tested OS -- Windows10 32bit

[bugday-20170329]
Comment on attachment 8846866 [details]
Bug 1342002 - Border color of overriden items in Rule view looks lighter than font color in dark theme.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1321791
[User impact if declined]: Inconsistent styles you see in the screenshot attachment.
[Is this code covered by automated tests?]: No, this is just a style change.
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Already verified fix.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No, just a style change.
[Why is the change risky/not risky?]: No.
[String changes made/needed]: No
Flags: needinfo?(gl)
Attachment #8846866 - Flags: approval-mozilla-aurora?
Comment on attachment 8846866 [details]
Bug 1342002 - Border color of overriden items in Rule view looks lighter than font color in dark theme.

Polish an UI issue in dark theme and was verified. Aurora54+.
Attachment #8846866 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I have reproduced this bug with Nightly 54.0a1 (2017-02-23) on Windows 10, 64 bit!

The bug's fix is now verified on Latest Beta 54.0b3

Build ID 	20170427091925
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0

[testday-20170428]
I have reproduced this bug with Nightly 54.0a1 (2017-02-23) (64-bit) on Manjaro Linux 17!

This bug's fix is verified with latest Beta!

Build ID 	20170511200247
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0

[testday-20170512]
 As per Comment 22 & Comment 23, I am marking this bug as verified fixed.
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: