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

Button padding change #272

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nathanbrud
Copy link
Collaborator

@nathanbrud nathanbrud commented May 22, 2023

What this PR does

Change the padding for sm/xs buttons based on https://vimean.atlassian.net/browse/DSYS-1

Screenshots & Recordings

image

How it does that

For general padding change the padding values per size.
For sm/xs specifically there are padding adjustments needed because the current logic doesn't cover it so added a map with adjustments to the end result for the requested values.

Testing

Use sm/xs button with / without text and see padding looks ok.
Added tests for padding size based on button size.

image

@nathanbrud nathanbrud marked this pull request as ready for review May 22, 2023 18:41
@nathanbrud nathanbrud requested review from a team as code owners May 22, 2023 18:41
@tylerthegrove
Copy link
Collaborator

I'd like to discuss this approach with @juliewongbandue a little bit more. It would be a larger change, but it might be worth examining why are sizing our icons in such a complex way.

@nathanbrud
Copy link
Collaborator Author

Sure, I tried to do it less intrusive but removing complexity is always good.

If you want to loop me in too would be great.

Anyway that my way of suggesting the solution and if you think we should go another way I can change it 🙂

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