From 4fb6e53441f3bae914eb483da2f0537a65273fd2 Mon Sep 17 00:00:00 2001
From: Niels Roozemond <n.roozemond@amsterdam.nl>
Date: Tue, 17 Sep 2024 10:50:16 +0200
Subject: [PATCH] feat!: Remove form from Dialog (#1582)

Co-authored-by: Aram <37216945+alimpens@users.noreply.github.com>
---
 .../css/src/components/dialog/dialog.scss     | 15 ++--
 packages/react/src/Dialog/Dialog.test.tsx     | 12 +--
 packages/react/src/Dialog/Dialog.tsx          | 12 +--
 .../src/components/ams/dialog.tokens.json     |  4 +-
 .../src/components/Dialog/Dialog.docs.mdx     | 13 ++-
 .../src/components/Dialog/Dialog.stories.tsx  | 80 ++++++++++++++-----
 6 files changed, 96 insertions(+), 40 deletions(-)

diff --git a/packages/css/src/components/dialog/dialog.scss b/packages/css/src/components/dialog/dialog.scss
index 4d074151b8..caa0653823 100644
--- a/packages/css/src/components/dialog/dialog.scss
+++ b/packages/css/src/components/dialog/dialog.scss
@@ -13,7 +13,6 @@
   background-color: var(--ams-dialog-background-color);
   border: var(--ams-dialog-border);
   inline-size: var(--ams-dialog-inline-size);
-  max-block-size: var(--ams-dialog-max-block-size);
   max-inline-size: var(--ams-dialog-max-inline-size);
 
   @include reset-dialog;
@@ -24,16 +23,16 @@
   }
 }
 
