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

fix: add shadow token, update overlay token #9812

Merged
merged 4 commits into from
Oct 7, 2021

Conversation

janhassel
Copy link
Member

Closes #9802

Changelog

New

  • Added new token shadow
    • white/g10/v9: rgba(0, 0, 0, 0.3)
    • g80/g90/g100: rgba(0, 0, 0, 0.8)

Changed

  • Updated overlay/overlay-01 token
    • white/g10/v9: unchanged (see below)
    • g80/g90/g100: rgba(0, 0, 0, 0.65)
  • Updated token snapshots

Testing / Reviewing

  • Verify visual appearance is as expected
  • According to @yumeforever, the current overlay/overlay-01 token in light themes should've been rgba(gray100, 0.7) though it being rgba(gray100, 0.5). Which one is preferred?
  • According to @yumeforever, the current shadow of overflow menus in light themes should've been rgba(0, 0, 0, 0.3) though it was rgba(0, 0, 0, 0.2). I changed it to @yumeforever's suggestion to increase the contrast. Let me know if this works for everyone.

@janhassel janhassel requested review from a team as code owners October 6, 2021 07:38
@netlify
Copy link

netlify bot commented Oct 6, 2021

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: 5895656

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/615f5a2e4351600007448933

😎 Browse the preview: https://deploy-preview-9812--carbon-react-next.netlify.app

@netlify
Copy link

netlify bot commented Oct 6, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 5895656

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/615f5a2ea1dc350008dcc36f

😎 Browse the preview: https://deploy-preview-9812--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Oct 6, 2021

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 5895656

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/615f5a2eef00bb00087c4e57

😎 Browse the preview: https://deploy-preview-9812--carbon-components-react.netlify.app

@yumeforever
Copy link

Testing / Reviewing

  • Verify visual appearance is as expected
  • According to @yumeforever, the current overlay/overlay-01 token in light themes should've been rgba(gray100, 0.7) though it being rgba(gray100, 0.5). Which one is preferred?

My bad, I thought it was 0.7, but I just checked the Color Guidelines and it is 0.5—Let's keep this way

  • According to @yumeforever, the current shadow of overflow menus in light themes should've been rgba(0, 0, 0, 0.3) though it was rgba(0, 0, 0, 0.2). I changed it to @yumeforever's suggestion to increase the contrast. Let me know if this works for everyone.

Here I was using as reference the Style infos on the Overflow menu that states it is 0.3—if it was not I assume it was a bug.
That being said the Dropdown shadow is 0.2—I don't see a reason to have one point stop difference, so I would opt for the one with the more contrast, 0.3

@janhassel
Copy link
Member Author

@yumeforever

Here I was using as reference the Style infos on the Overflow menu that states it is 0.3—if it was not I assume it was a bug.
That being said the Dropdown shadow is 0.2—I don't see a reason to have one point stop difference, so I would opt for the one with the more contrast, 0.3

That's probably a mistake on the documentation then as both components use the same code under the hood to display their box shadow. I agree that 0.3 seems better. 👍

Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay for making these tokens! And yeah I think @yumeforever has got you the right values. Thank you for making this PR!!! And thanks Morgana for doing the design leg work on this one. We really appreciate y'alls contribution.

PS Design team talked about this off-line and gave the thumbs up from our perspective.

Copy link
Member

@sstrubberg sstrubberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @janhassel!

Copy link
Contributor

@jnm2377 jnm2377 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 thanks for adding this!!!!

@kodiakhq kodiakhq bot merged commit 113bcf5 into carbon-design-system:main Oct 7, 2021
@janhassel janhassel deleted the 9802 branch October 11, 2021 07:19
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.

[Feature Request]: Improve contrast on shadows for dark themes (G90 and G100)
5 participants