Closed Bug 1460209 Opened 2 years ago Closed 2 years ago

Polish dark theme for the application panel


(DevTools :: Application Panel, enhancement)

Not set


(firefox62 fixed)

Firefox 62
Tracking Status
firefox62 --- fixed


(Reporter: ladybenko, Assigned: jdescottes)




(1 file)

At the moment, the dark theme isn't as readable (i.e. links, no workers screen, etc.) as the light theme for the application panel. We should fix this.
Current patch fixes the readability of links. Couldn't find a clear pattern for disabled links in devtools, but reduced opacity seemed to be used in some places (style editor, memory panel).

Temporarily re-added the disabled class on the debug link while we wait on Bug 1464354.
Assignee: nobody → jdescottes
Depends on: 1464354
Victoria: what is your opinion on disabled links? 

We used to grey them out, but we don't have a good theme variable for this. Here I used:
- --theme-highlight-blue (--blue-55 in light theme, #75BFFF in dark theme) when enabled 
- opacity: 0.5 when disabled
Flags: needinfo?(victoria)
Comment on attachment 8980530 [details]
Bug 1460209 - Use theme colors for links in application panel;

Thanks, the new blue is definitely more readable! I just have a minor comment about some class, and it's simple enough that I don't think it needs further review

::: devtools/client/application/src/components/Worker.js:127
(Diff revision 1)
>        },
>          Strings.GetStringFromName("unregister"))
>        : null;
> -    const debugLinkDisabled = this.isRunning() ? "" : "worker__debug-link--disabled";
> +    const debugLinkDisabled = this.isRunning() ? "" :
> +                                                 "worker__debug-link--disabled disabled";

We shouldn't need two classes here to set a link as disabled.

`.worker__debug-link--disabled` doesn't have any style rules associated, so I'd suggest to use just `.disabled` here.

The rationale is that we might have other disabled links in then panel, and that's why we have the `.disabled` class for links. If we had a preprocessor we could just have the `.disabled` styles as a mixin an import it in every component that needs disabled links –and thus reuse code– but it's not the case here.
Attachment #8980530 - Flags: review?(balbeza) → review+
Duplicate of this bug: 1464354
Pushed by
Use theme colors for links in application panel;r=ladybenko
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Hi Julian, thanks for the screenshots. This disabled link styling looks great! Very similar to the Photon guidelines for disabled buttons
Flags: needinfo?(victoria)
Thanks for the feedback and for linking to the photon styleguide, I should check it more regularly :)
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.