Open Bug 1898612 Opened 9 months ago Updated 9 months ago

Investigate adding a --icon-color-deemphasized token

Categories

(Toolkit :: Themes, task)

task

Tracking

()

People

(Reporter: hjones, Unassigned)

References

(Blocks 1 open bug)

Details

We should probably revisit the value of our --icon-color token. In most cases we probably want it to be currentColor or --text-color to match any adjacent text. The current value has also bee problematic in at least one case where we're using color-mix with currentColor as it can lead to unexpected changes in background color (see: Bug 1896140).

It would make sense to:

  • change --icon-color to currentColor or --text-color
  • add a --icon-color-deemphasized token with the current --icon-color value for cases where we want standalone icons in a slightly lighter grey (in light themes, at least)
  • clarify/come up with some rules about which icon color should be used for different cases

Hey Hanna, we discussed this during triage and have a few questions:

  • change --icon-color to currentColor or --text-color

Would we need this for --icon-color to be used as the fill value? Because for color, it seems like currentColor should be a no-op?

  • add a --icon-color-deemphasized token with the current --icon-color value for cases where we want standalone icons in a slightly lighter grey (in light themes, at least)

Would this be different from --text-color-deemphasized?

Flags: needinfo?(hjones)

Conceptually and ideally I think most icons are given the same treatment as we use for text in that same context. If it was practical to include them as glyphs in a font and make them text rather than images, we would do that. So I think I'm missing the use case that means we need these --icon-color variables? It only matter because the text and icon variables can diverge and we could end up with 2x the bugs.

(In reply to Dão Gottwald [:dao] from comment #1)

  • change --icon-color to currentColor or --text-color

Would we need this for --icon-color to be used as the fill value? Because for color, it seems like currentColor should be a no-op?

I was thinking it would be used as a fill value, but searching through the existing use cases it looks like it also gets used as a color value so --icon-color: var(--text-color) is likely the way we want to go.

  • add a --icon-color-deemphasized token with the current --icon-color value for cases where we want standalone icons in a slightly lighter grey (in light themes, at least)

Would this be different from --text-color-deemphasized?

Yes it would likely be the current value we have for --icon-color so light-dark(var(--color-gray-70), var(--color-gray-05));.

I could have been clearer in the bug description, but we probably need to answer a couple of questions before we start working on this, specifically:

  • do we want to always have the icon color match the text color in cases where they are used side by side? (from the comments above it sounds like the answer to this is "yes")
  • do we want to support a different, slightly lighter color for icon buttons (for example the gear icon in the top corner of about:home)?

If the answer to the second question is "no" and we always want the icon color to match the text color, then we can probably just peg the --icon-color variable to the --text-color variable and be done with it without adding a new "deemphasized" variant.

(In reply to Sam Foster [:sfoster] (he/him) from comment #2)

Conceptually and ideally I think most icons are given the same treatment as we use for text in that same context. If it was practical to include them as glyphs in a font and make them text rather than images, we would do that. So I think I'm missing the use case that means we need these --icon-color variables? It only matter because the text and icon variables can diverge and we could end up with 2x the bugs.

Just to address this briefly, I think the main point of continuing to have an --icon-color variable is to be able to use semantic CSS variables. We already have a whole host of different --icon-color-* variables for cases where we use color to convey a specific meaning, so it also makes sense to have a kind of "base" --icon-color variable. If we tie it to the --text-color variable it's arguably superfluous, but also the chance of the two diverging seems pretty minimal.

Flags: needinfo?(hjones)
You need to log in before you can comment on or make changes to this bug.