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

Border color variables: discrepancies between SASS and internal component variables #64649

Open
ciampo opened this issue Aug 20, 2024 · 3 comments
Labels
Needs Design Feedback Needs general design feedback.

Comments

@ciampo
Copy link
Contributor

ciampo commented Aug 20, 2024

I noticed that @wordpress/components' internal border color variables are not aligned with the equivalent SASS variables — we should probably align them

In particular:

  • SASS variables suggest a lighter border color (gray 300), while internal component variables suggest a darker one (gray 600)
  • internal component variables also have modifiers (hover, focus, disabled), while SASS variables don't
  • on the other hand, SASS variables also suggest that some border could be even lighter (gray 200), this is not reflected in the internal component variables

By looking at how the internal component variables are used, it seems that those borders are general applied to control components, which therefore may have different "border specs" from the rest of the UI.

Regardless, I think that we should:

  • agree on a common spec (border colors, variant states, controls vs general UI)
  • apply the spec everywhere consistently
@ciampo ciampo added the Needs Design Feedback Needs general design feedback. label Aug 20, 2024
@ciampo
Copy link
Contributor Author

ciampo commented Aug 20, 2024

@WordPress/gutenberg-components @WordPress/gutenberg-design

@jameskoster
Copy link
Contributor

Ideally this would be governed by the color scale with semantic tokens. Similar to Radix Colors.

In the mean time I agree it would be helpful if the in-code guidelines were aligned.

  • Gray 300 is used for decorative borders in the editor.
  • Gray 100 / 200 / 300 is used for decorative borders in admin pages such as data views.
  • Gray 600 is for interactive UI components (inputs) as it meets contrast requirements against white.

If it's trivial to implement I think it would be beneficial add border styles for other input states; hover could be gray 700, disabled could be gray 400. If not it may be better to wait for the aforementioned scale?

@ciampo
Copy link
Contributor Author

ciampo commented Aug 21, 2024

Thank you, @jameskoster !

I don't think we're in a rush to implement it right now in the shared components variables, especially since you confirmed that the current implementation is more or less already hearing to those specs (even without proper variables).

But it's definitely something that we should include in the theme work — and we're hopefully going to be able to rely on the design tokens exposed by the theme everywhere, making the variables that I shared previously unnecessary 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

No branches or pull requests

2 participants