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

refactor(app,components): borderRadius4 override #14661

Merged
merged 7 commits into from
Mar 18, 2024

Conversation

sfoster1
Copy link
Member

@sfoster1 sfoster1 commented Mar 14, 2024

Border radius 4. Don't storybook because the pipette render is broken. I think this is it for components from figma.

Closes EXEC-331

@sfoster1 sfoster1 requested a review from a team as a code owner March 14, 2024 20:00
@sfoster1 sfoster1 requested review from shlokamin and mjhuff and removed request for a team March 14, 2024 20:00
@mjhuff
Copy link
Contributor

mjhuff commented Mar 14, 2024

Definitely good for us not to hardcode these, thanks for changing that! The ticket is focused on changing borderRadius values not caught by then mass "search and replace" PR that are not currently 4px to 4px (with the shiny new constant).

Figma:
https://www.figma.com/file/bJxzgdG6IUVuN2eHZs8dBI/Design-Variables-Playground?type=design&node-id=209-9667&mode=design&t=gBA432JSuz8c8Jpb-0

For example: The LocationConflictModal has a couple boxes that have borderRadius4 (the grey boxes - you can see this if you click it in figma). We need to either:

  • Add a borderRadius4 if there isn't a border radius.
  • Change the border radius from whatever it was to borderRadius4.

I did a few of these already. I've pushed them up as examples if you'd like to take the rest of BR4!

@mjhuff mjhuff removed the request for review from shlokamin March 18, 2024 16:13
@mjhuff
Copy link
Contributor

mjhuff commented Mar 18, 2024

Alright, looks good now! 🚢

@mjhuff mjhuff merged commit 21213b9 into edge Mar 18, 2024
33 checks passed
@mjhuff mjhuff deleted the exec-331-override-border-radius-4 branch March 18, 2024 17:48
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
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

Successfully merging this pull request may close these issues.

2 participants