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: Allow overriding button labels in Alert, Dialog, Pagination, and Search Field #1220

Merged
merged 17 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from 11 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
9 changes: 9 additions & 0 deletions packages/react/src/Alert/Alert.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ describe('Alert', () => {
expect(closeButton).toBeVisible()
})

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

const component = container.querySelector(':only-child')
const closeButton = component?.querySelector('.ams-icon-button')

expect(closeButton).toHaveTextContent('Close')
dlnr marked this conversation as resolved.
Show resolved Hide resolved
})

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
21 changes: 19 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,27 @@ 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="Vorige pagina" nextAriaLabel="Volgende pagina" />)

expect(screen.getByRole('button', { name: 'Vorige pagina' })).toHaveAttribute('aria-label', 'Vorige pagina')
expect(screen.getByRole('button', { name: 'Volgende pagina' })).toHaveAttribute('aria-label', 'Volgende pagina')
dlnr marked this conversation as resolved.
Show resolved Hide resolved
})

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
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
/** 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')

expect(component).toHaveTextContent('Search')
dlnr marked this conversation as resolved.
Show resolved Hide resolved
})

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
Loading