fix(modal): prevent onClose$ from firing on initial mount when show is false

.changeset/floppy-donuts-begin.md

@@ -0,0 +1,5 @@
+---
+'@qwik-ui/headless': patch
+---
+
+fix: onClose$ now doesn't fire if the show prop is set to false by default

apps/website/src/routes/docs/headless/modal/examples/test-callbacks.tsx

@@ -0,0 +1,32 @@
+import { component$, useSignal, useStyles$ } from '@builder.io/qwik';
+import { Modal } from '@qwik-ui/headless';
+
+export default component$(() => {
+  useStyles$(styles);
+  const isOpen = useSignal(false);
+  const showCount = useSignal(0);
+  const closeCount = useSignal(0);
+
+  return (
+    <>
+      <button onClick$={() => (isOpen.value = true)} class="modal-trigger">
+        Open Modal
+      </button>
+      <p data-testid="show-count">onShow count: {showCount.value}</p>
+      <p data-testid="close-count">onClose count: {closeCount.value}</p>
+      <Modal.Root
+        bind:show={isOpen}
+        onShow$={() => showCount.value++}
+        onClose$={() => closeCount.value++}
+      >
+        <Modal.Panel class="modal-panel">
+          <Modal.Title>Callback Test Modal</Modal.Title>
+          <Modal.Close class="modal-close">Close</Modal.Close>
+        </Modal.Panel>
+      </Modal.Root>
+    </>
+  );
+});
+
+// internal
+import styles from '../snippets/modal.css?inline';

packages/kit-headless/src/components/modal/modal-panel.tsx

@@ -40,6 +40,7 @@ export const HModalPanel = component$((props: PropsOf<'dialog'>) => {
   const context = useContext(modalContextId);
 
   const panelRef = useSignal<HTMLDialogElement>();
+  const wasOpenSig = useSignal(false);
 
   useTask$(async function toggleModal({ track, cleanup }) {
     const isOpen = track(() => context.showSig.value);
@@ -69,11 +70,12 @@ export const HModalPanel = component$((props: PropsOf<'dialog'>) => {
   });
 
   useTask$(async ({ track }) => {
-    track(() => context.showSig.value);
+    const isOpen = track(() => context.showSig.value);
 
-    if (context.showSig.value) {
+    if (isOpen) {
+      wasOpenSig.value = true;
       await context.onShow$?.();
-    } else {
+    } else if (wasOpenSig.value) {
       await context.onClose$?.();
     }
   });

packages/kit-headless/src/components/modal/modal.test.ts

@@ -234,6 +234,67 @@ test.describe('Nested Modals', () => {
   });
 });
 
+test.describe('Callbacks', () => {
+  test(`GIVEN a modal with bind:show defaulting to false
+        WHEN the page loads
+        THEN onClose$ should NOT be called`, async ({ page }) => {
+    await setup(page, 'test-callbacks');
+
+    await expect(page.getByTestId('close-count')).toHaveText('onClose count: 0');
+  });
+
+  test(`GIVEN a modal with bind:show defaulting to false
+        WHEN the page loads
+        THEN onShow$ should NOT be called`, async ({ page }) => {
+    await setup(page, 'test-callbacks');
+
+    await expect(page.getByTestId('show-count')).toHaveText('onShow count: 0');
+  });
+
+  test(`GIVEN a closed modal
+        WHEN the modal is opened
+        THEN onShow$ should be called once`, async ({ page }) => {
+    const { driver: d } = await setup(page, 'test-callbacks');
+
+    await d.getTrigger().click();
+    await expect(d.getModal()).toBeVisible();
+
+    await expect(page.getByTestId('show-count')).toHaveText('onShow count: 1');
+  });
+
+  test(`GIVEN an open modal
+        WHEN the modal is closed
+        THEN onClose$ should be called once`, async ({ page }) => {
+    const { driver: d } = await setup(page, 'test-callbacks');
+
+    await d.getTrigger().click();
+    await expect(d.getModal()).toBeVisible();
+
+    await page.getByRole('button', { name: 'Close' }).click();
+    await expect(d.getModal()).toBeHidden();
+
+    await expect(page.getByTestId('close-count')).toHaveText('onClose count: 1');
+  });
+
+  test(`GIVEN a modal opened and closed twice
+        WHEN the modal is opened and closed a second time
+        THEN onShow$ and onClose$ should each have been called twice`, async ({
+    page,
+  }) => {
+    const { driver: d } = await setup(page, 'test-callbacks');
+
+    for (let i = 0; i < 2; i++) {
+      await d.getTrigger().click();
+      await expect(d.getModal()).toBeVisible();
+      await page.getByRole('button', { name: 'Close' }).click();
+      await expect(d.getModal()).toBeHidden();
+    }
+
+    await expect(page.getByTestId('show-count')).toHaveText('onShow count: 2');
+    await expect(page.getByTestId('close-count')).toHaveText('onClose count: 2');
+  });
+});
+
 test.describe('A11y', () => {
   test('Axe Validation Test', async ({ page }) => {
     const { driver: d } = await setup(page, 'hero');