-
Notifications
You must be signed in to change notification settings - Fork 605
fix(Avatar): merge style when style prop is provided #5963
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: b55409d 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 |
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 updates the Avatar component to correctly merge the internal CSS size variables with any external style prop and streamlines the rendering by always using BoxWithFallback.
- Merges the internal style (cssSizeVars) with any provided style prop so that user-defined styles override default sizing.
- Moves the component to consistently use BoxWithFallback in place of conditionally rendering an img element.
- Updates and adds tests in the Avatar test suite to validate the new style merging behavior.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/react/src/Avatar/Avatar.tsx | Refactors component to always use BoxWithFallback and merge styles |
packages/react/src/Avatar/Avatar.test.tsx | Adds tests to check the style merge behavior and moves tests to the Avatar folder |
.changeset/soft-cases-own.md | Updates changeset information for the patch release |
Comments suppressed due to low confidence (1)
packages/react/src/Avatar/Avatar.tsx:53
- The component now unconditionally uses BoxWithFallback instead of conditionally rendering either a Box or an img element. Confirm that this change is intentional and that BoxWithFallback handles all cases equivalently to ensure no unintended side effects in accessibility or rendering.
style={ style ? { ...cssSizeVars, ...style, } : (cssSizeVars as React.CSSProperties) }
👋 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.
size-limit report 📦
|
Co-authored-by: Copilot <[email protected]>
Closes https://github.com/github/primer/issues/5120
Update the
Avatar
component to merge style when thestyle
prop is providedChangelog
New
Changed
BoxWithFallback
instead of branching between two types of componentsstyle
prop when providedRemoved
Rollout strategy