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

Chip - Update indigo theme #1391

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Chip - Update indigo theme #1391

wants to merge 5 commits into from

Conversation

desig9stein
Copy link
Contributor

@desig9stein desig9stein commented Sep 18, 2024

make sure that the hover is working even when the chip is focused.
@andiesm813
Copy link

Everything looks good here too. Thanks @desig9stein

@yradoeva
Copy link

yradoeva commented Sep 24, 2024

There are some things I noticed during validation:

Light & Dark mode

  1. L, M and S size - the width of the chip is with 2 px larger because the border is implemented outside vs. inside in the Design kit.
    In that case left and right paddings should be:
    L - 7px
    M - 5px
    S - 3px
    (screenshot is from M)
image

Light mode
2. Default - Hover - border color should be grays.500 (not grays.400)
image

  1. Selected - disabled: text color should be white 40% (not grays.900 20%)
image
  1. Primary - disabled: text color should be white 40% (not white)
image
  1. Primary + Selected - disabled: text color should be white 40% (not white)
image

Dark mode
6. Default - Hover - border color should be grays.200 (not grays.100)
image

  1. Primary - disabled: text color should be white 20% (not white)
image
  1. Primary + Selected - disabled: text color should be white 20% (not white)
image

@andiesm813
Copy link

@desig9stein i dont know if you fixed it yet, but i dont see the issue nº 2...
image

Some of the opacity issues might be related to the addition of opacity on the disabled component, yes @desig9stein ? So the text color being white for some of the variant chips (info, success, warning and error) is correct according to the handoff. But the primary chip didnt follow these rules, because it looked identical to the selected chip... so i kept the same criteria for the primary chip. But this may be confusing....

The other variants are fine on disabled.... maybe i still need to fix the primary chip, to match the same behavior as the other colored variants?

image

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

Successfully merging this pull request may close these issues.

Indigo: Update Chip Theme
5 participants