-.ams-dialog__form {
+.ams-dialog__wrapper {
   display: grid;
-  gap: var(--ams-dialog-form-gap);
-  padding-block: var(--ams-dialog-form-padding-block);
-  padding-inline: var(--ams-dialog-form-padding-inline);
+  gap: var(--ams-dialog-wrapper-gap);
+  grid-template-rows: auto 1fr auto;
+  max-block-size: var(--ams-dialog-wrapper-max-block-size);
+  padding-block: var(--ams-dialog-wrapper-padding-block);
+  padding-inline: var(--ams-dialog-wrapper-padding-inline);
 }
 
-.ams-dialog__article {
-  display: grid;
-  gap: var(--ams-space-md); /* Until we have a consistent way of spacing text elements */
+.ams-dialog__content {
   max-block-size: 100%; /* Safari */
   overflow-y: auto;
   overscroll-behavior-y: contain;
diff --git a/packages/react/src/Dialog/Dialog.test.tsx b/packages/react/src/Dialog/Dialog.test.tsx
index 71a7beb457..f32dee8bba 100644
--- a/packages/react/src/Dialog/Dialog.test.tsx
+++ b/packages/react/src/Dialog/Dialog.test.tsx
@@ -66,16 +66,18 @@ describe('Dialog', () => {
     expect(getByText('Test content')).toBeInTheDocument()
   })
 
-  it('renders actions when provided', () => {
-    const { getByText } = render(<Dialog heading="Test heading" actions={<button>Click Me</button>} />)
+  it('renders footer when provided', () => {
+    const { getByText } = render(<Dialog heading="Test heading" footer={<button>Click Me</button>} />)
 
     expect(getByText('Click Me')).toBeInTheDocument()
   })
 
-  it('does not render actions when not provided', () => {
-    const { queryByText } = render(<Dialog heading="Test heading" />)
+  it('does not render footer when not provided', () => {
+    const { container } = render(<Dialog heading="Test heading" />)
 
-    expect(queryByText('Click Me')).not.toBeInTheDocument()
+    const component = container.querySelector('footer')
+
+    expect(component).not.toBeInTheDocument()
   })
 
   it('renders DialogClose button', () => {
diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx
index 275641539f..795c6d837f 100644
--- a/packages/react/src/Dialog/Dialog.tsx
+++ b/packages/react/src/Dialog/Dialog.tsx
@@ -11,7 +11,7 @@ import { IconButton } from '../IconButton'
 
 export type DialogProps = {
   /** The button(s) in the footer. Start with a primary button. */
-  actions?: ReactNode
+  footer?: ReactNode
   /** The label for the button that dismisses the Dialog. */
   closeButtonLabel?: string
   /** The text for the Heading. */
@@ -23,18 +23,18 @@ const openDialog = (id: string) => (document.querySelector(id) as HTMLDialogElem
 
 const DialogRoot = forwardRef(
   (
-    { actions, children, className, closeButtonLabel = 'Sluiten', heading, ...restProps }: DialogProps,
+    { footer, children, className, closeButtonLabel = 'Sluiten', heading, ...restProps }: DialogProps,
     ref: ForwardedRef<HTMLDialogElement>,
   ) => (
     <dialog {...restProps} ref={ref} className={clsx('ams-dialog', className)}>
-      <form className="ams-dialog__form" method="dialog">
+      <div className="ams-dialog__wrapper">
         <header className="ams-dialog__header">
           <Heading size="level-4">{heading}</Heading>
           <IconButton label={closeButtonLabel} onClick={closeDialog} size="level-4" type="button" />
         </header>
-        <article className="ams-dialog__article">{children}</article>
-        {actions && <footer className="ams-dialog__footer">{actions}</footer>}
-      </form>
+        <div className="ams-dialog__content">{children}</div>
+        {footer && <footer className="ams-dialog__footer">{footer}</footer>}
+      </div>
     </dialog>
   ),
 )
diff --git a/proprietary/tokens/src/components/ams/dialog.tokens.json b/proprietary/tokens/src/components/ams/dialog.tokens.json
index f30655d97f..054ea0cd88 100644
--- a/proprietary/tokens/src/components/ams/dialog.tokens.json
+++ b/proprietary/tokens/src/components/ams/dialog.tokens.json
@@ -4,10 +4,10 @@
       "background-color": { "value": "{ams.color.primary-white}" },
       "border": { "value": "0" },
       "inline-size": { "value": "calc(100% - 2 * {ams.space.grid.md})" },
-      "max-block-size": { "value": "calc(100% - 2 * {ams.space.grid.md})" },
       "max-inline-size": { "value": "48rem" },
-      "form": {
+      "wrapper": {
         "gap": { "value": "{ams.space.md}" },
+        "max-block-size": { "value": "calc(100dvh - 4 * {ams.space.grid.md})" },
         "padding-block": { "value": "{ams.space.grid.md}" },
         "padding-inline": { "value": "{ams.space.grid.lg}" }
       },
diff --git a/storybook/src/components/Dialog/Dialog.docs.mdx b/storybook/src/components/Dialog/Dialog.docs.mdx
index 60625eaa5b..5b2f8b1ca0 100644
--- a/storybook/src/components/Dialog/Dialog.docs.mdx
+++ b/storybook/src/components/Dialog/Dialog.docs.mdx
@@ -14,11 +14,22 @@ import README from "../../../../packages/css/src/components/dialog/README.md?raw
 
 ## Examples
 
+### Form in a Dialog
+
+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.
+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).
+
+<Canvas of={DialogStories.FormDialog} />
+
 ### With scrollbar
 
 Content taller than the dialog itself will scroll.
 
-<Canvas of={DialogStories.WithScrollbar} className="ams-dialog-story" />
+<Canvas of={DialogStories.WithScrollbar} />
 
 ### Trigger Button
 
diff --git a/storybook/src/components/Dialog/Dialog.stories.tsx b/storybook/src/components/Dialog/Dialog.stories.tsx
index 28117efbac..4571f5a95b 100644
--- a/storybook/src/components/Dialog/Dialog.stories.tsx
+++ b/storybook/src/components/Dialog/Dialog.stories.tsx
@@ -11,13 +11,10 @@ const meta = {
   title: 'Components/Containers/Dialog',
   component: Dialog,
   args: {
-    actions: (
-      <>
-        <Button type="submit">Doorgaan</Button>
-        <Button onClick={Dialog.close} variant="tertiary">
-          Stoppen
-        </Button>
-      </>
+    footer: (
+      <Button onClick={Dialog.close} variant="primary">
+        Sluiten
+      </Button>
     ),
     children: (
       <Paragraph>
@@ -28,7 +25,7 @@ const meta = {
     heading: 'Niet alle gegevens zijn opgeslagen',
   },
   argTypes: {
-    actions: {
+    footer: {
       table: { disable: true },
     },
   },
@@ -65,35 +62,72 @@ export const Default: Story = {
   },
 }
 
+export const FormDialog: Story = {
+  args: {
+    open: true,
+    footer: (
+      <>
+        <Button type="submit" form="dialog1" value="continue">
+          Doorgaan
+        </Button>
+        <Button onClick={Dialog.close} variant="tertiary">
+          Stoppen
+        </Button>
+      </>
+    ),
+    children: (
+      <form method="dialog" id="dialog1">
+        <Paragraph>
+          Weet u zeker dat u door wilt gaan met het uitvoeren van deze actie? Dat verwijdert gegevens die nog niet
+          opgeslagen zijn.
+        </Paragraph>
+      </form>
+    ),
+  },
+  decorators: [
+    (Story) => (
+      <div style={{ backgroundColor: '#0006', position: 'absolute', width: '100%', height: '100%' }}>
+        <Story />
+      </div>
+    ),
+  ],
+  parameters: {
+    docs: {
+      story: { height: '32em' },
+    },
+    layout: 'fullscreen',
+  },
+}
+
 export const WithScrollbar: Story = {
   args: {
-    actions: <Button onClick={Dialog.close}>Sluiten</Button>,
+    footer: <Button onClick={Dialog.close}>Sluiten</Button>,
     children: [
-      <Heading level={2} size="level-5" key={1}>
+      <Heading level={2} size="level-5" key={1} className="ams-mb--sm">
         Algemeen
       </Heading>,
-      <Paragraph key={2}>
+      <Paragraph key={2} className="ams-mb--sm">
         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.
       </Paragraph>,
-      <Paragraph key={3}>
+      <Paragraph key={3} className="ams-mb--sm">
         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.
       </Paragraph>,
-      <Paragraph key={4}>
+      <Paragraph key={4} className="ams-mb--sm">
         Meer specifieke informatie over privacy en de verwerking van persoonsgegevens door de gemeente Amsterdam kunt u
         op de hoofdpagina vinden.
       </Paragraph>,
-      <Paragraph key={5}>
+      <Paragraph key={5} className="ams-mb--sm">
         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.
       </Paragraph>,
-      <Heading level={2} size="level-5" key={6}>
+      <Heading level={2} size="level-5" key={6} className="ams-mb--sm">
         Geldende wet- en regelgeving en reikwijdte
       </Heading>,
-      <Paragraph key={7}>
+      <Paragraph key={7} className="ams-mb--sm">
         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
@@ -134,14 +168,24 @@ export const TriggerButton: Story = {
 
 export const VerticalButtons: Story = {
   args: {
-    actions: (
+    footer: (
       <>
-        <Button type="submit">Lange teksten op deze knoppen</Button>
+        <Button type="submit" form="dialog2">
+          Lange teksten op deze knoppen
+        </Button>
         <Button onClick={Dialog.close} variant="tertiary">
           Om verticaal stapelen te demonstreren
         </Button>
       </>
     ),
+    children: (
+      <form method="dialog" id="dialog2">
+        <Paragraph>
+          Weet u zeker dat u door wilt gaan met het uitvoeren van deze actie? Dat verwijdert gegevens die nog niet
+          opgeslagen zijn.
+        </Paragraph>
+      </form>
+    ),
     open: true,
   },
   decorators: [