Skip to content

Commit

Permalink
Merge branch 'develop' into feature/DES-772-rename-breadcrumb-item-to…
Browse files Browse the repository at this point in the history
…-link
  • Loading branch information
VincentSmedinga authored May 17, 2024
2 parents 0a05b30 + fd668c1 commit 637fb94
Show file tree
Hide file tree
Showing 14 changed files with 178 additions and 63 deletions.
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@
"private": true,
"engines": {
"node": "^20",
"npm": "^8",
"npm": "^10",
"pnpm": "^9"
},
"volta": {
"node": "20.11.1",
"npm": "10.5.0",
"pnpm": "9.0.2"
"node": "20.13.1",
"npm": "10.5.2",
"pnpm": "9.1.1"
},
"workspaces": [
"./packages/*",
Expand Down
1 change: 1 addition & 0 deletions packages/css/src/components/button/button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
font-family: var(--ams-button-font-family);
font-size: var(--ams-button-font-size);
gap: var(--ams-button-gap);
justify-content: center;
line-height: var(--ams-button-line-height);
outline-offset: var(--ams-button-outline-offset);
padding-block: var(--ams-button-padding-block);
Expand Down
7 changes: 7 additions & 0 deletions packages/css/src/components/dialog/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ A Dialog allows the user to focus on one task or a piece of information by poppi
- Use a dialog for short and non-frequent tasks.
Consider using the main flow for regular tasks.

## The order of buttons

If your Dialog needs more than one button, put the one for the primary action first and the other buttons behind it.
Sighted users will read the primary action first, in line with the natural reading order.
The same goes for users of screen readers, who will hear the primary action first, and users of a keyboard, who will focus the primary action first.
Also, this approach keeps the order of buttons consistent on both narrow and wide screens: if the buttons do not fit next to each other, they get stacked vertically with the primary action on top.

## Keyboard Support

| key | function |
Expand Down
14 changes: 7 additions & 7 deletions packages/css/src/components/dialog/dialog.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
* Copyright Gemeente Amsterdam
*/

@import "../../common/breakpoint";

.ams-dialog {
background-color: var(--ams-dialog-background-color);
border: var(--ams-dialog-border);
Expand Down Expand Up @@ -48,11 +46,13 @@

.ams-dialog__footer {
display: flex;
flex-direction: column;
grid-gap: var(--ams-dialog-footer-gap);
flex-wrap: wrap; // [1]
gap: var(--ams-dialog-footer-gap);
margin-inline-end: auto; // [1]

@media screen and (min-width: $ams-breakpoint-medium) {
flex-direction: row;
justify-content: end;
> * {
flex: auto; // [1]
}
}

// [1] This combination stacks the buttons vertically and stretches them, until they fit next to each other.
10 changes: 9 additions & 1 deletion packages/react/src/Alert/Alert.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fireEvent, render } from '@testing-library/react'
import { fireEvent, render, screen } from '@testing-library/react'
import { createRef } from 'react'
import { Alert } from './Alert'
import '@testing-library/jest-dom'
Expand Down Expand Up @@ -52,6 +52,14 @@ describe('Alert', () => {
expect(closeButton).toBeVisible()
})

it('renders the close button with a label', () => {
render(<Alert closeable={true} closeButtonLabel="Close" />)

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

expect(closeButton).toBeInTheDocument()
})

it('fires the onClose event when the close button is clicked', () => {
const onClose = jest.fn()
const { container } = render(<Alert closeable={true} onClose={onClose} />)
Expand Down
11 changes: 7 additions & 4 deletions packages/react/src/Alert/Alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { IconButton } from '../IconButton'
export type AlertProps = {
/** Whether the alert can be dismissed by the user. Adds a button to the top right. */
closeable?: boolean
/** The label for the button that dismisses the Alert. */
closeButtonLabel?: string
/**
* The hierarchical level of the alert title within the document.
* @default 2
Expand All @@ -40,11 +42,12 @@ export const Alert = forwardRef(
{
children,
className,
headingLevel = 2,
title,
severity = 'warning',
closeable,
closeButtonLabel = 'Sluiten',
headingLevel = 2,
onClose,
severity = 'warning',
title,
...restProps
}: AlertProps,
ref: ForwardedRef<HTMLDivElement>,
Expand All @@ -65,7 +68,7 @@ export const Alert = forwardRef(
)}
{children}
</div>
{closeable && <IconButton label="Sluiten" size={alertSize} onClick={onClose} />}
{closeable && <IconButton label={closeButtonLabel} size={alertSize} onClick={onClose} />}
</Tag>
)
},
Expand Down
10 changes: 9 additions & 1 deletion packages/react/src/Dialog/Dialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,15 @@ describe('Dialog', () => {
it('renders DialogClose button', () => {
render(<Dialog open />)

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

expect(closeButton).toBeInTheDocument()
})

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

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

expect(closeButton).toBeInTheDocument()
})
Expand Down
10 changes: 8 additions & 2 deletions packages/react/src/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,22 @@ import { Heading } from '../Heading'
import { IconButton } from '../IconButton'

export type DialogProps = {
/** Children for the footer of the dialog, like a save or close button. */
actions?: ReactNode
/** The label for the button that dismisses the Dialog. */
closeButtonLabel?: string
} & PropsWithChildren<DialogHTMLAttributes<HTMLDialogElement>>

export const Dialog = forwardRef(
({ children, className, title, actions, ...restProps }: DialogProps, ref: ForwardedRef<HTMLDialogElement>) => (
(
{ actions, children, className, closeButtonLabel = 'Sluiten', title, ...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>
<IconButton formNoValidate label="Sluiten" size="level-4" />
<IconButton formNoValidate label={closeButtonLabel} size="level-4" />
</header>
<article className="ams-dialog__article">{children}</article>
{actions && <footer className="ams-dialog__footer">{actions}</footer>}
Expand Down
24 changes: 22 additions & 2 deletions packages/react/src/Pagination/Pagination.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('Pagination', () => {
expect(screen.getByTestId('firstSpacer')).toBeInTheDocument()
})

it('should navigate to the next page when clicking on the "next" button', () => {
it('should navigate to the next page when clicking on the next button', () => {
const onPageChangeMock = jest.fn()
render(<Pagination page={6} totalPages={10} onPageChange={onPageChangeMock} />)

Expand All @@ -60,7 +60,7 @@ describe('Pagination', () => {
expect(screen.getByText('7')).toHaveAttribute('aria-current', 'true')
})

it('should navigate to the previous page when clicking on the "previous" button', () => {
it('should navigate to the previous page when clicking on the previous button', () => {
const onPageChangeMock = jest.fn()
render(<Pagination page={6} totalPages={10} onPageChange={onPageChangeMock} />)

Expand Down Expand Up @@ -98,10 +98,30 @@ describe('Pagination', () => {
expect(screen.getByText('5')).not.toHaveAttribute('aria-current', 'true')
})

it('renders custom labels for the ‘previous’ and ‘next’ buttons', () => {
render(<Pagination totalPages={10} previousLabel="previous" nextLabel="next" />)
const previousButton = screen.getByRole('button', { name: 'Vorige pagina' })
const nextButton = screen.getByRole('button', { name: 'Volgende pagina' })

expect(previousButton).toHaveTextContent('previous')
expect(nextButton).toHaveTextContent('next')
})

it('renders custom aria-labels for the ‘previous’ and ‘next’ buttons', () => {
render(<Pagination totalPages={10} previousAriaLabel="Previous page" nextAriaLabel="Next page" />)

const previousButton = screen.getByRole('button', { name: 'Previous page' })
const nextButton = screen.getByRole('button', { name: 'Next page' })

expect(previousButton).toBeInTheDocument()
expect(nextButton).toBeInTheDocument()
})

it('supports ForwardRef in React', () => {
const ref = createRef<HTMLElement>()
const { container } = render(<Pagination totalPages={10} ref={ref} />)
const component = container.querySelector(':only-child')

expect(ref.current).toBe(component)
})
})
45 changes: 28 additions & 17 deletions packages/react/src/Pagination/Pagination.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,22 @@ import type { ForwardedRef, HTMLAttributes } from 'react'
import { Icon } from '../Icon/Icon'

export type PaginationProps = {
/**
* The maximum amount of pages shown. This has a lower limit of 5
*/
/** The maximum amount of pages shown. This has a lower limit of 5. */
maxVisiblePages?: number
/**
* Callback triggered when interaction changes the page number.
*/
/** The accessible name for the link to the next page. */
nextAriaLabel?: string
/** The label for the link to the next page. */
nextLabel?: string
/** Callback triggered when interaction changes the page number. */
// eslint-disable-next-line no-unused-vars
onPageChange?: (page: number) => void
/**
* The current page number.
*/
/** The current page number. */
page?: number
/**
* The total amount of pages.
*/
/** The accessible name for the link to the previous page. */
previousAriaLabel?: string
/** The label for the link to the previous page. */
previousLabel?: string
/** The total amount of pages. */
totalPages: number
} & HTMLAttributes<HTMLElement>

Expand Down Expand Up @@ -81,7 +81,18 @@ function getRange(currentPage: number, totalPages: number, maxVisiblePages: numb

export const Pagination = forwardRef(
(
{ className, maxVisiblePages = 7, onPageChange, page = 1, totalPages, ...restProps }: PaginationProps,
{
className,
maxVisiblePages = 7,
nextAriaLabel = 'Volgende pagina',
nextLabel = 'volgende',
onPageChange,
page = 1,
previousAriaLabel = 'Vorige pagina',
previousLabel = 'vorige',
totalPages,
...restProps
}: PaginationProps,
ref: ForwardedRef<HTMLElement>,
) => {
const [currentPage, setCurrentPage] = useState(page)
Expand Down Expand Up @@ -117,14 +128,14 @@ export const Pagination = forwardRef(
<ol className="ams-pagination__list">
<li>
<button
aria-label="Vorige pagina"
aria-label={previousAriaLabel}
className="ams-pagination__button"
disabled={currentPage === 1}
onClick={onPrevious}
type="button"
>
<Icon svg={ChevronLeftIcon} size="level-5" />
vorige
{previousLabel}
</button>
</li>
{range.map((pageNumberOrSpacer) =>
Expand Down Expand Up @@ -156,13 +167,13 @@ export const Pagination = forwardRef(
)}
<li>
<button
aria-label="Volgende pagina"
aria-label={nextAriaLabel}
className="ams-pagination__button"
disabled={currentPage === totalPages}
onClick={onNext}
type="button"
>
volgende
{nextLabel}
<Icon svg={ChevronRightIcon} size="level-5" />
</button>
</li>
Expand Down
8 changes: 8 additions & 0 deletions packages/react/src/SearchField/SearchField.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ describe('Search field', () => {
expect(component).toBeVisible()
})

it('renders the button with a label', () => {
render(<SearchField.Button label="Search" />)

const component = screen.getByRole('button', { name: 'Search' })

expect(component).toBeInTheDocument()
})

it('renders the outer container design system BEM class name', () => {
render(<SearchField />)

Expand Down
9 changes: 6 additions & 3 deletions packages/react/src/SearchField/SearchFieldButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,17 @@ import type { ForwardedRef, HTMLAttributes } from 'react'
import { Icon } from '../Icon'
import { VisuallyHidden } from '../VisuallyHidden'

type SearchFieldButtonProps = HTMLAttributes<HTMLButtonElement>
type SearchFieldButtonProps = {
/** The label for the button that triggers the search action. */
label?: string
} & HTMLAttributes<HTMLButtonElement>

// TODO: replace this with IconButton when that's done
// TODO: discuss if IconButton is the right component to replace this
export const SearchFieldButton = forwardRef(
({ className, ...restProps }: SearchFieldButtonProps, ref: ForwardedRef<HTMLButtonElement>) => (
({ label = 'Zoeken', className, ...restProps }: SearchFieldButtonProps, ref: ForwardedRef<HTMLButtonElement>) => (
<button {...restProps} ref={ref} className={clsx('ams-search-field__button', className)}>
<VisuallyHidden>Zoeken</VisuallyHidden>
<VisuallyHidden>{label}</VisuallyHidden>
<Icon svg={SearchIcon} size="level-5" square />
</button>
),
Expand Down
15 changes: 15 additions & 0 deletions storybook/src/components/Dialog/Dialog.docs.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,18 @@ Content taller than the dialog itself will scroll.
Click or tap the button to open a dialog.

<Canvas of={DialogStories.TriggerButton} />

Some basic example code to open and close a dialog:

```ts
const openDialog = () => (document.querySelector("#openDialog") as HTMLDialogElement)?.showModal();

const closeDialog = (e: MouseEvent<HTMLButtonElement>) => e.currentTarget.closest("dialog")?.close();
```

## Vertically Stacked Buttons

If the buttons don’t fit next to each other, they will stack vertically and stretch to the full width.
This can occur with a narrow Dialog, long Button labels, a large text size or zooming in.

<Canvas of={DialogStories.VerticalButtons} />
Loading

0 comments on commit 637fb94

Please sign in to comment.