Repository: marigold-ui/marigold
Pull request #2766 (2023-02-14 08:52:11)

feat: add `open` prop to `Dialog.Controller`

changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: 03b81fab68e0cec172f837bdb0ca1213326d9a92

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/master/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/marigold-ui/marigold/new/dialog-open-prop?filename=.changeset/dull-paws-fry.md&value=---%0A%22%40marigold%2Fcomponents%22%3A%20patch%0A---%0A%0Afeat%3A%20add%20%60open%60%20prop%20to%20%60Dialog.Controller%60%0A)



Patch:
diff --git a/.changeset/dull-paws-fry.md b/.changeset/dull-paws-fry.md
new file mode 100644
index 000000000..037359805
--- /dev/null
+++ b/.changeset/dull-paws-fry.md
@@ -0,0 +1,5 @@
+---
+"@marigold/components": minor
+---
+
+feat: add `open` prop to `Dialog.Controller`
diff --git a/.changeset/tough-gifts-whisper.md b/.changeset/tough-gifts-whisper.md
index cf3cfe47a..492657a31 100644
--- a/.changeset/tough-gifts-whisper.md
+++ b/.changeset/tough-gifts-whisper.md
@@ -1,6 +1,6 @@
 ---
-"@marigold/docs": patch
-"@marigold/components": patch
+"@marigold/docs": minor
+"@marigold/components": minor
 ---
 
 feat: update Menu.Trigger with open and onOpenChange property
