Conversation
🦋 Changeset detectedLatest commit: 289544f 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 |
github-actions
bot
added
the
integration-tests: recommended
|
liuliu-dev
added
the
update snapshots
github-actions
bot
removed
the
update snapshots
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
liuliu-dev
added
the
Canary Release
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/17785 |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Card component (upstreamed from Pacer) into @primer/react as a minor semver feature, including several structured subcomponents for common card layouts.
Changes:
- Added
Cardwith subcomponents (Card.Icon,.Image,.Heading,.Description,.Menu,.Metadata) and wired it into the package exports. - Added Card styling via a new CSS module.
- Added unit tests + Storybook stories, updated the exports snapshot, and included a minor changeset.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/index.ts | Re-exports Card and CardProps from the package entrypoint. |
| packages/react/src/Card/index.ts | Defines the public Card object API via Object.assign and re-exports types. |
| packages/react/src/Card/Card.tsx | Implements Card and subcomponents, including child parsing / slot extraction. |
| packages/react/src/Card/Card.module.css | Provides Card layout, typography, and positioning styles. |
| packages/react/src/Card/Card.test.tsx | Adds unit coverage for rendering, edge-to-edge header styling, className, and ref forwarding. |
| packages/react/src/Card/Card.stories.tsx | Adds Storybook stories demonstrating common Card variants. |
| packages/react/src/tests/snapshots/exports.test.ts.snap | Updates export snapshot to include Card and CardProps. |
| .changeset/add-card-component.md | Adds a minor changeset for the new public component. |
Copilot's findings
- Files reviewed: 8/8 changed files
- Comments generated: 5
| const childArray = React.Children.toArray(children) | ||
|
|
||
| for (const child of childArray) { | ||
| if (!React.isValidElement(child)) continue | ||
|
|
There was a problem hiding this comment.
Card currently only extracts known subcomponents and silently drops any other children (they’re never rendered). It also matches slots via child.type === ..., which won’t recognize wrapped slot components (e.g. memo, forwardRef, or wrapper elements using __SLOT__). Consider switching to the existing useSlots/isSlot utilities and rendering the non-slot rest children (or warning in dev if arbitrary children are intentionally unsupported).
packages/react/src/Card/Card.tsx
Outdated
|
|
||
| return ( | ||
| <div ref={ref} className={clsx(classes.Card, className)} {...rest}> | ||
| <div className={clsx(classes.CardHeader, image && classes.CardHeaderEdgeToEdge)}>{icon || image}</div> |
There was a problem hiding this comment.
If both Card.Icon and Card.Image are provided, the header renders icon || image (icon wins) but still applies the edge-to-edge header class based on image. This results in inconsistent rendering. Prefer making Card.Image take precedence when present, or warn/error when both are used together.
| <div className={clsx(classes.CardHeader, image && classes.CardHeaderEdgeToEdge)}>{icon || image}</div> | |
| <div className={clsx(classes.CardHeader, image && classes.CardHeaderEdgeToEdge)}>{image || icon}</div> |
| } else if (child.type === CardDescription) { | ||
| description = child | ||
| } else if (child.type === CardMetadata) { | ||
| metadata = child | ||
| } else if (child.type === CardMenu) { | ||
| menu = child |
There was a problem hiding this comment.
Card.Metadata is stored as a single variable, so multiple <Card.Metadata> children will silently overwrite each other (last one wins) with no dev warning. Either support multiple metadata items (collect and render them all) or enforce a single instance and warn in dev when duplicates are provided (the existing useSlots helper already does this).
| const CardIcon = ({icon: IconComponent, className}: IconProps) => { | ||
| return ( | ||
| <span className={clsx(classes.CardIcon, className)}> | ||
| <IconComponent /> | ||
| </span> |
There was a problem hiding this comment.
Card.Icon renders the provided icon component without any way to set accessibility attributes (e.g. aria-hidden / aria-label) or pass through icon props like size. For decorative icons, consider defaulting to aria-hidden="true", and/or allow passing SVG/icon props through so consumers can provide an accessible name when the icon is meaningful.
| .CardDescription { | ||
| font-size: var(--text-body-size-medium); | ||
| color: var(--fgColor-muted); | ||
| display: -webkit-box; | ||
| overflow: hidden; |
There was a problem hiding this comment.
.CardDescription only resets margin-bottom, leaving the default <p> top margin in place, which can create unintended extra spacing (especially since .CardContent already uses gap). Consider resetting the full margin (e.g. margin: 0) to keep spacing controlled by layout gaps.
liuliu-dev
added
the
update snapshots
github-actions
bot
removed
the
update snapshots
Closes https://github.com/github/primer/issues/6467
Upstream the Card component from
Pacerinto@primer/react.Changelog
New
Card— A styled container component withborder,box-shadow, andborder-radiusCard.Icon— Renders an icon in the card headerCard.Image— Renders an edge-to-edge image in the card headerCard.Heading— Renders a heading (h3) with truncationCard.Description— Renders a description with 2-line clampCard.Menu— Slot for overlay menu (positioned absolute top-right)Card.Metadata— Renders metadata content below the descriptionRollout strategy
Testing & Reviewing
Merge checklist