-
Notifications
You must be signed in to change notification settings - Fork 614
fix(ProgressBar): pass-through style props #6169
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: 3202498 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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! |
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 enables consumers to pass a style
prop to the ProgressBar.Item
component and have it merged with the component’s internal CSS custom properties.
- Destructure and forward the
style
prop inProgressBar.Item
- Change internal
styles
object toReact.CSSProperties
- Merge internal styles and consumer-provided styles when rendering
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/react/src/ProgressBar/ProgressBar.tsx | Destructure style , convert styles to React.CSSProperties , and merge it with incoming style |
.changeset/small-baboons-press.md | Add changeset for style prop pass-through |
Comments suppressed due to low confidence (2)
packages/react/src/ProgressBar/ProgressBar.tsx:35
- New style prop pass-through functionality lacks unit tests. Consider adding tests to verify that custom style properties are correctly merged and override internal styles.
style,
packages/react/src/ProgressBar/ProgressBar.tsx:35
- Component documentation and Storybook stories should be updated to reflect that the style prop is now supported and passed through to the root element.
style,
export * from 'react' | ||
|
||
declare module 'react' { | ||
interface CSSProperties { |
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.
can you elaborate as to why this is needed? 🙏
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.
Oh, forgot to mention it in the description, this updates the definition of CSSProperties to allow for CSS variables, so that we can just used React.CSS properties
as the type in https://github.com/primer/react/pull/6169/files#diff-c755511099c2739a4276dc3b714e3ad5083385129f4fb3a461115179c50cbcbcR52, instead of {[key: string]: string}
which is a bit too generic.
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.
Actually looks like its causing the build to fail, so reverting that part of the change
Co-authored-by: Copilot <[email protected]>
size-limit report 📦
|
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
Merge checklist