-
Notifications
You must be signed in to change notification settings - Fork 614
Cleanup styled-components in *.stories.tsx files and convert to CSS modules #6197
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
|
… and LabelGroup stories Co-authored-by: jonrohan <[email protected]>
Co-authored-by: jonrohan <[email protected]>
@@ -0,0 +1,9 @@ | |||
@layer primer.stories.label-group { |
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.
Remove the CSS layers from these changes
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.
Removed CSS layers from all story CSS module files (5412c12)
@@ -172,9 +172,9 @@ export function ComplexListInsetVariantStory(): JSX.Element { | |||
<ActionList.Item | |||
{...props} | |||
leadingVisual={() => ( | |||
<StyledDiv sx={{'&>svg': {fill: 'white'}}}> | |||
<Box className={classes.StyledDivWithWhiteFill}> |
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.
Anytime we have a Box
component without any sx
prop or as
prop, use a div
instead
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.
Replaced Box components without sx/as props with div elements and removed unused Box import (5412c12)
- Removed @layer declarations from all story CSS module files - Replaced Box components without sx/as props with div elements in ActionList.stories.tsx - Removed unused Box import from ActionList.stories.tsx Co-authored-by: jonrohan <[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! |
size-limit report 📦
|
` | ||
const ErsatzOverlay = ({maxWidth, children, ...props}: {maxWidth?: string; children: React.ReactNode}) => ( | ||
<div | ||
className={clsx(classes.ErsatzOverlay, maxWidth && classes.ErsatzOverlay)} |
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.
this doesn't look right
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.
Fixed the duplicate className issue - removed the incorrect conditional duplicate class assignment (420f132)
box-shadow: | ||
0 1px 3px rgba(0, 0, 0, 0.12), | ||
0 8px 24px rgba(149, 157, 165, 0.2); | ||
overflow: hidden; |
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 don't see this one in the original
overflow: hidden; |
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.
Removed the incorrect overflow: hidden
property that wasn't in the original ActionMenu styled component (420f132)
/* Helper styles for focus trap visual aids */ | ||
.HelperGlobalStyling { | ||
/* These styles need to be applied globally for the demo */ | ||
} |
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.
/* Helper styles for focus trap visual aids */ | |
.HelperGlobalStyling { | |
/* These styles need to be applied globally for the demo */ | |
} | |
/* Helper styles for focus trap visual aids */ |
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.
Applied the suggested comment simplification (420f132)
Co-authored-by: francinelucca <[email protected]>
Head branch was pushed to by a user without write access
This PR removes all
styled-components
imports from story files and converts them to use CSS modules, following the established pattern in the codebase as outlined in ADR-016.Changes Made
Files Updated
Specific Conversions
ActionList/ActionMenu Stories:
ErsatzOverlay
styled div with CSS module classStyledDiv
components that usedsx
prop withBox
component + CSS classesLabelGroup Stories:
ResizableContainer
styled div with CSS module classHook Stories (useFocusZone, useFocusTrap, useAnchoredPosition, ThemeProvider):
MarginButton
styled Button components with CSS module classescreateGlobalStyle
usage to inline<style>
components for demo purposesFloat
,Anchor
,Nav
,Main
) with CSS module classesCSS Module Pattern
All new CSS modules follow the established conventions:
@layer primer.stories.<story-name>
for proper CSS layeringvar(--bgColor-default)
)Benefits
Verification
styled-components
imports remain in any*.stories.tsx
filesFixes #6196.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.