Skip to content

Commit

Permalink
feat!: Rename title props for Alert, Header and Dialog and require it…
Browse files Browse the repository at this point in the history
… for Dialog (#1251)

Co-authored-by: Vincent Smedinga <[email protected]>
  • Loading branch information
dlnr and VincentSmedinga authored Jun 7, 2024
1 parent 1f4c09a commit bbec4de
Show file tree
Hide file tree
Showing 19 changed files with 111 additions and 80 deletions.
2 changes: 1 addition & 1 deletion packages/css/src/components/alert/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ There are four types of notifications:
The grid’s white space is a good reference – place the Alert in its own cell.
- By default, the Alert cannot be closed.
This functionality can be added optionally.
- Optionally, the title can be omitted.
- Optionally, the heading can be omitted.
2 changes: 1 addition & 1 deletion packages/css/src/components/header/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# Header

Arranges the City’s logo, the title of the website, and a page menu.
Arranges the City’s logo, the name of the application, and a page menu.

## Guidelines

Expand Down
4 changes: 2 additions & 2 deletions packages/css/src/components/header/header.scss
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@
}
}

.ams-header__title {
.ams-header__app-name {
flex: 1 1 100%;

@media screen and (min-width: $ams-breakpoint-wide) {
min-inline-size: 0;
order: 2;

.ams-header__title-heading {
.ams-header__app-name-heading {
display: block;
inline-size: 100%;
line-height: 1;
Expand Down
10 changes: 10 additions & 0 deletions packages/react/src/Alert/Alert.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,16 @@ describe('Alert', () => {
expect(ref.current).toBe(component)
})

it('renders a heading', () => {
render(<Alert heading="Test heading" />)

const heading = screen.getByRole('heading', {
name: 'Test heading',
})

expect(heading).toBeInTheDocument()
})

it('renders the close button', () => {
const { container } = render(<Alert closeable={true} />)

Expand Down
14 changes: 7 additions & 7 deletions packages/react/src/Alert/Alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ export type AlertProps = {
closeable?: boolean
/** The label for the button that dismisses the Alert. */
closeButtonLabel?: string
/** The text for the Heading. */
heading?: string
/** The hierarchical level of the Alert’s heading within the document. */
headingLevel?: HeadingProps['level']
/** A function to run when dismissing. */
onClose?: () => void
/** The significance of the message conveyed. */
severity?: 'error' | 'info' | 'success' | 'warning'
/** The text for the Heading. */
title?: string
} & PropsWithChildren<HTMLAttributes<HTMLDivElement>>

const iconSvgBySeverity = {
Expand All @@ -41,26 +41,26 @@ export const Alert = forwardRef(
className,
closeable,
closeButtonLabel = 'Sluiten',
heading,
headingLevel = 2,
onClose,
severity = 'warning',
title,
...restProps
}: AlertProps,
ref: ForwardedRef<HTMLDivElement>,
) => {
const alertSize = title ? 'level-4' : 'level-5'
const Tag = title ? 'section' : 'div'
const alertSize = heading ? 'level-4' : 'level-5'
const Tag = heading ? 'section' : 'div'

return (
<Tag {...restProps} ref={ref} className={clsx('ams-alert', severity && `ams-alert--${severity}`, className)}>
<div className="ams-alert__icon">
<Icon size={alertSize} svg={iconSvgBySeverity[severity]} />
</div>
<div className="ams-alert__content">
{title && (
{heading && (
<Heading level={headingLevel} size="level-4">
{title}
{heading}
</Heading>
)}
{children}
Expand Down
32 changes: 18 additions & 14 deletions packages/react/src/Dialog/Dialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import '@testing-library/jest-dom'

describe('Dialog', () => {
it('renders', () => {
render(<Dialog open />)
render(<Dialog heading="Test heading" open />)

const component = screen.getByRole('dialog')

Expand All @@ -14,15 +14,15 @@ describe('Dialog', () => {
})

it('renders a design system BEM class name', () => {
render(<Dialog />)
render(<Dialog heading="Test heading" />)

const component = screen.getByRole('dialog', { hidden: true })

expect(component).toHaveClass('ams-dialog')
})

it('renders an additional class name', () => {
render(<Dialog className="extra" />)
render(<Dialog heading="Test heading" className="extra" />)

const component = screen.getByRole('dialog', { hidden: true })

Expand All @@ -34,56 +34,60 @@ describe('Dialog', () => {
it('supports ForwardRef in React', () => {
const ref = createRef<HTMLDialogElement>()

render(<Dialog ref={ref} />)
render(<Dialog heading="Test heading" ref={ref} />)

const component = screen.getByRole('dialog', { hidden: true })

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

it('is not visible when open attribute is not used', () => {
render(<Dialog />)
render(<Dialog heading="Test heading" />)

const component = screen.getByRole('dialog', { hidden: true })

expect(component).toBeInTheDocument()
expect(component).not.toBeVisible()
})

it('renders a title', () => {
const { getByText } = render(<Dialog title="Dialog Title" />)
it('renders a heading', () => {
render(<Dialog heading="Test heading" open />)

expect(getByText('Dialog Title')).toBeInTheDocument()
const heading = screen.getByRole('heading', {
name: 'Test heading',
})

expect(heading).toBeInTheDocument()
})

it('renders children', () => {
const { getByText } = render(<Dialog>Dialog Content</Dialog>)
const { getByText } = render(<Dialog heading="Test heading">Test content</Dialog>)

expect(getByText('Dialog Content')).toBeInTheDocument()
expect(getByText('Test content')).toBeInTheDocument()
})

it('renders actions when provided', () => {
const { getByText } = render(<Dialog actions={<button>Click Me</button>} />)
const { getByText } = render(<Dialog heading="Test heading" actions={<button>Click Me</button>} />)

expect(getByText('Click Me')).toBeInTheDocument()
})

it('does not render actions when not provided', () => {
const { queryByText } = render(<Dialog />)
const { queryByText } = render(<Dialog heading="Test heading" />)

expect(queryByText('Click Me')).not.toBeInTheDocument()
})

it('renders DialogClose button', () => {
render(<Dialog open />)
render(<Dialog heading="Test heading" open />)

const closeButton = screen.getByRole('button', { name: 'Sluiten' })

expect(closeButton).toBeInTheDocument()
})

it('renders a custom close label', () => {
render(<Dialog open closeButtonLabel="Close" />)
render(<Dialog heading="Test heading" open closeButtonLabel="Close" />)

const closeButton = screen.getByRole('button', { name: 'Close' })

Expand Down
6 changes: 4 additions & 2 deletions packages/react/src/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,19 @@ export type DialogProps = {
actions?: ReactNode
/** The label for the button that dismisses the Dialog. */
closeButtonLabel?: string
/** The text for the Heading. */
heading: string
} & PropsWithChildren<DialogHTMLAttributes<HTMLDialogElement>>

export const Dialog = forwardRef(
(
{ actions, children, className, closeButtonLabel = 'Sluiten', title, ...restProps }: DialogProps,
{ actions, children, className, closeButtonLabel = 'Sluiten', heading, ...restProps }: DialogProps,
ref: ForwardedRef<HTMLDialogElement>,
) => (
<dialog {...restProps} ref={ref} className={clsx('ams-dialog', className)}>
<form method="dialog" className="ams-dialog__form">
<header className="ams-dialog__header">
<Heading size="level-4">{title}</Heading>
<Heading size="level-4">{heading}</Heading>
<IconButton formNoValidate label={closeButtonLabel} size="level-4" />
</header>
<article className="ams-dialog__article">{children}</article>
Expand Down
15 changes: 13 additions & 2 deletions packages/react/src/Header/Header.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,24 @@ describe('Header', () => {
expect(logoLinkTitle).toHaveTextContent('Go to homepage')
})

it('renders an application name', () => {
render(<Header appName="Application name" />)

const heading = screen.getByRole('heading', {
name: 'Application name',
level: 1,
})

expect(heading).toBeInTheDocument()
})

it('renders with links', () => {
const { container } = render(<Header links={<div>Menu Content</div>} />)
const { container } = render(<Header links={<div>Test content</div>} />)

const menu = container.querySelector('.ams-header__links')

expect(menu).toBeInTheDocument()
expect(menu).toHaveTextContent('Menu Content')
expect(menu).toHaveTextContent('Test content')
})

it('renders with menu button', () => {
Expand Down
20 changes: 10 additions & 10 deletions packages/react/src/Header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,29 @@ import type { LogoBrand } from '../Logo'
import { VisuallyHidden } from '../VisuallyHidden'

export type HeaderProps = {
/** The name of the application. */
appName?: string
/** The list of menu links. Use a Page Menu here. */
links?: ReactNode
/** The name of the brand for which to display the logo. */
logoBrand?: LogoBrand
/** The url for the link on the logo. */
logoLink?: string
/** The accessible text for the link on the logo. */
logoLinkTitle?: string
/** The name of the application. */
title?: string
/** The list of menu links. Use a Page Menu here. */
links?: ReactNode
/** A button to toggle the visibility of a Mega Menu. */
menu?: ReactNode
} & HTMLAttributes<HTMLElement>

export const Header = forwardRef(
(
{
appName,
className,
links,
logoBrand = 'amsterdam',
logoLink = '/',
logoLinkTitle = 'Ga naar de homepage',
title,
links,
menu,
...restProps
}: HeaderProps,
Expand All @@ -49,10 +49,10 @@ export const Header = forwardRef(
</a>
{links && <div className="ams-header__links">{links}</div>}
{menu && <div className="ams-header__menu">{menu}</div>}
{title && (
<div className="ams-header__title">
<Heading level={1} size="level-3" className="ams-header__title-heading">
{title}
{appName && (
<div className="ams-header__app-name">
<Heading level={1} size="level-3" className="ams-header__app-name-heading">
{appName}
</Heading>
</div>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import { TableOfContents } from './TableOfContents'

describe('Table of Contents link', () => {
it('renders', () => {
render(<TableOfContents.Link label="Test" href="#" />)
render(<TableOfContents.Link label="Test label" href="#" />)

const link = screen.getByRole('link')

expect(link).toBeInTheDocument()
expect(link).toBeVisible()
expect(link).toHaveTextContent('Test')
expect(link).toHaveTextContent('Test label')
})

it('renders a design system BEM class name', () => {
Expand Down
6 changes: 3 additions & 3 deletions storybook/src/components/Alert/Alert.docs.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ For clarification, formatted text can be included in the alert.

<Canvas of={AlertStories.WithList} />

### Without Title
### Without Heading

Sometimes, a title is unnecessary.
Sometimes, a heading is unnecessary.
The icon automatically becomes slightly smaller.

<Canvas of={AlertStories.WithoutTitle} />
<Canvas of={AlertStories.WithoutHeading} />
14 changes: 7 additions & 7 deletions storybook/src/components/Alert/Alert.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ const meta = {
title: 'Components/Feedback/Alert',
component: Alert,
args: {
title: 'Let op',
closeable: false,
heading: 'Let op',
},
} satisfies Meta<typeof Alert>

Expand All @@ -33,7 +33,7 @@ export const Default: Story = {
export const Warning: Story = {
args: {
children: <Paragraph>U bent vergeten verplichte velden in te vullen.</Paragraph>,
title: 'Vul de gegevens aan',
heading: 'Vul de gegevens aan',
},
}

Expand All @@ -45,17 +45,17 @@ export const Error: Story = {
Probeer het over een paar minuten opnieuw.
</Paragraph>
),
heading: 'Niet gelukt',
severity: 'error',
title: 'Niet gelukt',
},
}

export const Success: Story = {
args: {
children: <Paragraph>Het formulier is verzonden. We hebben uw gegevens goed ontvangen.</Paragraph>,
closeable: true,
heading: 'Gelukt',
severity: 'success',
title: 'Gelukt',
},
}

Expand All @@ -74,14 +74,14 @@ export const Info: Story = {

export const WithList: Story = {
args: {
title: 'Vul de gegevens aan',
children: [
<Paragraph key={1}>U bent vergeten de volgende verplichte velden in te vullen:</Paragraph>,
<UnorderedList key={2}>
<UnorderedList.Item>Naam</UnorderedList.Item>
<UnorderedList.Item>Telefoonnummer</UnorderedList.Item>
</UnorderedList>,
],
heading: 'Vul de gegevens aan',
},
}

Expand All @@ -100,13 +100,13 @@ export const WithInlineLink: Story = {
},
}

export const WithoutTitle: Story = {
export const WithoutHeading: Story = {
args: {
children: (
<Paragraph>
De geschatte wachttijd in de adreszoeker klopt momenteel niet altijd. We werken aan een oplossing.
</Paragraph>
),
title: undefined,
heading: undefined,
},
}
Loading

0 comments on commit bbec4de

Please sign in to comment.