-
Notifications
You must be signed in to change notification settings - Fork 626
Update SelectPanel.playground.stories.tsx to no longer use styled-components #6525
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
base: main
Are you sure you want to change the base?
Conversation
|
…ories.tsx Co-authored-by: hectahertz <[email protected]>
👋 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 migrates the SelectPanel playground story from styled-components to CSS modules by replacing the Box
component with sx
prop usage with a standard div
element and CSS module styles.
Key Changes:
- Replaced
Box
component withdiv
element for color indicators - Created CSS module file with static styles for the color indicator
- Maintained inline styles for dynamic background colors
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
SelectPanel.playground.stories.tsx | Removed Box import, added CSS module import and clsx, replaced Box with div using CSS class |
SelectPanel.playground.stories.module.css | New CSS module file containing ColorIndicator styles |
width: 14px; | ||
height: 14px; | ||
border-radius: 100%; | ||
} |
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.
The CSS module should be wrapped in a CSS layer as mentioned in the PR description. Add @layer primer.components.select-panel-playground
wrapper around the styles to follow the project's CSS layering convention.
Copilot uses AI. Check for mistakes.
sx={{width: 14, height: 14, borderRadius: '100%'}} | ||
style={{backgroundColor: `#${label.color}`}} | ||
/> | ||
<div className={clsx(classes.ColorIndicator)} style={{backgroundColor: `#${label.color}`}} /> |
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.
The clsx
call is unnecessary here since you're only passing a single class name. You can simplify this to className={classes.ColorIndicator}
.
<div className={clsx(classes.ColorIndicator)} style={{backgroundColor: `#${label.color}`}} /> | |
<div className={classes.ColorIndicator} style={{backgroundColor: `#${label.color}`}} /> |
Copilot uses AI. Check for mistakes.
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.
Looks like this needs to be reverted since we don't want to add the snapshot with the cursor
sx={{width: 14, height: 14, borderRadius: '100%'}} | ||
style={{backgroundColor: `#${label.color}`}} | ||
/> | ||
<div className={clsx(classes.ColorIndicator)} style={{backgroundColor: `#${label.color}`}} /> |
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.
Agree that we don't need clsx here, and so we can probably remove the import as well
.ColorIndicator { | ||
width: 14px; | ||
height: 14px; | ||
border-radius: 100%; |
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.
I think we can use var(--borderRadius-full)
here for consistency with other files, but maybe not a strictly necessary change
This PR migrates the
SelectPanel.playground.stories.tsx
story from usingBox
with thesx
prop to CSS modules, removing the dependency on styled-components for this story.Changes Made
Box
import and usage withsx
prop for color indicatorsSelectPanel.playground.stories.module.css
with CSS layer and static stylesBox
component with adiv
using the CSS module classColorIndicator
clsx
and added CSS module importbackgroundColor
as inline style for label-specific colorsTechnical Details
The color indicator that was previously implemented as:
Is now implemented as:
With corresponding CSS module:
Visual Verification
The story maintains identical visual appearance and functionality. All color indicators render correctly as circular elements with the appropriate label colors.
Fixes #6523.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.