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

Change background color to white #890

Merged
merged 10 commits into from
Jun 4, 2021
Merged

Conversation

connor-baer
Copy link
Member

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

  • Updated the design tokens with the new background and overlay colors
  • Replaced the shadow of the Card component with a border and deprecated the shadow prop
  • Increased the border radius of the Card and Modal components

Definition of done

  • Development completed
  • Reviewers assigned
  • Unit and integration tests
  • Meets minimum browser support
  • Meets accessibility requirements

@connor-baer connor-baer added 🎨 design Requires input from designers ♿ accessibility Improves usability labels Apr 14, 2021
@connor-baer connor-baer added this to the v2.x milestone Apr 14, 2021
@changeset-bot
Copy link

changeset-bot bot commented Apr 14, 2021

🦋 Changeset detected

Latest commit: 5b8ec32

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@sumup/design-tokens Minor
@sumup/circuit-ui Major

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

@vercel
Copy link

vercel bot commented Apr 14, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sumup/oss-circuit-ui/4Bbrh9KbyuGkonEciGWAoiFf63D7
✅ Preview: https://oss-circuit-ui-git-feature-white-background-sumup.vercel.app

@codecov
Copy link

codecov bot commented Apr 14, 2021

Codecov Report

Merging #890 (5b8ec32) into main (35ce003) will increase coverage by 0.00%.
The diff coverage is 94.44%.

Impacted file tree graph

@@           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           
Impacted Files Coverage Δ
...uit-ui/components/CardList/components/Item/Item.js 85.00% <ø> (ø)
packages/design-tokens/themes/light.ts 0.00% <ø> (ø)
packages/circuit-ui/components/Card/Card.tsx 87.50% <80.00%> (-0.74%) ⬇️
...ackages/circuit-ui/components/CardList/CardList.js 100.00% <100.00%> (ø)
...nts/Modal/components/ModalWrapper/ModalWrapper.tsx 100.00% <100.00%> (ø)
...mponents/NotificationBanner/NotificationBanner.tsx 100.00% <100.00%> (ø)
...i/components/NotificationList/NotificationList.tsx 100.00% <100.00%> (ø)
packages/circuit-ui/components/Table/Table.tsx 94.56% <100.00%> (-0.06%) ⬇️
packages/circuit-ui/styles/style-mixins.ts 98.88% <100.00%> (+0.01%) ⬆️

Copy link
Contributor

@amelako amelako left a comment

Choose a reason for hiding this comment

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

🤍⬜️

@connor-baer connor-baer added the ⛔ do not merge There is a blocker label Apr 16, 2021
@connor-baer connor-baer removed the ⛔ do not merge There is a blocker label Jun 3, 2021
packages/circuit-ui/components/Card/Card.docs.mdx Outdated Show resolved Hide resolved
packages/circuit-ui/components/Card/Card.docs.mdx Outdated Show resolved Hide resolved
'@sumup/design-tokens': minor
---

Updated the body background color to white and darkened the overlay color.
Copy link
Contributor

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?

Copy link
Contributor

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

Copy link
Member Author

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

I will! 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, I already did 🤦

Screenshot 2021-06-04 at 11 23 34

Do you want to do it as part of this PR or should I make a separate one? Sorry about this

Copy link
Contributor

@robinmetral robinmetral Jun 4, 2021

Choose a reason for hiding this comment

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

Spec:

Screenshot 2021-06-04 at 11 31 37

What I suggest then is that I'll make a separate PR to next removing the old values (6px for example) and updating any component using it. I'll double check the naming with design and write a codemod to automate this when migrating to v3.

Copy link
Member Author

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 👍🏻

packages/circuit-ui/components/Card/Card.tsx Show resolved Hide resolved
packages/circuit-ui/components/CardList/CardList.js Outdated Show resolved Hide resolved
packages/circuit-ui/styles/style-mixins.ts Show resolved Hide resolved
Co-authored-by: Robin Métral <robin.metral@sumup.com>
@connor-baer connor-baer merged commit e6c3936 into main Jun 4, 2021
@connor-baer connor-baer deleted the feature/white-background branch June 4, 2021 09:39
@github-actions github-actions bot mentioned this pull request Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
♿ accessibility Improves usability 🗂 circuit-ui 🎨 design Requires input from designers 🗂 design-tokens
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants