-
Notifications
You must be signed in to change notification settings - Fork 616
fix(Button): correct loading alignment #6095
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
Conversation
🦋 Changeset detectedLatest commit: b8796ca 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! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the alignment of the loading spinner inside the Button component by making the spinner container a flexbox.
- Added
display: flex
to the.LoadingSpinner
class.
Comments suppressed due to low confidence (2)
packages/react/src/Button/ButtonBase.module.css:246
- [nitpick] Verify that this class name matches the JSX import (e.g.,
styles.loadingSpinner
). If the component expects camelCase, consider renaming toloadingSpinner
to avoid mismatches.
.LoadingSpinner{
packages/react/src/Button/ButtonBase.module.css:246
- Consider adding a visual regression or Storybook test for the button with a loading spinner to catch any future alignment regressions.
.LoadingSpinner{
size-limit report 📦
|
…ct into fix/loading-button-allignment
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/379073 |
🟢 golden-jobs completed with status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre approving, have a look at the copilot suggestions!
Co-authored-by: Copilot <[email protected]>
👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the alignment of the loading spinner within the Button component by making its container a flexbox. It updates the CSS module, refreshes a test snapshot to reflect new class hashes, and adds a changeset for a patch release.
- Added
.LoadingSpinner
styles to center the spinner in buttons - Updated an AnchoredOverlay snapshot due to updated CSS module class names
- Created a changeset entry for the patch release
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/react/src/Button/ButtonBase.module.css | Added .LoadingSpinner flex alignment rules |
packages/react/src/AnchoredOverlay/snapshots/AnchoredOverlay.test.tsx.snap | Updated class hashes in snapshot |
.changeset/light-pants-listen.md | Added patch changeset for fix(Button): correct loading alignment |
Comments suppressed due to low confidence (1)
packages/react/src/Button/ButtonBase.module.css:247
- Consider adding a unit or visual regression test that renders a Button with
isLoading
enabled and asserts the spinner is centered, ensuring this new alignment is covered by automated tests.
display: flex;
Co-authored-by: Copilot <[email protected]>
👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks! |
👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks! |
👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks! |
Closes https://github.com/github/primer/issues/5199
Changelog
Changed
LoadingSpinner
class in buttonRollout strategy
Testing & Reviewing
Merge checklist