From 7f636e43893683ddd292a504450b9b78dbe7c5dc Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Wed, 18 Sep 2024 16:34:22 +0200 Subject: [PATCH 01/17] Introduce ActionGroup subcomponent for Dialog buttons --- .../css/src/components/dialog/dialog.scss | 7 ++-- packages/react/src/Dialog/Dialog.tsx | 2 + .../src/Dialog/DialogActionGroup.test.tsx | 41 +++++++++++++++++++ .../react/src/Dialog/DialogActionGroup.tsx | 20 +++++++++ .../src/components/ams/dialog.tokens.json | 2 +- .../src/components/Dialog/Dialog.docs.mdx | 3 +- .../src/components/Dialog/Dialog.stories.tsx | 24 +++++++---- 7 files changed, 84 insertions(+), 15 deletions(-) create mode 100644 packages/react/src/Dialog/DialogActionGroup.test.tsx create mode 100644 packages/react/src/Dialog/DialogActionGroup.tsx diff --git a/packages/css/src/components/dialog/dialog.scss b/packages/css/src/components/dialog/dialog.scss index 60291e5ada..a37cd9c951 100644 --- a/packages/css/src/components/dialog/dialog.scss +++ b/packages/css/src/components/dialog/dialog.scss @@ -42,11 +42,10 @@ so do not apply these styles without an `open` attribute. */ justify-content: space-between; } -.ams-dialog__footer { - display: flex; +.ams-dialog__action-group { + display: inline-flex; flex-wrap: wrap; // [1] - gap: var(--ams-dialog-footer-gap); - margin-inline-end: auto; // [1] + gap: var(--ams-dialog-action-group-gap); > * { flex: auto; // [1] diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index 5f92e5feb0..64fa4140b4 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -8,6 +8,7 @@ import { forwardRef, MouseEvent } from 'react' import type { DialogHTMLAttributes, ForwardedRef, PropsWithChildren, ReactNode } from 'react' import { Heading } from '../Heading' import { IconButton } from '../IconButton' +import { DialogActionGroup } from './DialogActionGroup' export type DialogProps = { /** The label for the button that dismisses the Dialog. */ @@ -40,6 +41,7 @@ const DialogRoot = forwardRef( DialogRoot.displayName = 'Dialog' export const Dialog = Object.assign(DialogRoot, { + ActionGroup: DialogActionGroup, close: closeDialog, open: openDialog, }) diff --git a/packages/react/src/Dialog/DialogActionGroup.test.tsx b/packages/react/src/Dialog/DialogActionGroup.test.tsx new file mode 100644 index 0000000000..704a40dbbe --- /dev/null +++ b/packages/react/src/Dialog/DialogActionGroup.test.tsx @@ -0,0 +1,41 @@ +import { render } from '@testing-library/react' +import { createRef } from 'react' +import '@testing-library/jest-dom' +import { Dialog } from './Dialog' + +describe('Dialog Action Group', () => { + it('renders', () => { + const { container } = render() + + const component = container.querySelector(':only-child') + + expect(component).toBeInTheDocument() + expect(component).toBeVisible() + }) + + it('renders a design system BEM class name', () => { + const { container } = render() + + const component = container.querySelector(':only-child') + + expect(component).toHaveClass('ams-dialog__action-group') + }) + + it('renders an additional class name', () => { + const { container } = render() + + const component = container.querySelector(':only-child') + + expect(component).toHaveClass('ams-dialog__action-group extra') + }) + + 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/Dialog/DialogActionGroup.tsx b/packages/react/src/Dialog/DialogActionGroup.tsx new file mode 100644 index 0000000000..41a0212a40 --- /dev/null +++ b/packages/react/src/Dialog/DialogActionGroup.tsx @@ -0,0 +1,20 @@ +/** + * @license EUPL-1.2+ + * Copyright Gemeente Amsterdam + */ + +import clsx from 'clsx' +import { forwardRef } from 'react' +import type { ForwardedRef, HTMLAttributes, PropsWithChildren } from 'react' + +export type DialogActionGroupProps = PropsWithChildren> + +export const DialogActionGroup = forwardRef( + ({ children, className, ...restProps }: DialogActionGroupProps, ref: ForwardedRef) => ( +
+ {children} +
+ ), +) + +DialogActionGroup.displayName = 'DialogActionGroup' diff --git a/proprietary/tokens/src/components/ams/dialog.tokens.json b/proprietary/tokens/src/components/ams/dialog.tokens.json index 77c0a4a907..1917982293 100644 --- a/proprietary/tokens/src/components/ams/dialog.tokens.json +++ b/proprietary/tokens/src/components/ams/dialog.tokens.json @@ -12,7 +12,7 @@ "header": { "gap": { "value": "{ams.space.md}" } }, - "footer": { + "action-group": { "gap": { "value": "{ams.space.md}" } } } diff --git a/storybook/src/components/Dialog/Dialog.docs.mdx b/storybook/src/components/Dialog/Dialog.docs.mdx index 5b2f8b1ca0..28a69da1f4 100644 --- a/storybook/src/components/Dialog/Dialog.docs.mdx +++ b/storybook/src/components/Dialog/Dialog.docs.mdx @@ -18,8 +18,7 @@ import README from "../../../../packages/css/src/components/dialog/README.md?raw Set `method="dialog"` when using a form in Dialog. This closes the Dialog when submitting the form. -Pass the submit Button to the `footer` prop, -and link it to the form by passing its `id` to the Buttons `form` attribute. +Pass an Action Group containing the submit Button to the `footer` prop and provide the Form’s `id` to the Button’s `form` attribute. The Dialog returns the value of the submit Button, so you can check which Button was clicked. For more information, see [Handling the return value from the dialog (MDN)](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dialog#handling_the_return_value_from_the_dialog). diff --git a/storybook/src/components/Dialog/Dialog.stories.tsx b/storybook/src/components/Dialog/Dialog.stories.tsx index 4571f5a95b..e827969ff8 100644 --- a/storybook/src/components/Dialog/Dialog.stories.tsx +++ b/storybook/src/components/Dialog/Dialog.stories.tsx @@ -12,9 +12,11 @@ const meta = { component: Dialog, args: { footer: ( - + + + ), children: ( @@ -37,6 +39,8 @@ type Story = StoryObj export const Default: Story = { args: { + children: De gegevens zijn opgeslagen., + heading: 'Gelukt', open: true, }, argTypes: { @@ -66,14 +70,14 @@ export const FormDialog: Story = { args: { open: true, footer: ( - <> + - + ), children: (
@@ -101,7 +105,11 @@ export const FormDialog: Story = { export const WithScrollbar: Story = { args: { - footer: , + footer: ( + + + + ), children: [ Algemeen @@ -169,14 +177,14 @@ export const TriggerButton: Story = { export const VerticalButtons: Story = { args: { footer: ( - <> + - + ), children: ( From a1ef7a0fe6b63ccedf51c7729923c62430f19cb5 Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Wed, 18 Sep 2024 16:35:38 +0200 Subject: [PATCH 02/17] Sort object properties in Stories --- .../src/components/Dialog/Dialog.stories.tsx | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/storybook/src/components/Dialog/Dialog.stories.tsx b/storybook/src/components/Dialog/Dialog.stories.tsx index e827969ff8..b9cd410d8f 100644 --- a/storybook/src/components/Dialog/Dialog.stories.tsx +++ b/storybook/src/components/Dialog/Dialog.stories.tsx @@ -11,6 +11,12 @@ const meta = { title: 'Components/Containers/Dialog', component: Dialog, args: { + children: ( + + Weet u zeker dat u door wilt gaan met het uitvoeren van deze actie? Dat verwijdert gegevens die nog niet + opgeslagen zijn. + + ), footer: ( ), - children: ( - - Weet u zeker dat u door wilt gaan met het uitvoeren van deze actie? Dat verwijdert gegevens die nog niet - opgeslagen zijn. - - ), heading: 'Niet alle gegevens zijn opgeslagen', }, argTypes: { @@ -68,7 +68,14 @@ export const Default: Story = { export const FormDialog: Story = { args: { - open: true, + children: ( + + + Weet u zeker dat u door wilt gaan met het uitvoeren van deze actie? Dat verwijdert gegevens die nog niet + opgeslagen zijn. + + + ), footer: ( ), - children: ( -
- - Weet u zeker dat u door wilt gaan met het uitvoeren van deze actie? Dat verwijdert gegevens die nog niet - opgeslagen zijn. - -
- ), + open: true, }, decorators: [ (Story) => ( @@ -105,11 +105,6 @@ export const FormDialog: Story = { export const WithScrollbar: Story = { args: { - footer: ( - - - - ), children: [ Algemeen @@ -142,6 +137,11 @@ export const WithScrollbar: Story = { vinden op de website van Autoriteit Persoonsgegevens.
, ], + footer: ( + + + + ), heading: 'Privacyverklaring gemeente Amsterdam', open: true, }, @@ -176,6 +176,14 @@ export const TriggerButton: Story = { export const VerticalButtons: Story = { args: { + children: ( +
+ + Weet u zeker dat u door wilt gaan met het uitvoeren van deze actie? Dat verwijdert gegevens die nog niet + opgeslagen zijn. + +
+ ), footer: ( ), - children: ( -
- - Weet u zeker dat u door wilt gaan met het uitvoeren van deze actie? Dat verwijdert gegevens die nog niet - opgeslagen zijn. - -
- ), open: true, }, decorators: [ From e436f52696927eae2c15263898f5c473a7679abe Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Thu, 19 Sep 2024 15:32:17 +0200 Subject: [PATCH 03/17] Update documentation --- .../css/src/components/dialog/dialog.scss | 10 +- packages/react/src/Dialog/Dialog.tsx | 2 +- .../src/components/Dialog/Dialog.docs.mdx | 36 ++-- .../src/components/Dialog/Dialog.stories.tsx | 193 +++++++++--------- 4 files changed, 118 insertions(+), 123 deletions(-) diff --git a/packages/css/src/components/dialog/dialog.scss b/packages/css/src/components/dialog/dialog.scss index a37cd9c951..10cfb93e51 100644 --- a/packages/css/src/components/dialog/dialog.scss +++ b/packages/css/src/components/dialog/dialog.scss @@ -30,11 +30,6 @@ so do not apply these styles without an `open` attribute. */ } } -.ams-dialog__body { - overflow-y: auto; - overscroll-behavior-y: contain; -} - .ams-dialog__header { align-items: flex-start; display: flex; @@ -42,6 +37,11 @@ so do not apply these styles without an `open` attribute. */ justify-content: space-between; } +.ams-dialog__body { + overflow-y: auto; + overscroll-behavior-y: contain; +} + .ams-dialog__action-group { display: inline-flex; flex-wrap: wrap; // [1] diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index 64fa4140b4..718c8fcc26 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -13,7 +13,7 @@ import { DialogActionGroup } from './DialogActionGroup' export type DialogProps = { /** The label for the button that dismisses the Dialog. */ closeButtonLabel?: string - /** The button(s) in the footer. Start with a primary button. */ + /** Content for the footer, often an Action Group with one or more buttons. */ footer?: ReactNode /** The text for the Heading. */ heading: string diff --git a/storybook/src/components/Dialog/Dialog.docs.mdx b/storybook/src/components/Dialog/Dialog.docs.mdx index 28a69da1f4..297c58375b 100644 --- a/storybook/src/components/Dialog/Dialog.docs.mdx +++ b/storybook/src/components/Dialog/Dialog.docs.mdx @@ -14,33 +14,35 @@ import README from "../../../../packages/css/src/components/dialog/README.md?raw ## Examples -### Form in a Dialog +### Open and close -Set `method="dialog"` when using a form in Dialog. -This closes the Dialog when submitting the form. -Pass an Action Group containing the submit Button to the `footer` prop and provide the Form’s `id` to the Button’s `form` attribute. -The Dialog returns the value of the submit Button, so you can check which Button was clicked. -For more information, see [Handling the return value from the dialog (MDN)](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dialog#handling_the_return_value_from_the_dialog). +To open the Dialog, use `Dialog.open(dialogId)` from the React package. +To close the Dialog, use either `Dialog.close`, or a `
` as in the following example. - + -### With scrollbar +### Asking to confirm -Content taller than the dialog itself will scroll. +Use a `` when asking to confirm an action, e.g. through ‘OK’ and ‘Cancel’ buttons. +Add `method="dialog"` to let the browser close the Dialog automatically when the form is submitted. - +Wrap one or more buttons in an Action Group and place it in the `footer`. +This ensures correct whitespace and scrolling behaviour. +At the same time, it positions the buttons outside the `form` element. +Create an `id` for the form and add it to the submit Button’s `form` attribute to connect the two. -### Trigger Button +If the Action Group needs to live inside of the `form`, add a medium bottom margin (`ams-mb--md`) to the element before it and make scrolling work correctly. -Click or tap this Button to open the Dialog. +The form returns the `value` of the submit Button, which allows inferring which Button the user clicked. +For more information, see [Handling the return value from the dialog (MDN)](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dialog#handling_the_return_value_from_the_dialog). - + -#### Utility functions +### Tall content will scroll -To open the Dialog, use `Dialog.open(id)` from the React package. -Pass the Dialog’s `id` to the function to select it. -To close the Dialog, use `Dialog.close`. +Content that doesn’t fit entirely in the Dialog will scroll. + + ### Vertically stacked Buttons diff --git a/storybook/src/components/Dialog/Dialog.stories.tsx b/storybook/src/components/Dialog/Dialog.stories.tsx index b9cd410d8f..e717ddced8 100644 --- a/storybook/src/components/Dialog/Dialog.stories.tsx +++ b/storybook/src/components/Dialog/Dialog.stories.tsx @@ -3,33 +3,48 @@ * Copyright Gemeente Amsterdam */ -import { Button, Heading, Paragraph } from '@amsterdam/design-system-react' +import { Button, Column, Heading, Paragraph } from '@amsterdam/design-system-react' import { Dialog } from '@amsterdam/design-system-react/src' import { Meta, StoryObj } from '@storybook/react' +const defaultChildren = (formId: string) => ( + + + Weet u zeker dat u door wilt gaan met het uitvoeren van deze actie? Gegevens die nog niet opgeslagen zijn gaan dan + verloren. + + +) + +const defaultFooter = (formId: string) => ( + + + + +) + const meta = { title: 'Components/Containers/Dialog', component: Dialog, args: { - children: ( - - Weet u zeker dat u door wilt gaan met het uitvoeren van deze actie? Dat verwijdert gegevens die nog niet - opgeslagen zijn. - - ), - footer: ( - - - - ), + children: defaultChildren('ams-dialog-form'), + footer: defaultFooter('ams-dialog-form'), heading: 'Niet alle gegevens zijn opgeslagen', }, argTypes: { footer: { table: { disable: true }, }, + heading: { + description: 'The text for the heading.', + }, + open: { + description: 'Whether the dialog box is active and available for interaction.', + }, }, } satisfies Meta @@ -40,17 +55,16 @@ type Story = StoryObj export const Default: Story = { args: { children: De gegevens zijn opgeslagen., + footer: ( + + + + ), heading: 'Gelukt', open: true, }, - argTypes: { - heading: { - description: 'The text for the heading.', - }, - open: { - description: 'Whether the dialog box is active and available for interaction.', - }, - }, decorators: [ (Story) => (
@@ -60,32 +74,30 @@ export const Default: Story = { ], parameters: { docs: { - story: { height: '32em' }, + story: { height: '28rem' }, }, layout: 'fullscreen', }, } -export const FormDialog: Story = { +export const OpenAndClose: Story = { args: { - children: ( -
- - Weet u zeker dat u door wilt gaan met het uitvoeren van deze actie? Dat verwijdert gegevens die nog niet - opgeslagen zijn. - -
- ), - footer: ( - - - - + id: 'ams-dialog', + }, + decorators: [ + (Story) => ( +
+ + +
), + ], +} + +export const AskingToConfirm: Story = { + args: { + children: defaultChildren('ams-dialog-asking-to-confirm-form'), + footer: defaultFooter('ams-dialog-asking-to-confirm-form'), open: true, }, decorators: [ @@ -97,7 +109,7 @@ export const FormDialog: Story = { ], parameters: { docs: { - story: { height: '32em' }, + story: { height: '32rem' }, }, layout: 'fullscreen', }, @@ -105,38 +117,40 @@ export const FormDialog: Story = { export const WithScrollbar: Story = { args: { - children: [ - - Algemeen - , - - De gemeente Amsterdam verwerkt bij de uitvoering van haar taken en verplichtingen persoonsgegevens. De manier - waarop de gemeente Amsterdam om gaat met persoonsgegevens is vastgelegd in het stedelijk kader verwerken - persoonsgegevens. - , - - Deze verklaring geeft aanvullende informatie over de omgang met persoonsgegevens door de gemeente Amsterdam en - over uw mogelijkheden tot het uitoefenen van uw rechten met betrekking tot persoonsgegevens. - , - - Meer specifieke informatie over privacy en de verwerking van persoonsgegevens door de gemeente Amsterdam kunt u - op de hoofdpagina vinden. - , - - Vanwege nieuwe wetgeving of andere ontwikkelingen, past de gemeente regelmatig haar processen aan. Dit kunnen - ook wijzigingen zijn in de wijze van het verwerken van persoonsgegevens. Wij raden u daarom aan om regelmatig - deze pagina te bekijken. Deze pagina wordt doorlopend geactualiseerd. - , - - Geldende wet- en regelgeving en reikwijdte - , - - Vanaf 25 mei 2018 is de Algemene verordening gegevensbescherming (Avg) van toepassing op alle verwerkingen van - persoonsgegevens. Deze Europese wetgeving heeft directe werking in Nederland. Voor die zaken die nationaal - geregeld moeten worden, is de Uitvoeringswet Avg in Nederland aanvullend van toepassing. Deze wetteksten kunt u - vinden op de website van Autoriteit Persoonsgegevens. - , - ], + children: ( + + + Algemeen + + + De gemeente Amsterdam verwerkt bij de uitvoering van haar taken en verplichtingen persoonsgegevens. De manier + waarop de gemeente Amsterdam om gaat met persoonsgegevens is vastgelegd in het stedelijk kader verwerken + persoonsgegevens. + + + Deze verklaring geeft aanvullende informatie over de omgang met persoonsgegevens door de gemeente Amsterdam en + over uw mogelijkheden tot het uitoefenen van uw rechten met betrekking tot persoonsgegevens. + + + Meer specifieke informatie over privacy en de verwerking van persoonsgegevens door de gemeente Amsterdam kunt + u op de hoofdpagina vinden. + + + Vanwege nieuwe wetgeving of andere ontwikkelingen, past de gemeente regelmatig haar processen aan. Dit kunnen + ook wijzigingen zijn in de wijze van het verwerken van persoonsgegevens. Wij raden u daarom aan om regelmatig + deze pagina te bekijken. Deze pagina wordt doorlopend geactualiseerd. + + + Geldende wet- en regelgeving en reikwijdte + + + Vanaf 25 mei 2018 is de Algemene verordening gegevensbescherming (Avg) van toepassing op alle verwerkingen van + persoonsgegevens. Deze Europese wetgeving heeft directe werking in Nederland. Voor die zaken die nationaal + geregeld moeten worden, is de Uitvoeringswet Avg in Nederland aanvullend van toepassing. Deze wetteksten kunt + u vinden op de website van Autoriteit Persoonsgegevens. + + + ), footer: ( @@ -147,7 +161,7 @@ export const WithScrollbar: Story = { }, decorators: [ (Story) => ( -
+
), @@ -160,33 +174,12 @@ export const WithScrollbar: Story = { }, } -export const TriggerButton: Story = { - args: { - id: 'openDialog', - }, - decorators: [ - (Story) => ( -
- - -
- ), - ], -} - export const VerticalButtons: Story = { args: { - children: ( -
- - Weet u zeker dat u door wilt gaan met het uitvoeren van deze actie? Dat verwijdert gegevens die nog niet - opgeslagen zijn. - -
- ), + children: defaultChildren('ams-dialog-vertical-buttons-form'), footer: ( - , ], }, } satisfies Meta @@ -19,3 +20,13 @@ export default meta type Story = StoryObj export const Default: Story = {} + +export const Stacked: Story = { + args: { + children: [, ], + className: 'ams-resize-horizontal', + style: { + inlineSize: '16rem', + }, + }, +} From 678eb8fbcefa0dbbec594507771da2e2dbc61728 Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Thu, 26 Sep 2024 15:16:49 +0200 Subject: [PATCH 10/17] Remove Action Group as subcomponent of Dialog --- .../css/src/components/action-group/README.md | 7 +++ packages/css/src/components/dialog/README.md | 8 +--- .../css/src/components/dialog/dialog.scss | 12 ----- packages/react/src/Dialog/Dialog.tsx | 2 - .../src/Dialog/DialogActionGroup.test.tsx | 41 ----------------- .../react/src/Dialog/DialogActionGroup.tsx | 20 --------- .../src/components/ams/dialog.tokens.json | 3 -- .../src/components/Dialog/Dialog.docs.mdx | 9 +--- .../src/components/Dialog/Dialog.stories.tsx | 44 +++---------------- 9 files changed, 16 insertions(+), 130 deletions(-) delete mode 100644 packages/react/src/Dialog/DialogActionGroup.test.tsx delete mode 100644 packages/react/src/Dialog/DialogActionGroup.tsx diff --git a/packages/css/src/components/action-group/README.md b/packages/css/src/components/action-group/README.md index c53b3fcc83..171fc074d9 100644 --- a/packages/css/src/components/action-group/README.md +++ b/packages/css/src/components/action-group/README.md @@ -3,3 +3,10 @@ # Action Group Groups one or more related actions and manages their layout. + +## How to use + +- If two or more buttons or links are in a row, 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. diff --git a/packages/css/src/components/dialog/README.md b/packages/css/src/components/dialog/README.md index 0feaf32050..b06ccff7bc 100644 --- a/packages/css/src/components/dialog/README.md +++ b/packages/css/src/components/dialog/README.md @@ -9,13 +9,7 @@ A Dialog allows the user to focus on one task or a piece of information by poppi - Use dialogs sparingly because they interrupt the user’s workflow. - 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. +- Wrap multiple buttons in an Action Group. ## Keyboard support diff --git a/packages/css/src/components/dialog/dialog.scss b/packages/css/src/components/dialog/dialog.scss index 10cfb93e51..e293204719 100644 --- a/packages/css/src/components/dialog/dialog.scss +++ b/packages/css/src/components/dialog/dialog.scss @@ -41,15 +41,3 @@ so do not apply these styles without an `open` attribute. */ overflow-y: auto; overscroll-behavior-y: contain; } - -.ams-dialog__action-group { - display: inline-flex; - flex-wrap: wrap; // [1] - gap: var(--ams-dialog-action-group-gap); - - > * { - flex: auto; // [1] - } -} - -// [1] This combination stacks the buttons vertically and stretches them, until they fit next to each other. diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index 718c8fcc26..cc3f384022 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -8,7 +8,6 @@ import { forwardRef, MouseEvent } from 'react' import type { DialogHTMLAttributes, ForwardedRef, PropsWithChildren, ReactNode } from 'react' import { Heading } from '../Heading' import { IconButton } from '../IconButton' -import { DialogActionGroup } from './DialogActionGroup' export type DialogProps = { /** The label for the button that dismisses the Dialog. */ @@ -41,7 +40,6 @@ const DialogRoot = forwardRef( DialogRoot.displayName = 'Dialog' export const Dialog = Object.assign(DialogRoot, { - ActionGroup: DialogActionGroup, close: closeDialog, open: openDialog, }) diff --git a/packages/react/src/Dialog/DialogActionGroup.test.tsx b/packages/react/src/Dialog/DialogActionGroup.test.tsx deleted file mode 100644 index 8aa2c88625..0000000000 --- a/packages/react/src/Dialog/DialogActionGroup.test.tsx +++ /dev/null @@ -1,41 +0,0 @@ -import { render } from '@testing-library/react' -import { createRef } from 'react' -import '@testing-library/jest-dom' -import { Dialog } from './Dialog' - -describe('Dialog Action Group', () => { - it('renders', () => { - const { container } = render() - - const component = container.querySelector(':only-child') - - expect(component).toBeInTheDocument() - expect(component).toBeVisible() - }) - - it('renders a design system BEM class name', () => { - const { container } = render() - - const component = container.querySelector(':only-child') - - expect(component).toHaveClass('ams-dialog__action-group') - }) - - it('renders an additional class name', () => { - const { container } = render() - - const component = container.querySelector(':only-child') - - expect(component).toHaveClass('ams-dialog__action-group extra') - }) - - 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/Dialog/DialogActionGroup.tsx b/packages/react/src/Dialog/DialogActionGroup.tsx deleted file mode 100644 index dbecde6aa0..0000000000 --- a/packages/react/src/Dialog/DialogActionGroup.tsx +++ /dev/null @@ -1,20 +0,0 @@ -/** - * @license EUPL-1.2+ - * Copyright Gemeente Amsterdam - */ - -import clsx from 'clsx' -import { forwardRef } from 'react' -import type { ForwardedRef, HTMLAttributes, PropsWithChildren } from 'react' - -export type DialogActionGroupProps = PropsWithChildren> - -export const DialogActionGroup = forwardRef( - ({ children, className, ...restProps }: DialogActionGroupProps, ref: ForwardedRef) => ( -
- {children} -
- ), -) - -DialogActionGroup.displayName = 'DialogActionGroup' diff --git a/proprietary/tokens/src/components/ams/dialog.tokens.json b/proprietary/tokens/src/components/ams/dialog.tokens.json index 1917982293..c30f51e30e 100644 --- a/proprietary/tokens/src/components/ams/dialog.tokens.json +++ b/proprietary/tokens/src/components/ams/dialog.tokens.json @@ -11,9 +11,6 @@ "padding-inline": { "value": "{ams.space.grid.lg}" }, "header": { "gap": { "value": "{ams.space.md}" } - }, - "action-group": { - "gap": { "value": "{ams.space.md}" } } } } diff --git a/storybook/src/components/Dialog/Dialog.docs.mdx b/storybook/src/components/Dialog/Dialog.docs.mdx index 297c58375b..acac3d905f 100644 --- a/storybook/src/components/Dialog/Dialog.docs.mdx +++ b/storybook/src/components/Dialog/Dialog.docs.mdx @@ -26,7 +26,7 @@ To close the Dialog, use either `Dialog.close`, or a `
` as in the followin Use a `` when asking to confirm an action, e.g. through ‘OK’ and ‘Cancel’ buttons. Add `method="dialog"` to let the browser close the Dialog automatically when the form is submitted. -Wrap one or more buttons in an Action Group and place it in the `footer`. +Wrap one or more buttons in an [Action Group](?path=/docs/components-buttons-action-group--docs) and place it in the `footer`. This ensures correct whitespace and scrolling behaviour. At the same time, it positions the buttons outside the `form` element. Create an `id` for the form and add it to the submit Button’s `form` attribute to connect the two. @@ -43,10 +43,3 @@ For more information, see [Handling the return value from the dialog (MDN)](http Content that doesn’t fit entirely in the Dialog will scroll. - -### 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 when zooming in. - - diff --git a/storybook/src/components/Dialog/Dialog.stories.tsx b/storybook/src/components/Dialog/Dialog.stories.tsx index 3eee667f34..91e53241c0 100644 --- a/storybook/src/components/Dialog/Dialog.stories.tsx +++ b/storybook/src/components/Dialog/Dialog.stories.tsx @@ -3,7 +3,7 @@ * Copyright Gemeente Amsterdam */ -import { Button, Column, Heading, Paragraph } from '@amsterdam/design-system-react' +import { ActionGroup, Button, Column, Heading, Paragraph } from '@amsterdam/design-system-react' import { Dialog } from '@amsterdam/design-system-react/src' import { Meta, StoryObj } from '@storybook/react' @@ -17,14 +17,14 @@ const defaultChildren = (formId: string) => ( ) const defaultFooter = (formId: string) => ( - + - + ) const meta = { @@ -56,11 +56,11 @@ export const Default: Story = { args: { children: De gegevens zijn opgeslagen., footer: ( - + - + ), heading: 'Gelukt', open: true, @@ -152,9 +152,9 @@ export const WithScrollbar: Story = { ), footer: ( - + - + ), heading: 'Privacyverklaring gemeente Amsterdam', open: true, @@ -173,33 +173,3 @@ export const WithScrollbar: Story = { layout: 'fullscreen', }, } - -export const VerticalButtons: Story = { - args: { - children: defaultChildren('ams-dialog-vertical-buttons-form'), - footer: ( - - - - - ), - open: true, - }, - decorators: [ - (Story) => ( -
- -
- ), - ], - parameters: { - docs: { - story: { height: '36rem' }, - }, - layout: 'fullscreen', - }, -} From 778b95a06ce0869720ffce4bb5afa36a587f42ef Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Thu, 26 Sep 2024 15:30:40 +0200 Subject: [PATCH 11/17] Mention Action Group in Button documentation --- packages/css/src/components/button/README.md | 7 +++---- storybook/src/components/Dialog/Dialog.docs.mdx | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/css/src/components/button/README.md b/packages/css/src/components/button/README.md index 435acf9402..a510ed789d 100644 --- a/packages/css/src/components/button/README.md +++ b/packages/css/src/components/button/README.md @@ -6,11 +6,10 @@ Allows the user to perform actions and operate the user interface. ## Guidelines -- A short label text that describes the function of the button. -- A clear contrasting colour scheme so that it is easy to recognize and quickly locate. -- Use the correct type of button for the corresponding application. - For example, a button within a form must always be of the type `submit`. +- Choose a short label that describes the function of the button. +- Use the correct type of button for the corresponding application, e.g. `type="submit"` for the primary form button. - Make sure one can operate a button through a keyboard. +- Wrap 2 or more consecutive buttons in an [Action Group](?path=/docs/components-buttons-action-group--docs). - Primary, secondary and tertiary buttons can be used side by side. Skipping levels is allowed. diff --git a/storybook/src/components/Dialog/Dialog.docs.mdx b/storybook/src/components/Dialog/Dialog.docs.mdx index acac3d905f..b8f20c2820 100644 --- a/storybook/src/components/Dialog/Dialog.docs.mdx +++ b/storybook/src/components/Dialog/Dialog.docs.mdx @@ -26,9 +26,9 @@ To close the Dialog, use either `Dialog.close`, or a `` as in the followin Use a `` when asking to confirm an action, e.g. through ‘OK’ and ‘Cancel’ buttons. Add `method="dialog"` to let the browser close the Dialog automatically when the form is submitted. -Wrap one or more buttons in an [Action Group](?path=/docs/components-buttons-action-group--docs) and place it in the `footer`. +Wrap the buttons in an [Action Group](?path=/docs/components-buttons-action-group--docs) and place it in the `footer`. This ensures correct whitespace and scrolling behaviour. -At the same time, it positions the buttons outside the `form` element. +At the same time, this will position the buttons outside the `form` element. Create an `id` for the form and add it to the submit Button’s `form` attribute to connect the two. If the Action Group needs to live inside of the `form`, add a medium bottom margin (`ams-mb--md`) to the element before it and make scrolling work correctly. From df8dbc6429f8df6c90e9b19fabdc0884af7576c9 Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Thu, 26 Sep 2024 15:35:34 +0200 Subject: [PATCH 12/17] Fix token name --- packages/css/src/components/action-group/action-group.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/css/src/components/action-group/action-group.scss b/packages/css/src/components/action-group/action-group.scss index 1bf7c592bb..c1bf439d4c 100644 --- a/packages/css/src/components/action-group/action-group.scss +++ b/packages/css/src/components/action-group/action-group.scss @@ -6,7 +6,7 @@ .ams-action-group { display: inline-flex; flex-wrap: wrap; - gap: var(--ams-dialog-action-group-gap); + gap: var(--ams-action-group-gap); > * { flex: auto; From 6a0ed8ac73ee378f318c9e66f376749658846637 Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Thu, 26 Sep 2024 15:46:57 +0200 Subject: [PATCH 13/17] Allow Link in Action Group --- .../css/src/components/action-group/README.md | 1 + .../src/components/action-group/action-group.scss | 1 + packages/css/src/components/button/README.md | 2 +- packages/css/src/components/link/README.md | 1 + .../components/ActionGroup/ActionGroup.docs.mdx | 6 ++++++ .../ActionGroup/ActionGroup.stories.tsx | 15 +++++++++++++-- 6 files changed, 23 insertions(+), 3 deletions(-) diff --git a/packages/css/src/components/action-group/README.md b/packages/css/src/components/action-group/README.md index 171fc074d9..7a58c08583 100644 --- a/packages/css/src/components/action-group/README.md +++ b/packages/css/src/components/action-group/README.md @@ -6,6 +6,7 @@ Groups one or more related actions and manages their layout. ## How to use +- Both a [Button](?path=/docs/components-buttons-button--docs) and a [Link](?path=/docs/components-navigation-link--docs) can provide an ‘action’ in this context. - If two or more buttons or links are in a row, 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. diff --git a/packages/css/src/components/action-group/action-group.scss b/packages/css/src/components/action-group/action-group.scss index c1bf439d4c..20e132387b 100644 --- a/packages/css/src/components/action-group/action-group.scss +++ b/packages/css/src/components/action-group/action-group.scss @@ -4,6 +4,7 @@ */ .ams-action-group { + align-items: baseline; display: inline-flex; flex-wrap: wrap; gap: var(--ams-action-group-gap); diff --git a/packages/css/src/components/button/README.md b/packages/css/src/components/button/README.md index a510ed789d..9c95310120 100644 --- a/packages/css/src/components/button/README.md +++ b/packages/css/src/components/button/README.md @@ -9,7 +9,7 @@ Allows the user to perform actions and operate the user interface. - Choose a short label that describes the function of the button. - Use the correct type of button for the corresponding application, e.g. `type="submit"` for the primary form button. - Make sure one can operate a button through a keyboard. -- Wrap 2 or more consecutive buttons in an [Action Group](?path=/docs/components-buttons-action-group--docs). +- Wrap 2 or more consecutive buttons and/or links in an [Action Group](?path=/docs/components-buttons-action-group--docs). - Primary, secondary and tertiary buttons can be used side by side. Skipping levels is allowed. diff --git a/packages/css/src/components/link/README.md b/packages/css/src/components/link/README.md index 4169c6d57c..2dc3e31dc7 100644 --- a/packages/css/src/components/link/README.md +++ b/packages/css/src/components/link/README.md @@ -13,6 +13,7 @@ Use a link in the following cases: - To navigate to another website (see [External links](#external-links)) - To navigate to an element on the same page - To link to emails or phone numbers (start the link with `mailto:` or `tel:`) +- Wrap 2 or more consecutive buttons and/or links in an [Action Group](?path=/docs/components-buttons-action-group--docs). A link is a navigation component. Use a button instead of a link when an action is desired. diff --git a/storybook/src/components/ActionGroup/ActionGroup.docs.mdx b/storybook/src/components/ActionGroup/ActionGroup.docs.mdx index f18615d521..92a574054f 100644 --- a/storybook/src/components/ActionGroup/ActionGroup.docs.mdx +++ b/storybook/src/components/ActionGroup/ActionGroup.docs.mdx @@ -19,3 +19,9 @@ This can occur in a narrow Dialog, with long labels, a large text size, or when Resize the pink rectangle to see this in action. + +### With Link + +An action that involves navigation should be a link. + + diff --git a/storybook/src/components/ActionGroup/ActionGroup.stories.tsx b/storybook/src/components/ActionGroup/ActionGroup.stories.tsx index 2c6bfe34ab..3a3985ab22 100644 --- a/storybook/src/components/ActionGroup/ActionGroup.stories.tsx +++ b/storybook/src/components/ActionGroup/ActionGroup.stories.tsx @@ -3,7 +3,7 @@ * Copyright Gemeente Amsterdam */ -import { Button } from '@amsterdam/design-system-react' +import { Button, Link } from '@amsterdam/design-system-react' import { ActionGroup } from '@amsterdam/design-system-react/src' import { Meta, StoryObj } from '@storybook/react' @@ -23,10 +23,21 @@ export const Default: Story = {} export const Stacked: Story = { args: { - children: [, ], + children: [, ], className: 'ams-resize-horizontal', style: { inlineSize: '16rem', }, }, } + +export const WithLink: Story = { + args: { + children: [ + , + + Downloaden + , + ], + }, +} From fd90a13d1813cabf7527691f67b70ae163fb91d6 Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Thu, 26 Sep 2024 16:14:20 +0200 Subject: [PATCH 14/17] Improve prop description --- packages/react/src/Dialog/Dialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index cc3f384022..705f24c465 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -12,7 +12,7 @@ import { IconButton } from '../IconButton' export type DialogProps = { /** The label for the button that dismisses the Dialog. */ closeButtonLabel?: string - /** Content for the footer, often an Action Group with one or more buttons. */ + /** Content for the footer, often one Button or an Action Group containing more of them. */ footer?: ReactNode /** The text for the Heading. */ heading: string From c6b53e3dc52a7813676c47a8aabb9a680437f5a8 Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Fri, 27 Sep 2024 15:43:31 +0200 Subject: [PATCH 15/17] Fully qualify urls in plain markdown Co-authored-by: Niels Roozemond --- packages/css/src/components/dialog/README.md | 2 +- packages/css/src/components/link/README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/css/src/components/dialog/README.md b/packages/css/src/components/dialog/README.md index b06ccff7bc..00196bd171 100644 --- a/packages/css/src/components/dialog/README.md +++ b/packages/css/src/components/dialog/README.md @@ -9,7 +9,7 @@ A Dialog allows the user to focus on one task or a piece of information by poppi - Use dialogs sparingly because they interrupt the user’s workflow. - Use a dialog for short and non-frequent tasks. Consider using the main flow for regular tasks. -- Wrap multiple buttons in an Action Group. +- Wrap multiple buttons in an [Action Group](https://designsystem.amsterdam/?path=/docs/components-buttons-action-group--docs). ## Keyboard support diff --git a/packages/css/src/components/link/README.md b/packages/css/src/components/link/README.md index 2dc3e31dc7..ec09b2aed6 100644 --- a/packages/css/src/components/link/README.md +++ b/packages/css/src/components/link/README.md @@ -13,7 +13,7 @@ Use a link in the following cases: - To navigate to another website (see [External links](#external-links)) - To navigate to an element on the same page - To link to emails or phone numbers (start the link with `mailto:` or `tel:`) -- Wrap 2 or more consecutive buttons and/or links in an [Action Group](?path=/docs/components-buttons-action-group--docs). +- Wrap 2 or more consecutive buttons and/or links in an [Action Group](https://designsystem.amsterdam/?path=/docs/components-buttons-action-group--docs). A link is a navigation component. Use a button instead of a link when an action is desired. From 8d5a326fc73e8033d0df88f4dfe76d175cceab74 Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Fri, 27 Sep 2024 17:33:34 +0200 Subject: [PATCH 16/17] Add group role --- .../css/src/components/action-group/README.md | 1 + .../react/src/ActionGroup/ActionGroup.test.tsx | 18 +++++++++--------- packages/react/src/ActionGroup/ActionGroup.tsx | 2 +- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/css/src/components/action-group/README.md b/packages/css/src/components/action-group/README.md index 7a58c08583..6f4be7918f 100644 --- a/packages/css/src/components/action-group/README.md +++ b/packages/css/src/components/action-group/README.md @@ -11,3 +11,4 @@ Groups one or more related actions and manages their layout. - 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. +- Replace the default ’group’ role with `role="toolbar"` for button toolbars. diff --git a/packages/react/src/ActionGroup/ActionGroup.test.tsx b/packages/react/src/ActionGroup/ActionGroup.test.tsx index 129f3cf555..9af11a0a67 100644 --- a/packages/react/src/ActionGroup/ActionGroup.test.tsx +++ b/packages/react/src/ActionGroup/ActionGroup.test.tsx @@ -1,30 +1,30 @@ -import { render } from '@testing-library/react' +import { render, screen } from '@testing-library/react' import { createRef } from 'react' import { ActionGroup } from './ActionGroup' import '@testing-library/jest-dom' describe('Action Group', () => { it('renders', () => { - const { container } = render() + render() - const component = container.querySelector(':only-child') + const component = screen.getByRole('group') expect(component).toBeInTheDocument() expect(component).toBeVisible() }) it('renders a design system BEM class name', () => { - const { container } = render() + render() - const component = container.querySelector(':only-child') + const component = screen.getByRole('group') expect(component).toHaveClass('ams-action-group') }) it('renders an additional class name', () => { - const { container } = render() + render() - const component = container.querySelector(':only-child') + const component = screen.getByRole('group') expect(component).toHaveClass('ams-action-group extra') }) @@ -32,9 +32,9 @@ describe('Action Group', () => { it('supports ForwardRef in React', () => { const ref = createRef() - const { container } = render() + render() - const component = container.querySelector(':only-child') + const component = screen.getByRole('group') expect(ref.current).toBe(component) }) diff --git a/packages/react/src/ActionGroup/ActionGroup.tsx b/packages/react/src/ActionGroup/ActionGroup.tsx index d953e53895..348fda9843 100644 --- a/packages/react/src/ActionGroup/ActionGroup.tsx +++ b/packages/react/src/ActionGroup/ActionGroup.tsx @@ -11,7 +11,7 @@ export type ActionGroupProps = PropsWithChildren> export const ActionGroup = forwardRef( ({ children, className, ...restProps }: ActionGroupProps, ref: ForwardedRef) => ( -
+
{children}
), From 03e5eb6c90579388c50560fcdecda08d9d462184 Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Fri, 27 Sep 2024 17:37:31 +0200 Subject: [PATCH 17/17] Clarify docs --- storybook/src/components/Dialog/Dialog.docs.mdx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/storybook/src/components/Dialog/Dialog.docs.mdx b/storybook/src/components/Dialog/Dialog.docs.mdx index b8f20c2820..d6c92406b9 100644 --- a/storybook/src/components/Dialog/Dialog.docs.mdx +++ b/storybook/src/components/Dialog/Dialog.docs.mdx @@ -31,7 +31,9 @@ This ensures correct whitespace and scrolling behaviour. At the same time, this will position the buttons outside the `form` element. Create an `id` for the form and add it to the submit Button’s `form` attribute to connect the two. -If the Action Group needs to live inside of the `form`, add a medium bottom margin (`ams-mb--md`) to the element before it and make scrolling work correctly. +If the Action Group must be in the `form`, implement the whitespace and scrolling behaviour as well. +Add a medium bottom margin (`ams-mb--md`) to the element before it. +Make sure the content of the form scrolls if necessary, while the Action Group is visible at the bottom at all times. The form returns the `value` of the submit Button, which allows inferring which Button the user clicked. For more information, see [Handling the return value from the dialog (MDN)](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dialog#handling_the_return_value_from_the_dialog).