Skip to content

Conversation

pksjce
Copy link
Contributor

@pksjce pksjce commented Sep 24, 2025

Closes #6760

Current sx usage:

Button: 107
IconButton: 32

sx usages are only in repos other than github-ui; those usages have been added to the tracking issue

Changelog

Removed

Remove support for sx from the Button, IconButton components, and associated stories, docs, and tests

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

@Copilot Copilot AI review requested due to automatic review settings September 24, 2025 13:10
@pksjce pksjce requested a review from a team as a code owner September 24, 2025 13:10
@pksjce pksjce requested a review from hectahertz September 24, 2025 13:10
Copy link

changeset-bot bot commented Sep 24, 2025

🦋 Changeset detected

Latest commit: aefc40d

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

This PR includes changesets to release 2 packages
Name Type
@primer/react Major
@primer/styled-react Major

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

Copy link
Contributor

@Copilot Copilot AI left a 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 adapts the Button component to the styled-react package and removes the sx prop implementation from the main @primer/react package. The changes introduce custom styled-react implementations for Button, IconButton, and ActionMenu components that support the SxProp interface for backward compatibility.

Key changes:

  • Creates new styled-react wrapper components for Button, ActionMenu, and IconButton
  • Moves CSSCustomProperties type to a shared location in sx.ts
  • Updates package exports to use the new styled-react implementations

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/styled-react/src/sx.ts Adds CSSCustomProperties type export for shared use
packages/styled-react/src/index.tsx Updates exports to use new styled-react components instead of direct @primer/react exports
packages/styled-react/src/components/PageHeader.tsx Updates import to use shared CSSCustomProperties type
packages/styled-react/src/components/IconButton.tsx Creates new styled-react wrapper for IconButton with sx prop support
packages/styled-react/src/components/Button.tsx Creates new styled-react wrapper for Button with custom sx prop handling
packages/styled-react/src/components/ActionMenu.tsx Creates new styled-react wrapper for ActionMenu with sx prop support

@github-actions github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Sep 24, 2025
Copy link
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-actions bot requested a deployment to storybook-preview-6904 September 24, 2025 23:57 Abandoned
@github-actions github-actions bot requested a deployment to storybook-preview-6904 September 25, 2025 00:22 Abandoned
@github-actions github-actions bot temporarily deployed to storybook-preview-6904 September 25, 2025 00:30 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-6904 October 1, 2025 21:31 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-6904 October 1, 2025 22:25 Inactive
@github-actions github-actions bot requested a deployment to storybook-preview-6904 October 1, 2025 22:30 Abandoned
@github-actions github-actions bot temporarily deployed to storybook-preview-6904 October 1, 2025 22:38 Inactive
@@ -0,0 +1,35 @@
import {
Copy link
Member

Choose a reason for hiding this comment

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

borrowed from #6925 to get tests passing, I expect to merge that PR before this one so reviewers can ignore this since it'll be looked at in the context of the TextInput PR

@primer-integration
Copy link

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

@github-actions github-actions bot 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 Oct 1, 2025
@primer-integration
Copy link

🟢 ci completed with status success.

@github-actions github-actions bot 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 Oct 2, 2025
@francinelucca
Copy link
Member

Comment on lines -55 to -59
test('ActionMenu.Button supports `sx` prop', () => {
const {container} = render(<ActionMenu.Button sx={{background: 'red'}}>test</ActionMenu.Button>)
expect(window.getComputedStyle(container.firstElementChild!).backgroundColor).toBe('rgb(255, 0, 0)')
})

Copy link
Member

@francinelucca francinelucca Oct 2, 2025

Choose a reason for hiding this comment

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

No reason to support sx in ActionMenu.Button since the ActionMenu uses child type comparison that would be incompatible with an sx-wrapped component, making it essentially a no-op.
Instead, opting to remove any sx usage that we have right now on ActionMenu.Button.
I will make a note of this in v38 release notes.

Copy link
Contributor

@liuliu-dev liuliu-dev left a comment

Choose a reason for hiding this comment

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

🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove support for sx from the Button component
4 participants