Skip to content

fix(Button): only spread additional props onto StyledButton - #2359

Merged
joshblack merged 11 commits into
mainfrom
2351-update-button-spread
Oct 26, 2022
Merged

fix(Button): only spread additional props onto StyledButton#2359
joshblack merged 11 commits into
mainfrom
2351-update-button-spread

Conversation

@joshblack

Copy link
Copy Markdown
Member

Closes#2351

This PR grabs the extra values from props in ButtonBase and spreads them onto StyledButton.

One thing I noticed is that the props for this custom component would not align with ButtonProps and so I created a CustomButtonProps type to help out with this. I'd love feedback on if this would be the best way to support this, I wasn't quite sure what approach to take.

Something else I noticed is that the custom component through as must accept sx so I made sure the type intersects with SxProp.

@joshblack
joshblack requested review from a team and siddharthkpSeptember 21, 2022 16:48
@changeset-bot

changeset-botBot commented Sep 21, 2022

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b982e6a

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

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

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

@github-actions

github-actionsBot commented Sep 21, 2022

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js78.14 KB (+0.01% 🔺)
dist/browser.umd.js78.79 KB (+0.01% 🔺)

@joshblack
joshblack temporarily deployed to github-pages September 21, 2022 16:55 Inactive
@joshblack
joshblack temporarily deployed to github-pages October 7, 2022 16:25 Inactive
@joshblack
joshblack temporarily deployed to github-pages October 10, 2022 16:35 Inactive
@joshblackjoshblack changed the title feat(Button): support as prop, add CustomButtonProps typefix(Button): only spread additional props onto StyledButtonOct 10, 2022
@joshblack
joshblack temporarily deployed to github-pages October 20, 2022 18:31 Inactive

@siddharthkpsiddharthkp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No brainer, ship it!

@joshblack
joshblack enabled auto-merge (squash) October 26, 2022 16:25
@joshblack
joshblack temporarily deployed to github-pages October 26, 2022 16:31 Inactive
@joshblack
joshblack merged commit b05fa74 into mainOct 26, 2022
@joshblack
joshblack deleted the 2351-update-button-spread branch October 26, 2022 16:32
@primer-cssprimer-css mentioned this pull request Oct 26, 2022
Sign up for freeto 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.

Button props spilling to DOM element

2 participants

@joshblack@siddharthkp