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

Button component associated colors are inconsistent. #262

Closed
pksjce opened this issue Oct 25, 2021 · 2 comments
Closed

Button component associated colors are inconsistent. #262

pksjce opened this issue Oct 25, 2021 · 2 comments

Comments

@pksjce
Copy link
Contributor

pksjce commented Oct 25, 2021

I noticed a few issues with the colors in the btn section of component themes.

  • All the variants don't have all the colors. For eg - the activeBg is only available for the default button and not for primary etc
  • Many colors are defined with just the hex and not a functional variable or any variable. hoverBg and disabledBg
  • Colors for primary button are not available in the functional colors list at all.
  • No colors defined for invisible button
  • Should the shadow values be more parameterised? For eg - selectedShadow: (theme: any) =>inset 0 1px 0 ${alpha(get('scale.green.9'), 0.2)(theme)}, how do we determine the alpha value?
  • There was also this PR raised in @primer/components to highlight missing colors in danger and primary variants.

With this issue, I wonder if it possible to either

  • Get rid of the btn component colors and directly use functional color variables. OR
  • Make the btn component colors robust and work for all scenarios.

cc - @ashygee, @colebemis

@simurai
Copy link
Contributor

simurai commented Oct 25, 2021

Yeah, the button colors went through many iterations recently:

  1. Component refresh (pre color modes)
  2. Color modes V1
  3. Color modes V2

So it might be a good idea to take a closer look at the button colors to see if they can be improved or aligned to the system more.

With this issue, I wonder if it possible to either

  • Get rid of the btn component colors and directly use functional color variables. OR
  • Make the btn component colors robust and work for all scenarios.

Maybe it needs to be a combination of both. Especially for the btn.primary it was hard to use functional/global variables because these "green" buttons became somewhat iconic and almost part of the GitHub brand. And at the same time they need to be accessible which is harder to do with green. /cc @auareyou

Another tricky part is that for things like hover/active/selected states, the difference should be minimal compared to the default state. And using functional/global variables might not work for these cases because the difference is too big. I think @mperrotti brought up a similar issue when working on the states for ActionList?

So currently color functions like darken() are used to get a new color that is just a tiny bit different. Using alpha() would be another alternative.

So yeah, we could see if any of the functional/global variables would work, but my guess is that we still need a few colors specific to buttons and all their different states.

@simurai
Copy link
Contributor

simurai commented Dec 15, 2022

This came up in primer/view_components#1712. A nested Counter in dark_high_contrast has low contrast:

Screen Shot 2022-12-15 at 16 59 11

Possible options for dark_high_contrast:

  • Either add a new --color-btn-counter-fg variable that is light
  • Or updating --color-btn-counter-bg to be light

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

3 participants