-
Notifications
You must be signed in to change notification settings - Fork 129
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
Change background color to white #890
Conversation
🦋 Changeset detectedLatest commit: 5b8ec32 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/sumup/oss-circuit-ui/4Bbrh9KbyuGkonEciGWAoiFf63D7 |
Codecov Report
@@ Coverage Diff @@
## main #890 +/- ##
=======================================
Coverage 91.99% 91.99%
=======================================
Files 163 163
Lines 3009 3011 +2
Branches 784 783 -1
=======================================
+ Hits 2768 2770 +2
Misses 212 212
Partials 29 29
|
858289f
to
788cd5a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤍⬜️
788cd5a
to
6359f8a
Compare
'@sumup/design-tokens': minor | ||
--- | ||
|
||
Updated the body background color to white and darkened the overlay color. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also mention the new borderRadius?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah it's not there, do we want to add it? Then I can make a PR to next
that replaces all hardcoded values of 12px
and 16px
by the tokens
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the commit that added the border-radius to the design tokens since it would have conflicted with your change. Can you talk to Giao-Chung and figure out which value(s) to keep, please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will! 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make a separate PR against the next
branch as you suggested 👍🏻
Co-authored-by: Robin Métral <[email protected]>
Addresses TL-467.
Purpose
The brand design team requested to change the global background color to white. This required some further tweaks to the components to ensure an adequate color contrast.
👀 View the design in Figma
Approach and changes
shadow
propDefinition of done