Skip to content
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

feat: Let Grid and Grid Cell render any structural tag #1662

Merged
merged 5 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/react/src/Breakout/BreakoutCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
* @license EUPL-1.2+
* Copyright Gemeente Amsterdam
*/

import clsx from 'clsx'
import { forwardRef } from 'react'
import type { HTMLAttributes, PropsWithChildren } from 'react'
import type { ForwardedRef, HTMLAttributes, PropsWithChildren } from 'react'
import type { BreakoutRowNumber, BreakoutRowNumbers } from './Breakout'
import { breakoutCellClasses } from './breakoutCellClasses'
import type { GridColumnNumber, GridColumnNumbers } from '../Grid/Grid'
Expand Down Expand Up @@ -54,7 +55,7 @@ export const BreakoutCell = forwardRef(
rowStart,
...restProps
}: BreakoutCellProps,
ref: any,
ref: ForwardedRef<any>,
) => (
<Tag
{...restProps}
Expand Down
22 changes: 19 additions & 3 deletions packages/react/src/Grid/Grid.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { render } from '@testing-library/react'
import { render, screen } from '@testing-library/react'
import { createRef } from 'react'
import { Grid } from './Grid'
import { Grid, gridTags } from './Grid'
import type { GridPaddingSize } from './Grid'
import { ariaRoleForTag } from '../common/accessibility'
import '@testing-library/jest-dom'

const paddingSizes = ['small', 'medium', 'large']
Expand Down Expand Up @@ -87,8 +88,23 @@ describe('Grid', () => {
})
})

gridTags.forEach((tag) => {
it(`renders with a custom ${tag} tag`, () => {
const { container } = render(<Grid as={tag} aria-label={tag === 'section' ? 'Accessible name' : undefined} />)
alimpens marked this conversation as resolved.
Show resolved Hide resolved

let component: HTMLElement | null
if (tag === 'div') {
component = container.querySelector(tag)
} else {
component = screen.getByRole(ariaRoleForTag[tag])
}

expect(component).toBeInTheDocument()
})
})

