Closed
Bug 1152281
Opened 10 years ago
Closed 10 years ago
Playback rate in animations inspector panel is unreadable in dark theme
Categories
(DevTools :: Inspector, defect)
DevTools
Inspector
Tracking
(firefox40 fixed)
RESOLVED
FIXED
Firefox 40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: ntim, Assigned: chiragbhatia2006, Mentored)
References
Details
(Whiteboard: [good first bug][lang=css])
Attachments
(5 files)
No description provided.
Comment 1•10 years ago
|
||
Comment 2•10 years ago
|
||
Updated•10 years ago
|
Mentor: pbrosset
Whiteboard: [good first bug][lang=css]
Assignee | ||
Comment 3•10 years ago
|
||
Hi Patrick,
I would like to work on this bug. But I'm unable to reproduce this (screenshot attached). Not sure if I'm looking in the wrong place or something. I'm using the mozilla-central branch on Linux 64-bit btw.
Flags: needinfo?(pbrosset)
Comment 4•10 years ago
|
||
(In reply to Chirag Bhatia from comment #3)
> Created attachment 8591764 [details]
> animation.jpg
>
> Hi Patrick,
>
> I would like to work on this bug. But I'm unable to reproduce this
> (screenshot attached). Not sure if I'm looking in the wrong place or
> something. I'm using the mozilla-central branch on Linux 64-bit btw.
The attached screenshot doesn't have the playback rate selector at all, so you're probably testing with a build that was done before this new feature landed on m-c.
For info, the feature landed with bug 1144615, 4th of April in m-c.
Also, devtools developments are usually done off the fx-team branch, so make sure you clone and build from there: https://wiki.mozilla.org/DevTools/GetInvolved#Hacking
Flags: needinfo?(pbrosset)
Assignee | ||
Comment 5•10 years ago
|
||
Ok, I've got the fx-team branch and I was able to reproduce the bug. Can you please tell me where to look for to make the changes necessary to fix this bug?
Flags: needinfo?(pbrosset)
Comment 6•10 years ago
|
||
The <select> that is used for the playback rate drop-down is created here:
http://mxr.mozilla.org/mozilla-central/source/browser/devtools/animationinspector/animation-panel.js#776
And the <option> elements inside are created right after, here:
http://mxr.mozilla.org/mozilla-central/source/browser/devtools/animationinspector/animation-panel.js#785
The CSS file for these elements is here:
http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/animationinspector.css
In particular, the CSS for the <select> is here:
http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/animationinspector.css#178
So I guess we just need an extra CSS rule that, if the current theme is dark-theme, sets the color of the text in the <option> elements correctly.
There's a class you can use to prefix your rule to ensure it only applies when the dark theme is ON: .theme-dark
Flags: needinfo?(pbrosset)
Reporter | ||
Comment 7•10 years ago
|
||
(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #6)
> The <select> that is used for the playback rate drop-down is created here:
> http://mxr.mozilla.org/mozilla-central/source/browser/devtools/
> animationinspector/animation-panel.js#776
> And the <option> elements inside are created right after, here:
> http://mxr.mozilla.org/mozilla-central/source/browser/devtools/
> animationinspector/animation-panel.js#785
> The CSS file for these elements is here:
> http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/
> animationinspector.css
> In particular, the CSS for the <select> is here:
> http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/
> animationinspector.css#178
>
> So I guess we just need an extra CSS rule that, if the current theme is
> dark-theme, sets the color of the text in the <option> elements correctly.
> There's a class you can use to prefix your rule to ensure it only applies
> when the dark theme is ON: .theme-dark
Or better yet, you can set the color to inherit, or to var(--theme-body-color)
Assignee | ||
Comment 8•10 years ago
|
||
Thank you for the helpful pointers, Patrick and Tim. I've attached a screenshot that shows the changes.
Let me know if that is the expected behavior or you feel any more change is required. If this is correct, you can assign me the bug and I'll upload the patch :)
Flags: needinfo?(pbrosset)
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → chiragbhatia2006
Status: NEW → ASSIGNED
Comment 9•10 years ago
|
||
Thanks for working on this. Bug assigned thanks to Tim.
Please do attach the patch. I'm just wondering, why is the font-size bigger in the select than in the rest of the UI?
Flags: needinfo?(pbrosset)
Assignee | ||
Comment 10•10 years ago
|
||
Here's the patch. Let me know if you need me to approach the code fix in any other way. I've not changed the font-size so I'm not sure why it looks slightly bigger than the rest of the text. We could fix that as a separate bug or include it in this one?
Attachment #8593458 -
Flags: review?(pbrosset)
Comment 11•10 years ago
|
||
Comment on attachment 8593458 [details] [diff] [review]
1152281.patch
Review of attachment 8593458 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me. Thanks for the fix!
No need for a try build for this simple css fix. Pushed to fx-team directly:
https://hg.mozilla.org/integration/fx-team/rev/5cc6a0a94086
Attachment #8593458 -
Flags: review?(pbrosset) → review+
Comment 12•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•