Skip to content

feat(ButtonGroup): remove support for sx - #6835

Merged
francinelucca merged 5 commits into
mainfrom
feat/remove-support-for-sx-from-buttongroup
Sep 22, 2025
Merged

feat(ButtonGroup): remove support for sx#6835
francinelucca merged 5 commits into
mainfrom
feat/remove-support-for-sx-from-buttongroup

Conversation

@joshblack

@joshblackjoshblack commented Sep 10, 2025

Copy link
Copy Markdown
Member

Closes#6761

Changelog

New

Changed

Removed

  • Remove support for sx from ButtonGroup

Rollout strategy

  • Major release; if selected, include a written rollout or migration plan

This component has no sx usage in github-ui and can be safely merged in ✅

@joshblack
joshblack requested a review from a team as a code ownerSeptember 10, 2025 22:04
@changeset-bot

changeset-botBot commented Sep 10, 2025•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 844f8b5

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

This PR includes changesets to release 2 packages
NameType
@primer/reactMajor
@primer/styled-reactMajor

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

@joshblackjoshblack changed the title Feat/remove support for sx from buttongroupfeat(ButtonGroup): remove support for sxSep 10, 2025
@github-actionsgithub-actionsBot added the staff Author is a staff member label Sep 10, 2025
CopilotAI reviewed Sep 10, 2025
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 removes the deprecated sx prop from the ButtonGroup component as part of the ongoing migration away from the styled-components system to CSS modules. The change includes updating the component's TypeScript interface, replacing the internal BoxWithFallback wrapper with a polymorphic base component, and migrating existing sx usage in stories to standard CSS properties.

Key changes:

  • Removes sx prop support from ButtonGroup component interface
  • Replaces BoxWithFallback with polymorphic component pattern using as prop
  • Updates development stories to use inline styles instead of sx prop

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

FileDescription
packages/react/src/ButtonGroup/ButtonGroup.tsxRemoves sx prop, replaces BoxWithFallback with polymorphic component pattern
packages/react/src/ButtonGroup/ButtonGroup.docs.jsonRemoves deprecated sx prop from component documentation
packages/react/src/ButtonGroup/ButtonGroup.dev.stories.tsxMigrates sx usage to inline styles and removes SxProp story
.changeset/light-colts-burn.mdAdds changeset entry for major version bump

Comment threadpackages/react/src/ButtonGroup/ButtonGroup.tsx
Comment on lines +15 to +16
const ButtonGroup = React.forwardRef(function ButtonGroup(
{as: BaseComponent = 'div', children, className, role, ...rest},

CopilotAISep 10, 2025

Copy link

Choose a reason for hiding this comment

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

The as prop is destructured but not included in the ButtonGroupProps type definition. This creates a mismatch between the component's actual interface and its declared props type, which could confuse consumers and break TypeScript IntelliSense.

Copilot uses AI. Check for mistakes.
@github-actionsgithub-actionsBot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Sep 10, 2025
@github-actions

github-actionsBot commented Sep 10, 2025

Copy link
Copy Markdown
Contributor

👋 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

github-actionsBot commented Sep 10, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
packages/react/dist/browser.esm.js88.55 KB (-0.11% 🔽)
packages/react/dist/browser.umd.js88.76 KB (+0.04% 🔺)

@github-actions
github-actionsBottemporarily deployed to storybook-preview-6835 September 10, 2025 22:12 Inactive
@joshblack
joshblack requested a review from a team as a code ownerSeptember 10, 2025 22:15
@github-actions
github-actionsBottemporarily deployed to storybook-preview-6835 September 10, 2025 22:25 Inactive
@github-actionsgithub-actionsBot added integration-tests: failing Changes in this PR cause breaking changes in gh/gh and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Sep 16, 2025
@github-actions
github-actionsBottemporarily deployed to storybook-preview-6835 September 16, 2025 22:47 Inactive
@primer-integration

primer-integrationBot commented Sep 16, 2025

Copy link
Copy Markdown

👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/2668

@primer-integration

primer-integrationBot commented Sep 16, 2025

Copy link
Copy Markdown

🟢 ci completed with status success.

@github-actionsgithub-actionsBot added integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh and removed integration-tests: failing Changes in this PR cause breaking changes in gh/gh labels Sep 16, 2025
@github-actions

github-actionsBot commented Sep 22, 2025

Copy link
Copy Markdown
Contributor

👋 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!

@github-actionsgithub-actionsBot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Sep 22, 2025
@francinelucca
francinelucca added this pull request to the merge queueSep 22, 2025
@github-actions
github-actionsBottemporarily deployed to storybook-preview-6835 September 22, 2025 18:30 Inactive
Merged via the queue into main with commit 961c1c4Sep 22, 2025
42 checks passed
@francinelucca
francinelucca deleted the feat/remove-support-for-sx-from-buttongroup branch September 22, 2025 18:38
@primerprimerBot mentioned this pull request Sep 22, 2025
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@francineluccafrancineluccafrancinelucca approved these changes
@jonrohanjonrohanAwaiting requested review from jonrohanjonrohan is a code owner automatically assigned from primer/engineer-reviewers
@mperrottimperrottiAwaiting requested review from mperrottimperrotti was automatically assigned from primer/design-reviewers

Assignees

No one assigned

Labels

integration-tests: passingChanges in this PR do NOT cause breaking changes in gh/ghintegration-tests: recommendedThis change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpmstaffAuthor is a staff member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove support for sx from the ButtonGroup component

3 participants

@joshblack@francinelucca