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

VERIFIED FIXED in Firefox 54

Status

()

Firefox
Developer Tools: CSS Rules Inspector
P3
normal
VERIFIED FIXED
3 months ago
15 days ago

People

(Reporter: Jens, Assigned: rahulc)

Tracking

54 Branch
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox54 fixed, firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

3 months ago
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.
(Reporter)

Comment 1

3 months ago
Pinging Patrick. This caught my eyes but of course YMMV :)
Flags: needinfo?(pbrosset)

Updated

3 months ago
Flags: needinfo?(rahulch95)

Updated

3 months ago
Assignee: nobody → rahulch95
Status: NEW → ASSIGNED
Flags: needinfo?(rahulch95)
Priority: -- → P3
(Reporter)

Comment 2

3 months ago
Created attachment 8840842 [details]
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 hidden (mozreview-request)

Comment 5

3 months ago
mozreview-review
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 6

3 months ago
mozreview-review
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 hidden (mozreview-request)
(Assignee)

Comment 8

3 months ago
mozreview-review-reply
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 9

3 months ago
mozreview-review
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)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

3 months ago
Attachment #8844645 - Attachment is obsolete: true
Attachment #8844645 - Flags: review?(gl)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 14

3 months ago
mozreview-review
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+

Comment 15

3 months ago
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
Last Resolved: 3 months 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+

Comment 21

2 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/5fcb7e7b5f2d
status-firefox54: affected → fixed

Comment 22

a month ago
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]

Comment 24

15 days ago
 As per Comment 22 & Comment 23, I am marking this bug as verified fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.