Open Bug 1055311 Opened 10 years ago Updated 2 years ago

Remove remaining hardcoded colors from devtools/client/themes/

Categories

(DevTools :: Shared Components, defect, P4)

defect

Tracking

(Not tracked)

People

(Reporter: bgrins, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

After Bug 947242 and 1014205 land, most of the remaining hardcoded colors will go away.  However, there are a couple of spots that are using either a transparent variation of the colors from the palette or using a non-palette color altogether.

An easy way to find these would probably be to search for the string 'theme-light' or 'theme-dark' in the css files.
Summary: Remove remaining hardcoded colors from themes/shared/devtools/* → Remove remaining hardcoded colors from devtools/client/themes/
Product: Firefox → DevTools
Component: Framework → CSS and Themes

Hello! I am looking to make my first contribution and I think this would be a nice one. Is this issue still available to fix?

Priority: -- → P4

(In reply to helenaenred from comment #1)

Hello! I am looking to make my first contribution and I think this would be a nice one. Is this issue still available to fix?

Hi, Helena, yes!

To expand a bit on what :bgrins is referring to, colours in our CSS files need to be using a variable, instead of a raw hex value rgb(...) or hsl(...)

The colors are mostly defined in devtools/client/themes/variables.css, and there are some in devtools/client/themes/common.css. If an equivalent color already exists, let's use that name. If the color doesn't appear, we need to create a new name. If the color is an alpha version of an existing one, let's add a suffix to the original name. For instance: blue-50 -> blue-50-a30 (for a 30% of blue-50).

We're also deprecating some panels, either removing them or substituting for newer versions, so these stylesheets are probably not worth updating:

  • shadereditor.css
  • canvasdebugger.css
  • webaudiodebugger.css
  • webide.css
  • perf.css

Could you pls confirm you still want to take the bug, so I can assign it to you? Thanks! :)

Flags: needinfo?(helenaenred)

Could you pls confirm you still want to take the bug, so I can assign it to you? Thanks! :)

Hello Benko, thanks for your answer :)

Yes, I will take it but let me try first to set up the environment and run the project locally.

Flags: needinfo?(helenaenred)

Finally working on it :)

Assignee: nobody → helenaenred

If the color doesn't appear, we need to create a new name. If the color is an alpha version of an existing one, let's add a suffix to the original name. For instance: blue-50 -> blue-50-a30 (for a 30% of blue-50).

We actually went the other way round recently (after a discussion with :ntim). We had 4 --grey-xx-axx (Photon Grey colors at some arbitrary opacities) defined in variables.css but only used in the animation inspector. We removed them from variables.css and hardcoded them as specific variables in animations.css instead.

My concern is that if every tool that happens to tweak the opacity of a Photon palette color defines that variant in variables.css, we're going to have dozens of such variations. :/

Instead I would recommend using fully-opaque Photon colors that give a similar result, rather than partial opacity variants. For example, using "Grey 60" rather than "Grey 90 at 70% opacity".

We're also trying to standardize a few colors for common semantic uses and define variables for them:

We definitely need to consolidate the colors we use and eliminate duplicates. I'm OK with using opaque colors instead of transparent variants, however I'm wary of making up our own color numbers, and even if they are exactly equivalent to alpha values (assuming the same background), it's hard to maintain if what we are seeing in the Photon guidelines (for those components with alpha values) differ from what we're using. So if we go through that route, we'd need to make sure Victoria is OK with this, and for those components defined in Photon, let's update the guidelines with opaque values.

Assignee: helenaenred → nobody

Hi everyone,

I see that this is in a middle of an agreement, but I could be available to help once it's settled.

Thanks!

Component: CSS and Themes → Shared Components
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.