diff --git a/docs/src/demos/components/Menu/menu-open-dialog.demo.tsx b/docs/src/demos/components/Menu/menu-open-dialog.demo.tsx
index 13526664e..a1dcc905b 100644
--- a/docs/src/demos/components/Menu/menu-open-dialog.demo.tsx
+++ b/docs/src/demos/components/Menu/menu-open-dialog.demo.tsx
@@ -13,18 +13,20 @@ import {
 } from '@marigold/components';
 
 export const OpenDialogFromMenu = () => {
-  const [open, setOpen] = useState(false);
-  const handleAction = (action: string) => {
+  const [open, setDialogOpen] = useState(false);
+  const handleAction = (action: 'save' | 'delete') => {
     switch (action) {
-      case 'two':
-        setOpen(!open);
+      case 'save':
+        alert('saved!');
         break;
-      case 'one':
+      case 'delete':
+        setDialogOpen(true);
         break;
       default:
         throw new Error(`Unhandled action "${action}"!`);
     }
   };
+
   return (
     <>
       <Menu.Trigger>
@@ -32,35 +34,33 @@ export const OpenDialogFromMenu = () => {
           Settings
         </Button>
         <Menu onAction={handleAction}>
-          <Menu.Item key="one">Save</Menu.Item>
-          <Menu.Item key="two">Delete</Menu.Item>
+          <Menu.Item key="save">Save</Menu.Item>
+          <Menu.Item key="delete">Delete</Menu.Item>
         </Menu>
       </Menu.Trigger>
-      <Dialog.Controller onOpenChange={o => setOpen(o)}>
-        {open && (
-          <Dialog closeButton>
-            {({ close, titleProps }) => (
-              <Stack space="medium">
-                <Header>
-                  <Headline {...titleProps}>Confirm delete</Headline>
-                </Header>
-                <Body>
-                  <Text>Do you really wanna delete this?</Text>
-                </Body>
-                <Footer>
-                  <Inline space="medium">
-                    <Button size="small" variant="ghost" onPress={close}>
-                      Abbrechen
-                    </Button>
-                    <Button size="small" variant="primary">
-                      Delete
-                    </Button>
-                  </Inline>
-                </Footer>
-              </Stack>
-            )}
-          </Dialog>
-        )}
+      <Dialog.Controller open={open} onOpenChange={setDialogOpen}>
+        <Dialog closeButton>
+          {({ close, titleProps }) => (
+            <Stack space="medium">
+              <Header>
+                <Headline {...titleProps}>Confirm delete</Headline>
+              </Header>
+              <Body>
+                <Text>Do you really wanna delete this?</Text>
+              </Body>
+              <Footer>
+                <Inline space="medium">
+                  <Button size="small" variant="ghost" onPress={close}>
+                    Cancel
+                  </Button>
+                  <Button size="small" variant="primary" onPress={close}>
+                    Delete
+                  </Button>
+                </Inline>
+              </Footer>
+            </Stack>
+          )}
+        </Dialog>
       </Dialog.Controller>
     </>
   );
diff --git a/packages/components/src/Dialog/Dialog.stories.tsx b/packages/components/src/Dialog/Dialog.stories.tsx
index 7b8d31047..d9ecf9529 100644
--- a/packages/components/src/Dialog/Dialog.stories.tsx
+++ b/packages/components/src/Dialog/Dialog.stories.tsx
@@ -163,12 +163,17 @@ export const StickyFooter: ComponentStory<typeof Dialog.Trigger> = args => (
   </Dialog.Trigger>
 );
 
-export const FromMenu: ComponentStory<typeof Dialog.Trigger> = args => {
-  const [open, setOpen] = useState(false);
-  const handleAction = (action: string) => {
+export const WithDialogController: ComponentStory<
+  typeof Dialog.Trigger
+> = () => {
+  const [open, setDialogOpen] = useState(false);
+  const handleAction = (action: 'save' | 'delete') => {
     switch (action) {
-      case 'two':
-        setOpen(!open);
+      case 'save':
+        alert('saved!');
+        break;
+      case 'delete':
+        setDialogOpen(true);
         break;
       default:
         throw new Error(`Unhandled action "${action}"!`);
@@ -182,35 +187,33 @@ export const FromMenu: ComponentStory<typeof Dialog.Trigger> = args => {
           Settings
         </Button>
         <Menu onAction={handleAction}>
-          <Menu.Item key="one">Save</Menu.Item>
-          <Menu.Item key="two">Delete</Menu.Item>
+          <Menu.Item key="save">Save</Menu.Item>
+          <Menu.Item key="delete">Delete</Menu.Item>
         </Menu>
       </Menu.Trigger>
-      <Dialog.Controller onOpenChange={o => setOpen(o)}>
-        {open && (
-          <Dialog closeButton>
-            {({ close, titleProps }) => (
-              <Stack space="medium">
-                <Header>
-                  <Headline {...titleProps}>Confirm delete</Headline>
-                </Header>
-                <Body>
-                  <Text>Do you really wanna delete this?</Text>
-                </Body>
-                <Footer>
-                  <Inline space="medium">
-                    <Button size="small" variant="ghost" onPress={close}>
-                      Abbrechen
-                    </Button>
-                    <Button size="small" variant="primary">
-                      Delete
-                    </Button>
-                  </Inline>
-                </Footer>
-              </Stack>
-            )}
-          </Dialog>
-        )}
+      <Dialog.Controller open={open} onOpenChange={setDialogOpen}>
+        <Dialog closeButton>
+          {({ close, titleProps }) => (
+            <Stack space="medium">
+              <Header>
+                <Headline {...titleProps}>Confirm delete</Headline>
+              </Header>
+              <Body>
+                <Text>Do you really wanna delete this?</Text>
+              </Body>
+              <Footer>
+                <Inline space="medium">
+                  <Button size="small" variant="ghost" onPress={close}>
+                    Cancel
+                  </Button>
+                  <Button size="small" variant="primary" onPress={close}>
+                    Delete
+                  </Button>
+                </Inline>
+              </Footer>
+            </Stack>
+          )}
+        </Dialog>
       </Dialog.Controller>
     </>
   );
diff --git a/packages/components/src/Dialog/DialogController.tsx b/packages/components/src/Dialog/DialogController.tsx
index b162b854e..2bd51b70f 100644
--- a/packages/components/src/Dialog/DialogController.tsx
+++ b/packages/components/src/Dialog/DialogController.tsx
@@ -1,5 +1,5 @@
 import { useOverlayTriggerState } from '@react-stately/overlays';
-import React, { ReactElement, ReactNode, useRef } from 'react';
+import React, { ReactNode } from 'react';
 import { Modal, Overlay } from '../Overlay';
 import { DialogContext } from './Context';
 
@@ -15,21 +15,11 @@ export const DialogController = ({
   children,
   dismissable = true,
   keyboardDismissable = true,
+  open,
   onOpenChange,
 }: DialogControllerProps) => {
-  const childArray = React.Children.toArray(children);
-  if (childArray.length > 1) {
-    throw new Error('Only a single child can be passed to DialogController.');
-  }
-
-  const lastChild = useRef<ReactElement | null>(null);
-  const child = React.isValidElement(childArray[0]) ? childArray[0] : null;
-  if (child) {
-    lastChild.current = child;
-  }
-
   const state = useOverlayTriggerState({
-    isOpen: !!child,
+    isOpen: open,
     onOpenChange,
   });
 
@@ -44,7 +34,7 @@ export const DialogController = ({
           dismissable={dismissable}
           keyboardDismissable={keyboardDismissable}
         >
-          {lastChild.current}
+          {children}
         </Modal>
       </Overlay>
     </DialogContext.Provider>


Test patch:
diff --git a/packages/components/src/Dialog/Dialog.test.tsx b/packages/components/src/Dialog/Dialog.test.tsx
index 9ec33cc5d..a73d31fc1 100644
--- a/packages/components/src/Dialog/Dialog.test.tsx
+++ b/packages/components/src/Dialog/Dialog.test.tsx
@@ -1,6 +1,6 @@
 /* eslint-disable testing-library/no-node-access */
 /* eslint-disable testing-library/no-container */
-import React from 'react';
+import React, { useState } from 'react';
 import { fireEvent, render, screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 
@@ -432,7 +432,7 @@ test('dialog supports size', () => {
 test('renders with dialog controller', () => {
   render(
     <OverlayProvider>
-      <Dialog.Controller>
+      <Dialog.Controller open>
         <Dialog>
           <Headline>Headline</Headline>Content
         </Dialog>
@@ -441,17 +441,13 @@ test('renders with dialog controller', () => {
   );
 
   const dialog = screen.getByRole('dialog');
-  expect(dialog).toBeVisible();
+  expect(dialog).toBeInTheDocument();
 });
 
-test('onOpenChange has been called in dialog controller', () => {
-  const setState = jest.fn();
-  const useStateSpy = jest.spyOn(React, 'useState');
-  // eslint-disable-next-line no-sparse-arrays
-  useStateSpy.mockImplementation(() => [, setState]);
+test('renders nothing by default', () => {
   render(
     <OverlayProvider>
-      <Dialog.Controller onOpenChange={a => setState(a)}>
+      <Dialog.Controller>
         <Dialog>
           <Headline>Headline</Headline>Content
         </Dialog>
@@ -459,58 +455,93 @@ test('onOpenChange has been called in dialog controller', () => {
     </OverlayProvider>
   );
 
-  expect(setState).toHaveBeenCalled();
+  const dialog = screen.queryByRole('dialog');
+  expect(dialog).not.toBeInTheDocument();
 });
 
-test('dialog controller accepts only one child', () => {
-  render(
-    <OverlayProvider>
-      <Dialog.Controller>
-        <Dialog>
-          <Headline>Headline</Headline>Content
-        </Dialog>
-      </Dialog.Controller>
-    </OverlayProvider>
-  );
+test('dialog can be controlled', async () => {
+  const Component = () => {
+    const [open, setOpen] = useState(false);
 
-  const dialog = screen.getByRole('dialog');
-  expect(dialog).toBeVisible();
+    return (
+      <OverlayProvider>
+        <Button data-testid="button" onPress={() => setOpen(true)}>
+          Open Dialog
+        </Button>
+        <Dialog.Controller open={open}>
+          <Dialog>
+            <Headline>Headline</Headline>
+            <Button data-testid="close" onPress={() => setOpen(false)}>
+              Close
+            </Button>
+          </Dialog>
+        </Dialog.Controller>
+      </OverlayProvider>
+    );
+  };
+
+  render(<Component />);
+
+  let dialog = screen.queryByRole('dialog');
+  expect(dialog).not.toBeInTheDocument();
 
-  const parent = dialog.parentElement;
-  expect(parent?.children).toHaveLength(1);
-  expect(parent?.children).not.toHaveLength(2);
+  const button = screen.getByTestId('button');
+  await user.click(button);
 
-  expect(parent).toBeValid();
-  expect(dialog).toBeValid();
+  dialog = screen.queryByRole('dialog');
+  expect(dialog).toBeInTheDocument();
+
+  const close = screen.getByTestId('close');
+  await user.click(close);
+
+  dialog = screen.queryByRole('dialog');
+  expect(dialog).not.toBeInTheDocument();
 });
 
-test('dialog controller throw errof if not one child', () => {
-  expect(() => {
-    render(
+test('close state has a listener', async () => {
+  const spy = jest.fn();
+
+  const Component = () => {
+    const [open, setOpen] = useState(false);
+
+    return (
       <OverlayProvider>
-        <Dialog.Controller>
-          <Dialog>Content</Dialog>
-          <div>something</div>
+        <Button data-testid="button" onPress={() => setOpen(true)}>
+          Open Dialog
+        </Button>
+        <Dialog.Controller open={open} onOpenChange={spy}>
+          <Dialog>
+            {({ close }) => (
+              <>
+                <Headline>Headline</Headline>
+                <Button data-testid="close" onPress={close}>
+                  Close
+                </Button>
+              </>
+            )}
+          </Dialog>
         </Dialog.Controller>
       </OverlayProvider>
     );
-  }).toThrow(Error);
-  expect(errorMock).toHaveBeenCalled();
-});
+  };
 
-test('dialog controller expect no valid child', () => {
-  const children = null;
-  const { container } = render(
-    <OverlayProvider>
-      <Dialog.Controller data-testid="dialog">{children}</Dialog.Controller>
-    </OverlayProvider>
-  );
+  render(<Component />);
+
+  expect(spy.mock.calls).toMatchInlineSnapshot(`[]`);
+
+  const button = screen.getByTestId('button');
+  await user.click(button);
+
+  expect(spy.mock.calls).toMatchInlineSnapshot(`[]`);
 
-  expect(
-    container.querySelector(`div[data-overlay-container="true"]`)?.childNodes
-  ).toHaveLength(0);
+  const close = screen.getByTestId('close');
+  await user.click(close);
 
-  expect(
-    container.querySelector(`div[data-overlay-container="true"]`)?.lastChild
-  ).toBeNull();
+  expect(spy.mock.calls).toMatchInlineSnapshot(`
+    [
+      [
+        false,
+      ],
+    ]
+  `);
 });