Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Button: add 12px padding and expanded content example #79

Merged
merged 1 commit into from
Aug 28, 2019

Conversation

evans
Copy link
Contributor

@evans evans commented Aug 22, 2019

Copy link
Contributor

@mayakoneval mayakoneval left a comment

Choose a reason for hiding this comment

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

100% we will have to remove the extra padding in engine that I / some others added in

@@ -425,6 +425,8 @@ export const Button: ComponentWithAs<Props, "button"> = ({
alignItems: "center",
display: "flex",
justifyContent: "center",
paddingLeft: "12px",
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes need to be applied only when buttons are not icon only. Note that the designs don't have the padding applied for icon-only buttons.

Use the iconOnly variable to conditionally add this padding.

Design

image

This PR

image

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked into this further; unfortunately this is adding padding to a second spot and will compound with the original padding already applied. I'll send an update.

I'll also send another update that will only add styles to one spot so there's no more confusion!

Copy link
Contributor

@justinanastos justinanastos left a comment

Choose a reason for hiding this comment

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

I moved the padding and made it only apply to buttons that aren't icon only. Merging!

@justinanastos justinanastos merged commit 35db29c into master Aug 28, 2019
@justinanastos justinanastos deleted the evans/add-button-padding branch August 28, 2019 12:03
@justinanastos
Copy link
Contributor

🚀 PR was released in v2.1.1 🚀

@justinanastos justinanastos added the released This issue/pull request has been released. label Aug 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants