diff --git a/.changeset/floppy-donuts-begin.md b/.changeset/floppy-donuts-begin.md new file mode 100644 index 000000000..2b10ac9ea --- /dev/null +++ b/.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 diff --git a/apps/website/src/routes/docs/headless/modal/examples/test-callbacks.tsx b/apps/website/src/routes/docs/headless/modal/examples/test-callbacks.tsx new file mode 100644 index 000000000..36d1241ec --- /dev/null +++ b/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 ( + <> + +

onShow count: {showCount.value}

+

onClose count: {closeCount.value}

+ showCount.value++} + onClose$={() => closeCount.value++} + > + + Callback Test Modal + Close + + + + ); +}); + +// internal +import styles from '../snippets/modal.css?inline'; diff --git a/packages/kit-headless/src/components/modal/modal-panel.tsx b/packages/kit-headless/src/components/modal/modal-panel.tsx index f63042241..22841d8fb 100644 --- a/packages/kit-headless/src/components/modal/modal-panel.tsx +++ b/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(); + 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$?.(); } }); diff --git a/packages/kit-headless/src/components/modal/modal.test.ts b/packages/kit-headless/src/components/modal/modal.test.ts index f16bb63d1..8c860e009 100644 --- a/packages/kit-headless/src/components/modal/modal.test.ts +++ b/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');