-
Notifications
You must be signed in to change notification settings - Fork 646
Remove sx prop support from ToggleSwitch
#6627
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: d073e36 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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! |
size-limit report 📦
|
|
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/397774 |
|
🔴 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.
Pull Request Overview
Removes sx prop support from the ToggleSwitch component by migrating from Box-based layout to CSS modules and data attributes for styling. This is part of the broader effort to move away from styled-components and sx prop usage throughout the design system.
- Replaced
Boxwrapper components with native HTML elements styled via CSS modules - Removed
sxprop support fromToggleSwitchcomponent interface - Added backward compatibility by providing an
sx-enabled wrapper in the@primer/styled-reactpackage
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/styled-react/src/index.ts | Adds ToggleSwitch with sx support using createStyledComponent wrapper |
| packages/styled-react/src/tests/snapshots/exports.test.ts.snap | Updates test snapshot to reflect ToggleSwitch export order change |
| packages/react/src/ToggleSwitch/ToggleSwitchStoryWrapper.tsx | Converts Box-based layout to CSS module styling |
| packages/react/src/ToggleSwitch/ToggleSwitchStoryWrapper.module.css | Defines CSS module styles for story wrapper component |
| packages/react/src/ToggleSwitch/ToggleSwitch.tsx | Removes sx prop and Box components, implements CSS module styling with data attributes |
| packages/react/src/ToggleSwitch/ToggleSwitch.module.css | CSS module implementation of component styles with state-based data attribute selectors |
| .changeset/bright-parents-flow.md | Documents breaking change for ToggleSwitch sx removal |
|
|
||
| exports[`@primer/styled-react exports 1`] = ` | ||
| [ | ||
| "ToggleSwitch", |
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.
looks like this was previously following alphabetical order, can we fix back ?
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.
exports.test.ts.snap is auto-generated by the test runner.
I guess the reason ToggleSwitch appears first is that it's handled differently from other components, it's wrapped with createStyledComponent and exported separately.
|
integration checks manually ran here: https://github.com/github/github-ui/pull/1177 |
Migrates the
ToggleSwitchcomponent away fromBoxcomponents andsxprop support to use CSS modules, closes #4826What Changed
Removed
Boxwrapper andsxprop support from the main ToggleSwitch componentReplaced
sxprop styling with CSS modules and data attributes for state-based stylingAdded ToggleSwitch to the
styled-reactpackage for repos still usingsxpropsNote:
SwitchButtonandToggleKnobstill use styled-components (styled.buttonandstyled.div) and will be migrated in a future iterationRollout strategy