Replace Modal.isOpen with controlled boolean (#422)

Author: mcous

packages/core/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@viamrobotics/prime-core",
-  "version": "0.0.58",
+  "version": "0.0.59",
   "publishConfig": {
     "access": "public"
   },

packages/core/src/lib/__tests__/modal.spec.ts

@@ -1,63 +1,81 @@
-import { describe, it, expect, beforeEach } from 'vitest';
+import { describe, it, expect, vi } from 'vitest';
 import { render, screen, within } from '@testing-library/svelte';
 import userEvent from '@testing-library/user-event';
+import type { ComponentProps } from 'svelte';
 import Modal from '../modal.svelte';
-import { writable, get } from 'svelte/store';
 
 describe('Modal', () => {
-  let isOpen = writable(true);
+  const onClose = vi.fn();
 
-  beforeEach(() => {
-    isOpen = writable(true);
+  const renderSubject = (props: ComponentProps<Modal>) => {
+    const { component } = render(Modal, props);
+    component.$on('close', onClose);
+  };
+
+  it('should be visible if open is true ', () => {
+    renderSubject({ isOpen: true });
+
+    const modal = screen.queryByRole('dialog');
+
+    expect(modal).toBeInTheDocument();
+    expect(modal).toHaveAttribute('aria-modal', 'true');
+    expect(onClose).not.toHaveBeenCalled();
+  });
+
+  it('should not be visible if open is false', () => {
+    renderSubject({ isOpen: false });
+
+    const modal = screen.queryByRole('dialog');
+
+    expect(modal).not.toBeInTheDocument();
+    expect(onClose).not.toHaveBeenCalled();
   });
 
   it('should close modal when close icon button is clicked', async () => {
-    render(Modal, { isOpen });
     const user = userEvent.setup();
+    renderSubject({ isOpen: true });
 
     const modal = screen.getByRole('dialog');
     const closeButton = within(modal).getByRole('button', { name: /close/iu });
 
     await user.click(closeButton);
-
-    expect(get(isOpen)).toBe(false);
+    expect(onClose).toHaveBeenCalledOnce();
   });
 
   it('should close modal when clicked outside the modal', async () => {
-    render(Modal, { isOpen });
     const user = userEvent.setup();
+    renderSubject({ isOpen: true });
 
     const modal = screen.getByRole('dialog');
     await user.click(modal.parentElement!);
 
-    expect(get(isOpen)).toBe(false);
+    expect(onClose).toHaveBeenCalledOnce();
   });
 
-  it('if open is true, modal should be visible', () => {
-    render(Modal, { isOpen });
-    const modal = screen.queryByRole('dialog');
-    expect(modal).toBeInTheDocument();
-    expect(modal).toHaveAttribute('aria-modal', 'true');
-  });
+  it('should close modal when escape key is pressed', async () => {
+    const user = userEvent.setup();
+    renderSubject({ isOpen: true });
 
-  it('if open is false, modal should not be visible', () => {
-    isOpen.set(false);
-    render(Modal, { isOpen });
-    const modal = screen.queryByRole('dialog');
-    expect(modal).not.toBeInTheDocument();
+    await user.keyboard('{Escape}');
+
+    expect(onClose).toHaveBeenCalledOnce();
   });
 
-  it('should close modal when escape key is pressed', async () => {
-    render(Modal, { isOpen });
+  it('should not emit close events on escape if the modal is closed', async () => {
     const user = userEvent.setup();
+    renderSubject({ isOpen: false });
+
     await user.keyboard('{Escape}');
-    expect(get(isOpen)).toBe(false);
+
+    expect(onClose).not.toHaveBeenCalled();
   });
 
   it('should focus on heading element on mount', () => {
-    render(Modal, { isOpen });
+    render(Modal, { isOpen: true });
+
     const modal = screen.getByRole('dialog');
     const heading = within(modal).getByRole('heading');
+
     expect(heading).toHaveFocus();
   });
 });

packages/core/src/lib/modal.svelte

@@ -4,50 +4,62 @@
 Creates a modal overlay.
 
 ```svelte
-    <div>
-      <Button on:click={handleOpenModal}>Open Modal</Button>
-      <Modal open={modalOpen} on:close={handleCloseModal}>
-        <span slot="title">This is the modal demo</span>
-        <span slot="message">Are you sure you want to print a statement to the console?</span>
-        <Button
-          slot="primary"
-          on:click={() => console.log('statement')}
-        >
-          Print
-        </Button>
-        <Button slot="secondary" variant="dark" on:click={handleCloseModal}>
-          Cancel
-        </Button>
-      </Modal>
-    </div>
+<script lang="ts">
+  let isOpen = false
+  const handleOpen = () => (isOpen = true)
+  const handleClose = () => (isOpen = false)
+</script>
+
+<div>
+  <Button on:click={handleOpen}>
+    Open Modal
+  </Button>
+  <Modal {isOpen} on:close={handleClose}>
+    <svelte:fragment slot="title">
+      This is the modal demo
+    </svelte:fragment>
+    <svelte:fragment slot="message">
+      Are you sure you want to print a statement to the console?
+    </svelte:fragment>
+    <Button
+      slot="primary"
+      on:click={() => console.log('statement')}
+    >
+      Print
+    </Button>
+    <Button
+      slot="secondary"
+      variant="dark"
+      on:click={handleClose}
+    >
+      Cancel
+    </Button>
+  </Modal>
+</div>
 ```
 -->
 <svelte:options immutable />
 
 <script lang="ts">
 import cx from 'classnames';
+import { createEventDispatcher } from 'svelte';
 import IconButton from './button/icon-button.svelte';
 import { clickOutside } from '$lib';
-import type { Writable } from 'svelte/store';
 
 /** Whether the modal is open. */
-export let isOpen: Writable<boolean>;
+export let isOpen: boolean;
 
-$: if (typeof document !== 'undefined') {
-  document.body.classList.toggle('overflow-hidden', $isOpen);
-}
+/** The variant of the modal. */
+export let variant: 'small' | '' = '';
 
 let headingElement: HTMLElement | undefined;
 
-$: headingElement?.focus();
-
-/** The variant of the modal. */
-export let variant: 'small' | '' = '';
+const dispatch = createEventDispatcher<{
+  close: undefined;
+}>();
 
 const handleCloseModal = () => {
-  if ($isOpen) {
-    isOpen.set(false);
-  }
+  dispatch('close');
 };
 
 const handleEscapePress = (event: KeyboardEvent) => {
@@ -56,11 +68,17 @@ const handleEscapePress = (event: KeyboardEvent) => {
     handleCloseModal();
   }
 };
+
+$: if (typeof document !== 'undefined') {
+  document.body.classList.toggle('overflow-hidden', isOpen);
+}
+
+$: headingElement?.focus();
 </script>
 
-<svelte:window on:keydown={handleEscapePress} />
+<svelte:window on:keydown={isOpen ? handleEscapePress : undefined} />
 
-{#if $isOpen}
+{#if isOpen}
   <div
     class="fixed left-0 top-0 z-50 flex h-full w-full items-center justify-center bg-black bg-opacity-40"
     role="dialog"

packages/core/src/routes/+page.svelte

@@ -42,25 +42,25 @@ import {
   CodeSnippet,
 } from '$lib';
 import { uniqueId } from 'lodash';
-import { writable } from 'svelte/store';
 
 provideNotify();
 
 let buttonClickedTimes = 0;
 let preventHandlerDisabled = true;
-const modalOpen = writable(false);
+let modalOpen = false;
 
 const handleTogglePreventHandler = (event: CustomEvent<boolean>) => {
   preventHandlerDisabled = event.detail;
 };
 
 const handleCloseModal = () => {
-  modalOpen.set(false);
+  modalOpen = false;
 };
 
 const handleOpenModal = () => {
-  modalOpen.set(true);
+  modalOpen = true;
 };
+
 const notify = useNotify();
 
 let restrictedValue = '';
@@ -114,28 +114,28 @@ const jsSnippet = `
  */
 function fizzBuzz(n) {
     const result = [];
- 
+
     for (let i = 1; i <= n; i++) {
         let output = "";
- 
+
         if (i % 3 === 0) {
             output += "Fizz";
         }
- 
+
         if (i % 5 === 0) {
             output += "Buzz";
-        } 
- 
+        }
+
         if (output === "") {
             output = i.toString();
         }
- 
+
         result.push(output);
     }
- 
+
     return result;
 }
- 
+
 // Usage Example
 const fizzBuzzSequence = fizzBuzz(15);
 console.log(fizzBuzzSequence);`.trim();
@@ -181,7 +181,7 @@ console.log(sequence); // Outputs: ["1", "2", "Fizz", "4", "Buzz", "Fizz", "7",
 
 const goSnippet = `
 import "fmt"
- 
+
 // FizzBuzz
 //
 // Parameters:
@@ -191,7 +191,7 @@ import "fmt"
 // []string: A slice of strings containing the FizzBuzz results for each number from 1 to n.
 func FizzBuzz(n int) []string {
   result := make([]string, n)
- 
+
   for i := 1; i <= n; i++ {
     if i%3 == 0 && i%5 == 0 {
       result[i-1] = "FizzBuzz"
@@ -203,16 +203,16 @@ func FizzBuzz(n int) []string {
       result[i-1] = fmt.Sprintf("%d", i)
     }
   }
- 
+
   return result
 }
- 
+
 // Usage Example for FizzBuzz
- 
+
 func main() {
   // Apply FizzBuzz algorithm up to 20
   fizzBuzzResult := FizzBuzz(20)
- 
+
   // Print the FizzBuzz results
   for _, value := range fizzBuzzResult {
     fmt.Println(value)
@@ -223,27 +223,27 @@ const pythonSnippet = `
 def fizzbuzz(n: int):
     """
     Function to implement the FizzBuzz algorithm.
- 
+
     Parameters:
     - n: int
         The number up to which the FizzBuzz algorithm should be applied.
- 
+
     Returns:
     - list:
         A list of strings representing the FizzBuzz sequence from 1 to n.
- 
+
     Raises:
     - ValueError:
         Will raise an error if the input number 'n' is less than 1.
     """
- 
+
     # Validating the input number
     if n < 1:
         raise ValueError("Input number should be greater than or equal to 1.")
- 
+
     # Initializing an empty list to store the FizzBuzz sequence
     fizzbuzz_sequence = []
- 
+
     # Looping through numbers from 1 to n (inclusive)
     for i in range(1, n+1):
         # Checking if the number is divisible by both 3 and 5
@@ -258,9 +258,9 @@ def fizzbuzz(n: int):
         # If none of the above conditions are met, add the number itself
         else:
             fizzbuzz_sequence.append(str(i))
- 
+
     return fizzbuzz_sequence
- 
+
 # Example usage of the fizzbuzz function
 n = 20
 result = fizzbuzz(n)
@@ -269,41 +269,41 @@ print(result)`.trim();
 const cppSnippet = `
 #include <iostream>
 #include <string>
- 
+
 /**
  * @brief Implements the FizzBuzz algorithm.
- * 
+ *
  * The FizzBuzz algorithm is a common programming task where you iterate over a range of numbers
  * and print "Fizz" for numbers divisible by 3, "Buzz" for numbers divisible by 5, and "FizzBuzz"
  * for numbers divisible by both 3 and 5. For all other numbers, the number itself is printed.
- * 
+ *
  * @param n The number of iterations to perform.
  */
 void fizzBuzz(int n) {
     for (int i = 1; i <= n; i++) {
         std::string output = "";
- 
+
         if (i % 3 == 0) {
             output += "Fizz";
         }
- 
+
         if (i % 5 == 0) {
             output += "Buzz";
         }
- 
+
         if (output.empty()) {
             output = std::to_string(i);
         }
- 
+
         std::cout << output << std::endl;
     }
 }
- 
+
 int main() {
     int n = 100; // Number of iterations
- 
+
     fizzBuzz(n);
- 
+
     return 0;
 }`.trim();
 
@@ -1020,7 +1020,10 @@ const htmlSnippet = `
 
   <div>
     <Button on:click={handleOpenModal}>Open Modal</Button>
-    <Modal isOpen={modalOpen}>
+    <Modal
+      isOpen={modalOpen}
+      on:close={handleCloseModal}
+    >
       <span slot="title">This is the modal demo</span>
       <span slot="message"
         >Are you sure you want to kick off a notify toast?</span

packages/storybook/.eslintrc.cjs

@@ -18,4 +18,7 @@ module.exports = {
     browser: true,
     node: true,
   },
+  rules: {
+    'no-console': 'off',
+  },
 };

packages/storybook/src/stories/modal.stories.svelte

@@ -1,66 +1,64 @@
 <script lang="ts">
 import { Meta, Story } from '@storybook/addon-svelte-csf';
 import { Modal, Button } from '@viamrobotics/prime-core';
-import { writable } from 'svelte/store';
+
+let defaultModalOpen = false;
+let smallModalOpen = false;
 </script>
 
 <Meta title="Elements/Modal" />
 
 <Story name="Default">
-  <Modal isOpen={writable(true)}>
-    <svelte:fragment slot="title">Default title</svelte:fragment>
-    <svelte:fragment slot="message">This is the default modal.</svelte:fragment>
-    <svelte:fragment slot="primary">
+  <div class="h-[500px]">
+    <Button on:click={() => (defaultModalOpen = true)}>Open</Button>
+    <Modal
+      isOpen={defaultModalOpen}
+      on:close={() => (defaultModalOpen = false)}
+    >
+      <svelte:fragment slot="title">Default title</svelte:fragment>
+      <svelte:fragment slot="message"
+        >This is the default modal.</svelte:fragment
+      >
       <Button
-        on:click={() => {
-          /* eslint-disable-next-line no-console */
-          console.log('Primary button clicked');
-        }}
+        slot="primary"
+        on:click={() => console.log('Primary button clicked')}
       >
         Primary
       </Button>
-    </svelte:fragment>
-    <svelte:fragment slot="secondary">
       <Button
+        slot="secondary"
         variant="dark"
-        on:click={() => {
-          /* eslint-disable-next-line no-console */
-          console.log('Secondary button clicked');
-        }}
+        on:click={() => console.log('Secondary button clicked')}
       >
         Secondary
       </Button>
-    </svelte:fragment>
-  </Modal>
+    </Modal>
+  </div>
 </Story>
 
 <Story name="Small">
-  <Modal
-    isOpen={writable(true)}
-    variant="small"
-  >
-    <svelte:fragment slot="title">Small modal title</svelte:fragment>
-    <svelte:fragment slot="message">This is a small modal.</svelte:fragment>
-    <svelte:fragment slot="primary">
+  <div class="h-[200px]">
+    <Button on:click={() => (smallModalOpen = true)}>Open</Button>
+    <Modal
+      isOpen={smallModalOpen}
+      variant="small"
+      on:close={() => (smallModalOpen = false)}
+    >
+      <svelte:fragment slot="title">Small modal title</svelte:fragment>
+      <svelte:fragment slot="message">This is a small modal.</svelte:fragment>
       <Button
-        on:click={() => {
-          /* eslint-disable-next-line no-console */
-          console.log('Primary button clicked');
-        }}
+        slot="primary"
+        on:click={() => console.log('Primary button clicked')}
       >
         Primary
       </Button>
-    </svelte:fragment>
-    <svelte:fragment slot="secondary">
       <Button
+        slot="secondary"
         variant="dark"
-        on:click={() => {
-          /* eslint-disable-next-line no-console */
-          console.log('Secondary button clicked');
-        }}
+        on:click={() => console.log('Secondary button clicked')}
       >
         Secondary
       </Button>
-    </svelte:fragment>
-  </Modal>
+    </Modal>
+  </div>
 </Story>