diff --git a/.changeset/lucky-terms-sing.md b/.changeset/lucky-terms-sing.md new file mode 100644 index 00000000000..c9fc974e679 --- /dev/null +++ b/.changeset/lucky-terms-sing.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Card: Add `data-component` attributes to `Card` and its subcomponents (`Icon`, `Image`, `Heading`, `Description`, `Metadata`, `Menu`). Add an `as` prop (`'div' | 'section'`) so standalone Cards can render as a labelled region landmark; `as="section"` requires `aria-label` or `aria-labelledby`. `Card` now requires `children`. Also improves docs and stories. diff --git a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-colorblind-linux.png b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-colorblind-linux.png index 9d513c74652..91f0540bf99 100644 Binary files a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-colorblind-linux.png and b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-dimmed-linux.png b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-dimmed-linux.png index 692774fea4f..d527154a961 100644 Binary files a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-dimmed-linux.png and b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-dimmed-linux.png differ diff --git a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-high-contrast-linux.png b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-high-contrast-linux.png index ac8caa19164..97b47c0bc02 100644 Binary files a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-high-contrast-linux.png and b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-linux.png b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-linux.png index 9d513c74652..91f0540bf99 100644 Binary files a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-linux.png and b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-linux.png differ diff --git a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-tritanopia-linux.png b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-tritanopia-linux.png index 9d513c74652..91f0540bf99 100644 Binary files a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-tritanopia-linux.png and b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-dark-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-colorblind-linux.png b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-colorblind-linux.png index 171b4a01008..5be730c7196 100644 Binary files a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-colorblind-linux.png and b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-high-contrast-linux.png b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-high-contrast-linux.png index 501b37ef7e1..65f7e72ad59 100644 Binary files a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-high-contrast-linux.png and b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-linux.png b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-linux.png index 171b4a01008..5be730c7196 100644 Binary files a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-linux.png and b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-linux.png differ diff --git a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-tritanopia-linux.png b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-tritanopia-linux.png index 171b4a01008..5be730c7196 100644 Binary files a/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-tritanopia-linux.png and b/.playwright/snapshots/components/Card.test.ts-snapshots/Card-Default-light-tritanopia-linux.png differ diff --git a/packages/react/src/Card/Card.docs.json b/packages/react/src/Card/Card.docs.json index 60805a2a823..12e1a5d1ae9 100644 --- a/packages/react/src/Card/Card.docs.json +++ b/packages/react/src/Card/Card.docs.json @@ -13,9 +13,27 @@ }, { "id": "experimental-components-card-features--with-metadata" + }, + { + "id": "experimental-components-card-features--with-menu" + }, + { + "id": "experimental-components-card-features--standalone-section" + }, + { + "id": "experimental-components-card-features--in-list" + }, + { + "id": "experimental-components-card-features--interactive-content" } ], "props": [ + { + "name": "children", + "type": "React.ReactNode", + "required": true, + "description": "The contents of the card. Provide either `Card.*` subcomponents (for example `Card.Heading`, `Card.Description`, `Card.Metadata`) or any custom content. A card with no children will not render." + }, { "name": "className", "type": "string", @@ -32,6 +50,12 @@ "type": "'medium' | 'large'", "defaultValue": "'large'", "description": "Controls the border radius of the Card." + }, + { + "name": "as", + "type": "'div' | 'section'", + "defaultValue": "'div'", + "description": "The HTML element to render. Use `'section'` for **standalone** Cards (not inside a list of cards) so screen readers announce the Card as a labelled region. When `Card.Heading` is present, `aria-labelledby` is automatically wired. Use the default `'div'` when the Card is inside an `
  • ` of a list of cards." } ], "subcomponents": [ @@ -73,21 +97,42 @@ "name": "as", "type": "'h2' | 'h3' | 'h4' | 'h5' | 'h6'", "defaultValue": "'h3'", - "description": "The heading level to render." + "description": "The heading level to render. When the parent Card uses `as=\"section\"`, the heading's `id` is automatically wired to the section's `aria-labelledby`." } ] }, { "name": "Card.Description", - "props": [] + "props": [ + { + "name": "children", + "type": "React.ReactNode", + "required": true, + "description": "The descriptive text for the card. Rendered inside a `

    ` element so should be flowing text content." + } + ] }, { "name": "Card.Menu", - "props": [] + "props": [ + { + "name": "children", + "type": "React.ReactNode", + "required": true, + "description": "Interactive control for the top-right corner of the card." + } + ] }, { "name": "Card.Metadata", - "props": [] + "props": [ + { + "name": "children", + "type": "React.ReactNode", + "required": true, + "description": "Metadata content for the bottom of the card. Accepts any content: plain text, icons, or other Primer components." + } + ] } ] } diff --git a/packages/react/src/Card/Card.features.stories.tsx b/packages/react/src/Card/Card.features.stories.tsx index 67e664489c8..6e6f93d38be 100644 --- a/packages/react/src/Card/Card.features.stories.tsx +++ b/packages/react/src/Card/Card.features.stories.tsx @@ -1,56 +1,164 @@ import type {Meta} from '@storybook/react-vite' -import {RepoIcon, StarIcon} from '@primer/octicons-react' +import {KebabHorizontalIcon, RepoIcon, RepoForkedIcon, StarIcon} from '@primer/octicons-react' +import {ActionList, ActionMenu, Button, IconButton, VisuallyHidden} from '..' import {Card} from './index' +import classes from './Card.stories.module.css' const meta = { title: 'Experimental/Components/Card/Features', component: Card, + decorators: [ + Story => ( +

    + +
    + ), + ], } satisfies Meta export default meta export const WithImage = () => { return ( -
    - - - Card with Image - This card uses an edge-to-edge image instead of an icon. - -
    + + + Card with Image + This card uses an edge-to-edge image instead of an icon. + ) } export const WithMetadata = () => { return ( -
    + + + primer/react + + {"GitHub's design system implemented as React components for building consistent user interfaces."} + + + + 1.2k stars + + + ) +} + +export const WithMenu = () => { + return ( + + + primer/react + + {"GitHub's design system implemented as React components for building consistent user interfaces."} + + + + + + + + + Star + Watch + Fork + + + + + + ) +} + +export const CustomContent = () => ( + +
    +

    Custom Content Card

    +

    This card uses arbitrary custom content instead of the built-in subcomponents.

    +
      +
    • Item one
    • +
    • Item two
    • +
    • Item three
    • +
    +
    +
    +) + +export const StandaloneSection = () => ( + + + primer/react + + { + 'Standalone cards render as a labelled
    landmark. aria-labelledby is automatically wired to Card.Heading.' + } + + +) + +export const InList = () => ( +
      +
    • - primer/react - - {"GitHub's design system implemented as React components for building consistent user interfaces."} - + primer/react 1.2k stars -
    +
  • +
  • + + + primer/css + + + 850 stars + + +
  • +
  • + + + primer/octicons + + + 2.1k stars + + +
  • + +) + +/** + * When several Cards share the same interactive controls (for example "Star" + * or "Fork" buttons in a list of repositories), the controls' accessible + * names must include enough context to distinguish one card's action from + * another's. This story uses `VisuallyHidden` to append the repo name to + * each button's accessible name — a common pattern across GitHub. + */ +export const InteractiveContent = () => { + const repos = [{name: 'primer/react'}, {name: 'primer/css'}, {name: 'primer/octicons'}] + + return ( + ) } - -export const CustomContent = () => ( -
    - -
    - Custom Content Card -

    This card uses arbitrary custom content instead of the built-in subcomponents.

    -
      -
    • Item one
    • -
    • Item two
    • -
    • Item three
    • -
    -
    -
    -
    -) diff --git a/packages/react/src/Card/Card.stories.module.css b/packages/react/src/Card/Card.stories.module.css new file mode 100644 index 00000000000..bd3ff8acdeb --- /dev/null +++ b/packages/react/src/Card/Card.stories.module.css @@ -0,0 +1,28 @@ +.WidthConstraintContainer { + max-width: 400px; +} + +.CustomContentLayout { + display: flex; + flex-direction: column; + gap: var(--stack-gap-condensed); +} + +.CustomContentLayout > h3, +.CustomContentLayout > p, +.CustomContentLayout > ul { + margin: 0; +} + +.CustomContentLayout > ul { + padding-inline-start: var(--base-size-16); +} + +.CardList { + display: flex; + flex-direction: column; + gap: var(--stack-gap-condensed); + margin: 0; + padding: 0; + list-style: none; +} diff --git a/packages/react/src/Card/Card.stories.tsx b/packages/react/src/Card/Card.stories.tsx index 692ddde46a0..3f0f9d30e54 100644 --- a/packages/react/src/Card/Card.stories.tsx +++ b/packages/react/src/Card/Card.stories.tsx @@ -1,24 +1,32 @@ import type {Meta, StoryFn} from '@storybook/react-vite' -import {RocketIcon} from '@primer/octicons-react' +import {PeopleIcon, RocketIcon} from '@primer/octicons-react' import {Card} from './index' +import classes from './Card.stories.module.css' const meta = { title: 'Experimental/Components/Card', component: Card, + decorators: [ + Story => ( +
    + +
    + ), + ], } satisfies Meta export default meta export const Default = () => { return ( -
    - - - Card Heading - This is a description of the card providing supplemental information. - Updated 2 hours ago - -
    + + + Card Heading + This is a description of the card providing supplemental information. + + 3 contributors + + ) } @@ -30,14 +38,12 @@ type PlaygroundArgs = { } export const Playground: StoryFn = ({showIcon, showMetadata, padding, borderRadius}) => ( -
    - - {showIcon && } - Playground Card - Experiment with the Card component and its subcomponents. - {showMetadata && Just now} - -
    + + {showIcon && } + Playground Card + Experiment with the Card component and its subcomponents. + {showMetadata && Just now} + ) Playground.args = { diff --git a/packages/react/src/Card/Card.test.tsx b/packages/react/src/Card/Card.test.tsx index 817459d4766..ff64867c34d 100644 --- a/packages/react/src/Card/Card.test.tsx +++ b/packages/react/src/Card/Card.test.tsx @@ -1,4 +1,4 @@ -import {describe, expect, it} from 'vitest' +import {describe, expect, it, vi} from 'vitest' import {render, screen} from '@testing-library/react' import {Card} from '../Card' import {implementsClassName} from '../utils/testing' @@ -7,7 +7,14 @@ import classes from './Card.module.css' const TestIcon = () => describe('Card', () => { - implementsClassName(props => , classes.Card) + implementsClassName( + props => ( + + Card Heading + + ), + classes.Card, + ) it('should render a Card with heading and description', () => { render( @@ -171,4 +178,115 @@ describe('Card', () => { ) expect(container.firstChild).toHaveAttribute('data-border-radius', 'medium') }) + + it('should set data-component attributes on Card and its subcomponents', () => { + const {container} = render( + + + With data-component + Description text + Metadata text + + + + , + ) + expect(container.querySelector('[data-component="Card"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="Card.Icon"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="Card.Heading"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="Card.Description"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="Card.Metadata"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="Card.Menu"]')).toBeInTheDocument() + }) + + it('should set data-component="Card.Image" on Card.Image', () => { + const {container} = render( + + + With Image + , + ) + expect(container.querySelector('[data-component="Card.Image"]')).toBeInTheDocument() + }) + + it('should set data-component="Card" on custom content cards', () => { + const {container} = render( + +

    Custom

    +
    , + ) + expect(container.querySelector('[data-component="Card"]')).toBeInTheDocument() + }) + + it('should not render when there are no children', () => { + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + // @ts-expect-error - children is required, but we want to verify the runtime behaviour + const {container} = render() + expect(container).toBeEmptyDOMElement() + consoleSpy.mockRestore() + }) + + it('should not render when all children are falsy', () => { + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const {container} = render({false}) + expect(container).toBeEmptyDOMElement() + consoleSpy.mockRestore() + }) + + it('should warn in development when rendered with no children', () => { + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + // @ts-expect-error - children is required, but we want to verify the dev warning + render() + expect(consoleSpy).toHaveBeenCalledWith('Warning:', expect.stringContaining('was rendered with no children')) + consoleSpy.mockRestore() + }) + + it('should render as a
    by default', () => { + const {container} = render( + + Default Element + , + ) + expect(container.firstChild?.nodeName).toBe('DIV') + }) + + it('should render as a
    when as="section"', () => { + const {container} = render( + + Standalone + This card is standalone. + , + ) + expect(container.firstChild?.nodeName).toBe('SECTION') + expect(container.firstChild).toHaveAttribute('aria-label', 'Standalone card') + }) + + it('should expose the section as a labelled region landmark', () => { + render( + + Standalone + This card is standalone. + , + ) + expect(screen.getByRole('region', {name: 'Standalone'})).toBeInTheDocument() + }) + + it('should auto-wire aria-labelledby to Card.Heading when as="section"', () => { + render( + + Auto-wired + No manual id needed. + , + ) + expect(screen.getByRole('region', {name: 'Auto-wired'})).toBeInTheDocument() + }) + + it('should not forward the `as` prop to the DOM', () => { + const {container} = render( + + Heading + , + ) + expect(container.firstChild).not.toHaveAttribute('as') + }) }) diff --git a/packages/react/src/Card/Card.tsx b/packages/react/src/Card/Card.tsx index c6f81ffb311..1d8661d74d9 100644 --- a/packages/react/src/Card/Card.tsx +++ b/packages/react/src/Card/Card.tsx @@ -1,26 +1,35 @@ import {clsx} from 'clsx' -import React, {forwardRef} from 'react' +import React, {type ForwardedRef, createContext, forwardRef, useContext} from 'react' import classes from './Card.module.css' +import {warning} from '../utils/warning' +import {fixedForwardRef, type PolymorphicProps} from '../utils/modern-polymorphic' +import {useId} from '../hooks/useId' -export type CardProps = React.ComponentPropsWithoutRef<'div'> & { - /** - * Provide an optional className to add to the outermost element rendered by - * the Card - */ - className?: string +type CardContextValue = {titleId?: string} +const CardContext = createContext({}) - /** - * Controls the internal padding of the Card. - * @default 'normal' - */ - padding?: 'none' | 'condensed' | 'normal' +type CardAs = 'div' | 'section' - /** - * Controls the border radius of the Card. - * @default 'large' - */ - borderRadius?: 'medium' | 'large' -} +export type CardProps = PolymorphicProps< + As, + 'div', + { + /** Optional className for the root element. */ + className?: string + + /** Internal padding. @default 'normal' */ + padding?: 'none' | 'condensed' | 'normal' + + /** Border radius. @default 'large' */ + borderRadius?: 'medium' | 'large' + + /** + * Card contents. Provide either `Card.*` subcomponents (e.g. `Card.Heading`, + * `Card.Description`, `Card.Metadata`) or custom content. Empty cards do not render. + */ + children: React.ReactNode + } +> type HeadingLevel = 'h2' | 'h3' | 'h4' | 'h5' | 'h6' @@ -33,6 +42,9 @@ type HeadingProps = React.ComponentPropsWithoutRef<'h3'> & { } type DescriptionProps = React.ComponentPropsWithoutRef<'p'> & { + /** + * Card description. Rendered as a `

    `, so keep it to flowing text. + */ children: React.ReactNode } @@ -60,17 +72,40 @@ type ImageProps = React.ComponentPropsWithoutRef<'img'> & { } type MenuProps = { + /** Interactive control for the top-right corner of the card. */ children: React.ReactNode } type MetadataProps = React.ComponentPropsWithoutRef<'div'> & { + /** + * Metadata row at the bottom of the card. Any content works: text, icons, + * a `Label`, an `Octicon`. + */ children: React.ReactNode } -const CardImpl = forwardRef(function Card( - {children, className, padding = 'normal', borderRadius = 'large', ...rest}, - ref, +function CardComponent( + props: CardProps, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ref: ForwardedRef, ) { + const { + children, + className, + padding = 'normal', + borderRadius = 'large', + as = 'div', + ...rest + } = props as CardProps + const Component = as as React.ElementType + const generatedId = useId() + const titleId = as === 'section' ? generatedId : undefined + + // Auto-wire aria-labelledby when as="section" unless consumer provides their own + if (as === 'section' && !('aria-label' in props) && !('aria-labelledby' in props)) { + ;(rest as Record)['aria-labelledby'] = titleId + } + let icon: React.ReactNode = null let image: React.ReactNode = null let heading: React.ReactNode = null @@ -100,47 +135,71 @@ const CardImpl = forwardRef(function Card( const hasSlotChildren = icon || image || heading || description || metadata || menu + // `React.Children.toArray` already drops `null`/`undefined`/`false`/`true`, + // so an empty array means we have nothing to render. + const isEmpty = !hasSlotChildren && childArray.length === 0 + + warning( + isEmpty, + 'The component was rendered with no children and will not render. Provide either Card subcomponents (Card.Heading, Card.Description, etc.) or custom content.', + ) + + if (isEmpty) { + return null + } + if (!hasSlotChildren) { return ( -

    + + {children} + + + ) + } + + return ( + + - {children} -
    - ) - } - - return ( -
    - {(image || icon) && ( -
    {image || icon}
    - )} -
    -
    - {heading} - {description} + {(image || icon) && ( +
    {image || icon}
    + )} +
    +
    + {heading} + {description} +
    + {metadata ?
    {metadata}
    : null}
    - {metadata ?
    {metadata}
    : null} -
    - {menu ?
    {menu}
    : null} -
    + {menu ?
    {menu}
    : null} + + ) -}) +} + +CardComponent.displayName = 'Card' -const CardIcon = ({icon: IconComponent, 'aria-label': ariaLabel, className}: IconProps) => { +const CardImpl = fixedForwardRef(CardComponent) + +function CardIcon({icon: IconComponent, 'aria-label': ariaLabel, className}: IconProps) { return ( { - return {alt} +function CardImage({src, alt = '', className, ...rest}: ImageProps) { + return ( + {alt} + ) } CardImage.displayName = 'Card.Image' +/** + * Heading shown at the top of a Card. + * + * When the parent Card uses `as="section"`, the heading's `id` is + * automatically wired to the section's `aria-labelledby`. + */ const CardHeading = forwardRef(function CardHeading( - {as: Component = 'h3', children, className, ...rest}, + {as: Component = 'h3', children, className, id, ...rest}, ref, ) { + const {titleId} = useContext(CardContext) return ( - + {children} ) }) +CardHeading.displayName = 'Card.Heading' + const CardDescription = forwardRef(function CardDescription( {children, className, ...rest}, ref, ) { return ( -

    +

    {children}

    ) }) -const CardMenu = ({children}: MenuProps) => { - return <>{children} +CardDescription.displayName = 'Card.Description' + +/** + * Top-right slot for a single interactive control. + * + * Give the control a label that names the card (e.g. `"More options for + * Project Alpha"`, not just `"More options"`) so users can tell which card + * the action applies to when several cards are visible. + */ +function CardMenu({children}: MenuProps) { + return
    {children}
    } CardMenu.displayName = 'Card.Menu' @@ -191,12 +276,14 @@ const CardMetadata = forwardRef(function CardMeta ref, ) { return ( -
    +
    {children}
    ) }) +CardMetadata.displayName = 'Card.Metadata' + export {CardImpl, CardIcon, CardImage, CardHeading, CardDescription, CardMenu, CardMetadata} export type {HeadingProps as CardHeadingProps} export type {DescriptionProps as CardDescriptionProps}