From c88e569acd2dd56b37b24ad57e21fc47254d0aaf Mon Sep 17 00:00:00 2001 From: Niels Roozemond Date: Thu, 16 May 2024 10:39:07 +0200 Subject: [PATCH] feat: Allow overriding button labels in Alert, Dialog, Pagination, and Search Field (#1220) Co-authored-by: Vincent Smedinga Co-authored-by: Aram <37216945+alimpens@users.noreply.github.com> --- packages/react/src/Alert/Alert.test.tsx | 10 ++++- packages/react/src/Alert/Alert.tsx | 11 +++-- packages/react/src/Dialog/Dialog.test.tsx | 10 ++++- packages/react/src/Dialog/Dialog.tsx | 10 ++++- .../react/src/Pagination/Pagination.test.tsx | 24 +++++++++- packages/react/src/Pagination/Pagination.tsx | 45 ++++++++++++------- .../src/SearchField/SearchField.test.tsx | 8 ++++ .../src/SearchField/SearchFieldButton.tsx | 9 ++-- 8 files changed, 97 insertions(+), 30 deletions(-) diff --git a/packages/react/src/Alert/Alert.test.tsx b/packages/react/src/Alert/Alert.test.tsx index 87aee7b0a7..28dde6ea6d 100644 --- a/packages/react/src/Alert/Alert.test.tsx +++ b/packages/react/src/Alert/Alert.test.tsx @@ -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' @@ -52,6 +52,14 @@ describe('Alert', () => { expect(closeButton).toBeVisible() }) + it('renders the close button with a label', () => { + render() + + 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() diff --git a/packages/react/src/Alert/Alert.tsx b/packages/react/src/Alert/Alert.tsx index 1c23611006..93c468337f 100644 --- a/packages/react/src/Alert/Alert.tsx +++ b/packages/react/src/Alert/Alert.tsx @@ -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 @@ -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, @@ -65,7 +68,7 @@ export const Alert = forwardRef( )} {children} - {closeable && } + {closeable && } ) }, diff --git a/packages/react/src/Dialog/Dialog.test.tsx b/packages/react/src/Dialog/Dialog.test.tsx index 3f827e2697..1dcfb2ad0c 100644 --- a/packages/react/src/Dialog/Dialog.test.tsx +++ b/packages/react/src/Dialog/Dialog.test.tsx @@ -77,7 +77,15 @@ describe('Dialog', () => { it('renders DialogClose button', () => { render() - const closeButton = screen.getByText('Sluiten') + const closeButton = screen.getByRole('button', { name: 'Sluiten' }) + + expect(closeButton).toBeInTheDocument() + }) + + it('renders a custom close label', () => { + render() + + const closeButton = screen.getByRole('button', { name: 'Close' }) expect(closeButton).toBeInTheDocument() }) diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index e94691192f..8b94f1fc20 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -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> export const Dialog = forwardRef( - ({ children, className, title, actions, ...restProps }: DialogProps, ref: ForwardedRef) => ( + ( + { actions, children, className, closeButtonLabel = 'Sluiten', title, ...restProps }: DialogProps, + ref: ForwardedRef, + ) => (
{title} - +
{children}
{actions &&
{actions}
} diff --git a/packages/react/src/Pagination/Pagination.test.tsx b/packages/react/src/Pagination/Pagination.test.tsx index 867fde8a01..373de078ae 100644 --- a/packages/react/src/Pagination/Pagination.test.tsx +++ b/packages/react/src/Pagination/Pagination.test.tsx @@ -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() @@ -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() @@ -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() + 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() + + 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() const { container } = render() const component = container.querySelector(':only-child') + expect(ref.current).toBe(component) }) }) diff --git a/packages/react/src/Pagination/Pagination.tsx b/packages/react/src/Pagination/Pagination.tsx index c787abb4cc..1ed090b665 100644 --- a/packages/react/src/Pagination/Pagination.tsx +++ b/packages/react/src/Pagination/Pagination.tsx @@ -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 @@ -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, ) => { const [currentPage, setCurrentPage] = useState(page) @@ -117,14 +128,14 @@ export const Pagination = forwardRef(
  1. {range.map((pageNumberOrSpacer) => @@ -156,13 +167,13 @@ export const Pagination = forwardRef( )}
  2. diff --git a/packages/react/src/SearchField/SearchField.test.tsx b/packages/react/src/SearchField/SearchField.test.tsx index 0ebc7f99d5..4187fd95fc 100644 --- a/packages/react/src/SearchField/SearchField.test.tsx +++ b/packages/react/src/SearchField/SearchField.test.tsx @@ -31,6 +31,14 @@ describe('Search field', () => { expect(component).toBeVisible() }) + it('renders the button with a label', () => { + render() + + const component = screen.getByRole('button', { name: 'Search' }) + + expect(component).toBeInTheDocument() + }) + it('renders the outer container design system BEM class name', () => { render() diff --git a/packages/react/src/SearchField/SearchFieldButton.tsx b/packages/react/src/SearchField/SearchFieldButton.tsx index c36a46027b..6d2a10d251 100644 --- a/packages/react/src/SearchField/SearchFieldButton.tsx +++ b/packages/react/src/SearchField/SearchFieldButton.tsx @@ -10,14 +10,17 @@ import type { ForwardedRef, HTMLAttributes } from 'react' import { Icon } from '../Icon' import { VisuallyHidden } from '../VisuallyHidden' -type SearchFieldButtonProps = HTMLAttributes +type SearchFieldButtonProps = { + /** The label for the button that triggers the search action. */ + label?: string +} & HTMLAttributes // 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) => ( + ({ label = 'Zoeken', className, ...restProps }: SearchFieldButtonProps, ref: ForwardedRef) => ( ),