it('supports ForwardRef in React', () => {
const ref = createRef<HTMLDivElement>()
const ref = createRef<HTMLElement>()

const { container } = render(<Grid ref={ref} />)

Expand Down
22 changes: 18 additions & 4 deletions packages/react/src/Grid/Grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ export type GridColumnNumbers = {
}
export type GridPaddingSize = 'small' | 'medium' | 'large'

export const gridTags = ['article', 'aside', 'div', 'footer', 'header', 'main', 'nav', 'section'] as const
export type GridTag = (typeof gridTags)[number]

type GridPaddingVerticalProp = {
paddingBottom?: never
paddingTop?: never
Expand All @@ -33,17 +36,28 @@ type GridPaddingTopAndBottomProps = {
}

export type GridProps = {
/** The HTML tag to use. */
as?: GridTag
/** The amount of space between rows. */
gapVertical?: 'none' | 'small' | 'large'
} & (GridPaddingVerticalProp | GridPaddingTopAndBottomProps) &
PropsWithChildren<HTMLAttributes<HTMLDivElement>>

const GridRoot = forwardRef(
(
{ children, className, gapVertical, paddingBottom, paddingTop, paddingVertical, ...restProps }: GridProps,
ref: ForwardedRef<HTMLDivElement>,
{
as: Tag = 'div',
children,
className,
gapVertical,
paddingBottom,
paddingTop,
paddingVertical,
...restProps
}: GridProps,
ref: ForwardedRef<any>,
) => (
<div
<Tag
{...restProps}
ref={ref}
className={clsx(
Expand All @@ -54,7 +68,7 @@ const GridRoot = forwardRef(
)}
>
{children}
</div>
</Tag>
),
)

Expand Down
39 changes: 25 additions & 14 deletions packages/react/src/Grid/GridCell.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { render, screen } from '@testing-library/react'
import { createRef } from 'react'
import { Grid } from './Grid'
import { gridCellTags } from './GridCell'
import { ariaRoleForTag } from '../common/accessibility'
import '@testing-library/jest-dom'

describe('Grid cell', () => {
Expand Down Expand Up @@ -29,16 +31,6 @@ describe('Grid cell', () => {
expect(component).toHaveClass('ams-grid__cell extra')
})

it('supports ForwardRef in React', () => {
const ref = createRef<HTMLDivElement>()

const { container } = render(<Grid.Cell ref={ref} />)

const component = container.querySelector(':only-child')

expect(ref.current).toBe(component)
})

it('renders no class names for undefined values for start and span', () => {
const { container } = render(<Grid.Cell />)

Expand Down Expand Up @@ -113,11 +105,30 @@ describe('Grid cell', () => {
expect(component).toHaveClass('ams-grid__cell--span-all')
})

it('renders a custom tag', () => {
render(<Grid.Cell as="article" />)
gridCellTags.forEach((tag) => {
it(`renders with a custom ${tag} tag`, () => {
const { container } = render(
<Grid.Cell as={tag} aria-label={tag === 'section' ? 'Accessible name' : undefined} />,
)

let component: HTMLElement | null
if (tag === 'div') {
component = container.querySelector(tag)
} else {
component = screen.getByRole(ariaRoleForTag[tag])
}

expect(component).toBeInTheDocument()
})
})

it('supports ForwardRef in React', () => {
const ref = createRef<HTMLElement>()

const { container } = render(<Grid.Cell ref={ref} />)

const cell = screen.getByRole('article')
const component = container.querySelector(':only-child')

expect(cell).toBeInTheDocument()
expect(ref.current).toBe(component)
})
})
8 changes: 6 additions & 2 deletions packages/react/src/Grid/GridCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@
* @license EUPL-1.2+
* Copyright Gemeente Amsterdam
*/

import clsx from 'clsx'
import { forwardRef } from 'react'
import type { HTMLAttributes, PropsWithChildren } from 'react'
import type { GridColumnNumber, GridColumnNumbers } from './Grid'
import { gridCellClasses } from './gridCellClasses'

export const gridCellTags = ['article', 'aside', 'div', 'footer', 'header', 'main', 'nav', 'section'] as const
export type GridCellTag = (typeof gridCellTags)[number]

type GridCellSpanAllProp = {
/** Lets the cell span the full width of all grid variants. */
span: 'all'
Expand All @@ -22,8 +26,8 @@ type GridCellSpanAndStartProps = {
}

export type GridCellProps = {
/** The HTML element to use. */
as?: 'article' | 'div' | 'section'
/** The HTML tag to use. */
as?: GridCellTag
} & (GridCellSpanAllProp | GridCellSpanAndStartProps) &
PropsWithChildren<HTMLAttributes<HTMLElement>>

Expand Down
14 changes: 14 additions & 0 deletions packages/react/src/common/accessibility.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
alimpens marked this conversation as resolved.
Show resolved Hide resolved
* @license EUPL-1.2+
* Copyright Gemeente Amsterdam
*/

export const ariaRoleForTag: Record<string, string> = {
article: 'article',
aside: 'complementary',
footer: 'contentinfo',
header: 'banner',
main: 'main',
nav: 'navigation',
section: 'region',
}
4 changes: 2 additions & 2 deletions storybook/src/components/Grid/Grid.docs.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ Use the `start` prop with 3 values, e.g. `start={{ narrow: 2, medium: 4, wide: 6

### Improve semantics

By default, a Grid Cell renders a `<div>` element in HTML.
Use the `as` prop to make your markup more semantic.
By default, both Grid and Grid Cell render a `<div>` element in HTML.
Use the `as` prop on either to make your markup more semantic.

<Canvas of={GridStories.ImproveSemantics} />

Expand Down
4 changes: 0 additions & 4 deletions storybook/src/components/Grid/Grid.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ export default meta
const cellMeta = {
component: Grid.Cell,
argTypes: {
as: {
control: { type: 'radio' },
options: ['article', 'div', 'section'],
},
span: {
control: { type: 'number', min: 1, max: 12 },
},
Expand Down