bugfix(Spinner): Spinner component size get's large when passing sx and size props #5236
Conversation
🦋 Changeset detectedLatest commit: 293ca50 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
github-actions
bot
added
the
integration-tests: recommended
size-limit report 📦
|
jonrohan
changed the title
|
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/350104 |
Co-authored-by: Josh Black <joshblack@github.com>
Co-authored-by: Josh Black <joshblack@github.com>
| } | ||
|
|
||
| return <Spinner className={classes.SpinnerAnimation} {...props} /> | ||
| return <Spinner className={clsx(className, classes.SpinnerAnimation)} {...props} /> |
There was a problem hiding this comment.
Not related to the bug, but was pointed out that className was overwriting the CSS modules classname when being passed in, so I added this
|
🟢 golden-jobs completed with status |
Closes https://github.com/github/primer/issues/4391
This fixes an issue with the Spinner and CSS modules we discovered this morning. The problem is when
sx=andsize=is passed to the Spinner component, we render<Box as={Spinner} {...props} />but Box accepts asize=prop as well asSpinnersoBoxends up creating some styles thinking that it's meant to render at different breakpoints.To fix I figured we need to not pass
sizeto Box. So to fix it, I ended up removingBoxand using astyled.divcomponent with nothing but${sx}inside to render any sx props passed along.This fixed the issue. We created a dev story to recreate the issue.
Changelog
New
Changed
Spinner component size get's large when passing sx and size props
Removed
Rollout strategy
Testing & Reviewing
Merge checklist