Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify text & icon colour tokens? #313

Open
nadonomy opened this issue Feb 6, 2024 · 8 comments
Open

Unify text & icon colour tokens? #313

nadonomy opened this issue Feb 6, 2024 · 8 comments

Comments

@nadonomy
Copy link
Contributor

nadonomy commented Feb 6, 2024

I noticed that we maintain the exact same colour values across 2 sets of semantic tokens in color/text/* & color/icon/*.

Does anyone have a strong rationale for why we should maintain both of these? Why not just have one set (color/fill/* ?) which can be used to style whatever's needed?

@nadonomy nadonomy changed the title Unify text & icon colour tokens Unify text & icon colour tokens? Feb 6, 2024
@americanrefugee
Copy link
Collaborator

They are not 1:1...

Text colour styles include:

  • Primary
  • Secondary
  • Placeholder
  • Disabled

Whereas icon colour styles use:

  • Primary
  • Secondary
  • Tertiary
  • Quaternary
  • Disabled
  • Primary-alpha
  • Secondary-alpha
  • Tertiary-alpha
  • Quaternary-alpha

In application, two use cases for separate semantic tokens come to mind:

  • On a tinted background (such as iOS full-page bottom sheet) the icon needs to be Alpha so that there's sufficient contrast against the tinted background, whereas the text remains solid.

  • Badges use alpha colours so that they remain readable on hover.

Does this help?

@nadonomy
Copy link
Contributor Author

nadonomy commented Feb 6, 2024

@americanrefugee thx for the input!

So for the alpha variants, we could also maintain them, so e.g. we could end up with:

color/fill/primary
color/fill/secondary
color/fill/tertiary
color/fill/quarternary
color/fill/primary-alpha
color/fill/secondary-alpha
color/fill/tertiary-alpha
color/fill/quarternary-alpha

And then in the descriptors we could just note that the alpha variants wouldn't pass a11y if used for text.

Would you have concerns about this?

For badges - can you link to the hover states somewhere? I'm struggling to grok the use in practise.

@americanrefugee
Copy link
Collaborator

Agreed on the above!

Badges themselves do not have a hover state since they aren't interactive. However, they may appear within an interactive item, such a table row. Like this example:

Example-badge-hover item

@americanrefugee
Copy link
Collaborator

Actually, let's discuss badge semantic tokens separately because that would include many more colours (e.x. green, blue) for a very limited use case. What you've proposed above should cover the vast majority of use cases.

@pixlwave
Copy link
Contributor

pixlwave commented Feb 7, 2024

This change sounds positive to me given it felt like in practice, text and icon were being used interchangeably at times. It will be massively breaking given these tokens are used everywhere, but seems worthwhile to me and (at least on iOS) we can use a workaround until we're ready to adopt it properly)

@americanrefugee
Copy link
Collaborator

That's true @pixlwave, there will be lots of breaking changes...

@nadonomy Is it at least possible to merge the old styles into the new ones in Token Studio so that the changes are applied automatically to all components in Figma? Or would we need to apply new color tokens to every single component again??

If we would need to update everything manually, especially on both sides, then I wonder if we should just keep it like it is now??? Also, nobody is confused about the current semantics, which is another argument for NOT changing anything.

@nadonomy
Copy link
Contributor Author

That's true @pixlwave, there will be lots of breaking changes...

@nadonomy Is it at least possible to merge the old styles into the new ones in Token Studio so that the changes are applied automatically to all components in Figma? Or would we need to apply new color tokens to every single component again??

If we would need to update everything manually, especially on both sides, then I wonder if we should just keep it like it is now??? Also, nobody is confused about the current semantics, which is another argument for NOT changing anything.

Notes from our discussion this morning (please add anything I missed!):

  1. Can find/replace styles in Figma documents, so would expect to in the docs in the Compound folder/published libraries
  2. Need to better understand how breaking removing styles is for Android & Web - @robintown @jmartinesp can you input pls?

@robintown
Copy link
Member

Merging these sets of tokens will definitely be a breaking change for Web, but I believe it's one we can manage: The downstream products using Compound don't have to upgrade to the new token sets at the same time, so EW and EC can do this independently. We haven't set up our tooling in those products to detect and flag broken token references, so there is some risk of regressions, but the migration process sounds simple enough that I'm not too concerned.

The name fill seems potentially confusing as I'm used to associating that with background colors, like in spreadsheet cells. Maybe fg as the counterpart to our bg tokens would be clearer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants