From 4471c7a154e9f1f31dd92298f932f4bbc134dbf9 Mon Sep 17 00:00:00 2001 From: Aram <37216945+alimpens@users.noreply.github.com> Date: Wed, 15 May 2024 13:04:42 +0200 Subject: [PATCH 1/2] feat!: Use logical properties for Search Field, Select, Text Area and Text Input (#1226) Co-authored-by: Vincent Smedinga --- .../css/src/components/search-field/search-field.scss | 6 +++--- packages/css/src/components/select/select.scss | 2 +- packages/css/src/components/text-area/text-area.scss | 8 ++++---- packages/css/src/components/text-input/text-input.scss | 2 +- .../tokens/src/components/ams/search-field.tokens.json | 10 +++++----- .../tokens/src/components/ams/text-area.tokens.json | 2 +- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/css/src/components/search-field/search-field.scss b/packages/css/src/components/search-field/search-field.scss index 56cdf5069e..f3ffaf1450 100644 --- a/packages/css/src/components/search-field/search-field.scss +++ b/packages/css/src/components/search-field/search-field.scss @@ -27,12 +27,12 @@ font-family: var(--ams-search-field-input-font-family); font-size: var(--ams-search-field-input-font-size); font-weight: var(--ams-search-field-input-font-weight); + inline-size: 100%; line-height: var(--ams-search-field-input-line-height); outline-offset: var(--ams-search-field-input-outline-offset); padding-block: var(--ams-search-field-input-padding-block); padding-inline: var(--ams-search-field-input-padding-inline); touch-action: manipulation; - width: 100%; @include text-rendering; @include reset-input; @@ -53,10 +53,10 @@ .ams-search-field__input::-webkit-search-cancel-button { appearance: none; background-image: var(--ams-search-field-input-cancel-button-background-image); + block-size: var(--ams-search-field-input-cancel-button-block-size); cursor: pointer; - height: var(--ams-search-field-input-cancel-button-height); + inline-size: var(--ams-search-field-input-cancel-button-inline-size); margin-inline-start: 0.5rem; - width: var(--ams-search-field-input-cancel-button-width); } @mixin reset-button { diff --git a/packages/css/src/components/select/select.scss b/packages/css/src/components/select/select.scss index 72db3c7e95..f8de4f618b 100644 --- a/packages/css/src/components/select/select.scss +++ b/packages/css/src/components/select/select.scss @@ -16,8 +16,8 @@ font-family: var(--ams-select-font-family); font-size: var(--ams-select-font-size); font-weight: var(--ams-select-font-weight); + inline-size: 100%; line-height: var(--ams-select-line-height); - max-inline-size: 100%; outline-offset: var(--ams-select-outline-offset); padding-block: var(--ams-select-padding-block); padding-inline: var(--ams-select-padding-inline); diff --git a/packages/css/src/components/text-area/text-area.scss b/packages/css/src/components/text-area/text-area.scss index 7c253dc504..96aeb5de55 100644 --- a/packages/css/src/components/text-area/text-area.scss +++ b/packages/css/src/components/text-area/text-area.scss @@ -18,14 +18,14 @@ font-family: var(--ams-text-area-font-family); font-size: var(--ams-text-area-font-size); font-weight: var(--ams-text-area-font-weight); + inline-size: 100%; line-height: var(--ams-text-area-line-height); - max-width: 100%; - min-height: var(--ams-text-area-min-height); + max-inline-size: 100%; // This prevents overflow when using the `cols` prop + min-block-size: var(--ams-text-area-min-block-size); outline-offset: var(--ams-text-area-outline-offset); padding-block: var(--ams-text-area-padding-block); padding-inline: var(--ams-text-area-padding-inline); touch-action: manipulation; - width: 100%; @include text-rendering; @include reset; @@ -69,5 +69,5 @@ } .ams-text-area--cols { - width: auto; + inline-size: auto; } diff --git a/packages/css/src/components/text-input/text-input.scss b/packages/css/src/components/text-input/text-input.scss index 22f3e1e938..e839902c41 100644 --- a/packages/css/src/components/text-input/text-input.scss +++ b/packages/css/src/components/text-input/text-input.scss @@ -18,12 +18,12 @@ font-family: var(--ams-text-input-font-family); font-size: var(--ams-text-input-font-size); font-weight: var(--ams-text-input-font-weight); + inline-size: 100%; line-height: var(--ams-text-input-line-height); outline-offset: var(--ams-text-input-outline-offset); padding-block: var(--ams-text-input-padding-block); padding-inline: var(--ams-text-input-padding-inline); touch-action: manipulation; - width: 100%; @include text-rendering; @include reset; diff --git a/proprietary/tokens/src/components/ams/search-field.tokens.json b/proprietary/tokens/src/components/ams/search-field.tokens.json index ba1f3e1f73..a8e2d86b39 100644 --- a/proprietary/tokens/src/components/ams/search-field.tokens.json +++ b/proprietary/tokens/src/components/ams/search-field.tokens.json @@ -5,11 +5,11 @@ "background-color": { "value": "{ams.color.primary-blue}" }, "color": { "value": "{ams.color.primary-white}" }, "outline-offset": { "value": "{ams.focus.outline-offset}" }, + "padding-block": { "value": "{ams.space.inside.xs}" }, + "padding-inline": { "value": "{ams.space.inside.xs}" }, "hover": { "background-color": { "value": "{ams.color.dark-blue}" } - }, - "padding-block": { "value": "{ams.space.inside.xs}" }, - "padding-inline": { "value": "{ams.space.inside.xs}" } + } }, "input": { "background-color": { "value": "{ams.color.primary-white}" }, @@ -28,9 +28,9 @@ "background-image": { "value": "url(\"data:image/svg+xml;utf8,\")" }, + "block-size": { "value": "{ams.text.level.5.font-size}" }, "color": { "value": "{ams.color.primary-blue}" }, - "height": { "value": "{ams.text.level.5.font-size}" }, - "width": { "value": "{ams.text.level.5.font-size}" } + "inline-size": { "value": "{ams.text.level.5.font-size}" } }, "hover": { "box-shadow": { diff --git a/proprietary/tokens/src/components/ams/text-area.tokens.json b/proprietary/tokens/src/components/ams/text-area.tokens.json index 7cf68b7d6d..fc9b9e488b 100644 --- a/proprietary/tokens/src/components/ams/text-area.tokens.json +++ b/proprietary/tokens/src/components/ams/text-area.tokens.json @@ -8,7 +8,7 @@ "font-size": { "value": "{ams.text.level.5.font-size}" }, "font-weight": { "value": "{ams.text.font-weight.normal}" }, "line-height": { "value": "{ams.text.level.5.line-height}" }, - "min-height": { + "min-block-size": { "value": "calc({ams.text.level.5.line-height} * 1em + 2 * {ams.text-area.padding-block})" }, "outline-offset": { "value": "{ams.focus.outline-offset}" }, From c88e569acd2dd56b37b24ad57e21fc47254d0aaf Mon Sep 17 00:00:00 2001 From: Niels Roozemond Date: Thu, 16 May 2024 10:39:07 +0200 Subject: [PATCH 2/2] 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(
    2. {range.map((pageNumberOrSpacer) => @@ -156,13 +167,13 @@ export const Pagination = forwardRef( )}
    3. 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) => ( ),