-
Notifications
You must be signed in to change notification settings - Fork 639
feat(Flash): remove support for sx #6834
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: b25c42a 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 |
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 removes sx prop support from the Flash component in @primer/react and adds a styled version to @primer/styled-react to maintain backward compatibility during migration.
- Remove
sxprop from Flash component and replace with standard CSS styling approaches - Migrate Flash component to use CSS modules instead of sx styling system
- Update Storybook stories to use standard React
styleprop and CSS classes instead ofsx
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/styled-react/src/index.tsx | Adds Flash component with sx support to styled-react package for backward compatibility |
| packages/react/src/Flash/Flash.tsx | Removes sx prop and BoxWithFallback dependency, simplifies component to use standard HTML div |
| packages/react/src/Flash/Flash.stories.module.css | Adds CSS class for WithIconActionDismiss story layout |
| packages/react/src/Flash/Flash.features.stories.tsx | Updates stories to use style prop and CSS classes instead of sx, removes Octicon wrapper |
| packages/react/src/Flash/Flash.features.stories.module.css | Adds CSS styling for icon color and WithIconActionDismiss layout |
| .changeset/metal-deer-refuse.md | Documents breaking change for major version release |
| display: 'grid'; | ||
| grid-template-columns: 'min-content 1fr minmax(0, auto)'; | ||
| grid-template-rows: 'min-content'; | ||
| grid-template-areas: 'visual message actions close'; | ||
|
|
||
| @media screen and (max-width: 543.98px) { | ||
| grid-template-columns: 'min-content 1fr'; | ||
| grid-template-rows: 'min-content min-content'; | ||
| grid-template-areas: 'visual message close' '. actions actions'; |
Copilot
AI
Sep 10, 2025
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.
CSS property values should not be wrapped in quotes. Remove the quotes around 'grid', 'min-content 1fr minmax(0, auto)', 'min-content', and 'visual message actions close'.
| display: 'grid'; | |
| grid-template-columns: 'min-content 1fr minmax(0, auto)'; | |
| grid-template-rows: 'min-content'; | |
| grid-template-areas: 'visual message actions close'; | |
| @media screen and (max-width: 543.98px) { | |
| grid-template-columns: 'min-content 1fr'; | |
| grid-template-rows: 'min-content min-content'; | |
| grid-template-areas: 'visual message close' '. actions actions'; | |
| display: grid; | |
| grid-template-columns: min-content 1fr minmax(0, auto); | |
| grid-template-rows: min-content; | |
| grid-template-areas: "visual message actions close"; | |
| @media screen and (max-width: 543.98px) { | |
| grid-template-columns: min-content 1fr; | |
| grid-template-rows: min-content min-content; | |
| grid-template-areas: "visual message close" ". actions actions"; |
| display: 'grid'; | ||
| grid-template-columns: 'min-content 1fr minmax(0, auto)'; | ||
| grid-template-rows: 'min-content'; | ||
| grid-template-areas: 'visual message actions close'; | ||
|
|
||
| @media screen and (max-width: 543.98px) { | ||
| grid-template-columns: 'min-content 1fr'; | ||
| grid-template-rows: 'min-content min-content'; | ||
| grid-template-areas: 'visual message close' '. actions actions'; |
Copilot
AI
Sep 10, 2025
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.
CSS property values should not be wrapped in quotes. Remove the quotes around 'min-content 1fr', 'min-content min-content', and 'visual message close' '. actions actions'.
| display: 'grid'; | |
| grid-template-columns: 'min-content 1fr minmax(0, auto)'; | |
| grid-template-rows: 'min-content'; | |
| grid-template-areas: 'visual message actions close'; | |
| @media screen and (max-width: 543.98px) { | |
| grid-template-columns: 'min-content 1fr'; | |
| grid-template-rows: 'min-content min-content'; | |
| grid-template-areas: 'visual message close' '. actions actions'; | |
| display: grid; | |
| grid-template-columns: min-content 1fr minmax(0, auto); | |
| grid-template-rows: min-content; | |
| grid-template-areas: "visual message actions close"; | |
| @media screen and (max-width: 543.98px) { | |
| grid-template-columns: min-content 1fr; | |
| grid-template-rows: min-content min-content; | |
| grid-template-areas: "visual message close" ". actions actions"; |
|
👋 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 📦
|
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.
Nice! There's a failing type-check but approved in advance
|
Waiting on: https://github.com/github/github-ui/pull/2732 to merge |
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/2820 |
|
🟢 ci completed with status |
|
👋 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! |
|
👋 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/4824
Changelog
New
Changed
Flashto supportsxin@primer/styled-reactOcticonin storiesRemoved
sxfromFlashRollout strategy
sxusage downstream will pull from@primer/styled-react. We will make sure usage is updated before merging