From 407b72c7a5e917d31315f89d9434b2e9da8a2d6b Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Fri, 16 May 2025 06:02:02 -0400 Subject: [PATCH 1/9] fix drag and drop selected options and reinstate playwright test also reinstate multiselect test 'opens dropdown on focus', no longer fails --- src/lib/CmdPalette.svelte | 1 - src/lib/MultiSelect.svelte | 6 +++--- src/lib/Wiggle.svelte | 1 - src/routes/+layout.svelte | 6 ++---- tests/MultiSelect.test.ts | 4 ++-- 5 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/lib/CmdPalette.svelte b/src/lib/CmdPalette.svelte index ba049cf6..d0e4994c 100644 --- a/src/lib/CmdPalette.svelte +++ b/src/lib/CmdPalette.svelte @@ -1,7 +1,6 @@ @@ -79,16 +73,7 @@ onadd={trigger_action_and_close} onkeydown={toggle} {...rest} - > - {#snippet children({ option })} - - {#if children_render} - {@render children_render()} - {:else} - {option.label} - {/if} - {/snippet} - + /> {/if} diff --git a/src/lib/MultiSelect.svelte b/src/lib/MultiSelect.svelte index 22594573..f64ba1bf 100644 --- a/src/lib/MultiSelect.svelte +++ b/src/lib/MultiSelect.svelte @@ -500,7 +500,7 @@ class:single={maxSelect === 1} class:open class:invalid - class="multiselect {outerDivClass}" + class="multiselect {outerDivClass} {rest.class ?? ``}" onmouseup={open_dropdown} title={disabled ? disabledInputTitle : null} data-id={id} diff --git a/src/routes/(demos)/+layout.svelte b/src/routes/(demos)/+layout.svelte index b704d4f4..85df44f5 100644 --- a/src/routes/(demos)/+layout.svelte +++ b/src/routes/(demos)/+layout.svelte @@ -3,10 +3,11 @@ import { name } from '$root/package.json' import { DemoNav } from '$site' import { demos } from '$site/stores' + import type { Snippet } from 'svelte' import { PrevNext } from 'svelte-zoo' interface Props { - children?: import('svelte').Snippet + children?: Snippet } let { children }: Props = $props() diff --git a/tests/unit/CmdPalette.svelte.test.ts b/tests/unit/CmdPalette.svelte.test.ts index 06f7eb22..38926553 100644 --- a/tests/unit/CmdPalette.svelte.test.ts +++ b/tests/unit/CmdPalette.svelte.test.ts @@ -1,83 +1,247 @@ import { CmdPalette } from '$lib' -import { flushSync, mount } from 'svelte' +import { mount, tick } from 'svelte' import { expect, test, vi } from 'vitest' import { doc_query } from '.' const actions = [{ label: `action 1`, action: vi.fn() }] -test.each([[[`k`]], [[`o`]], [[`k`, `o`]]])( - `opens the dialog on cmd+ custom trigger keys`, - async (triggers) => { - mount(CmdPalette, { target: document.body, props: { triggers, actions } }) +test.each([ + { triggers: [`k`], key_to_press: `k` }, + { triggers: [`o`], key_to_press: `o` }, + { triggers: [`k`, `o`], key_to_press: `k` }, + { triggers: [`k`, `o`], key_to_press: `o` }, +])( + `opens the dialog on cmd + $key_to_press (triggers: $triggers)`, + async ({ triggers, key_to_press }) => { + const props = $state({ open: false, triggers, actions, fade_duration: 0 }) + mount(CmdPalette, { target: document.body, props }) - // dialog should initially be absent expect(document.querySelector(`dialog`)).toBe(null) + expect(props.open).toBe(false) - // press cmd + trigger to open the palette + // Press cmd + trigger to open the palette window.dispatchEvent( - new KeyboardEvent(`keydown`, { key: triggers[0], metaKey: true }), + new KeyboardEvent(`keydown`, { key: key_to_press, metaKey: true }), ) - flushSync() + await tick() expect(doc_query(`dialog`)).toBeTruthy() + expect(props.open).toBe(true) + + // Test that pressing trigger key without metaKey does not open if already open then closed + props.open = false + await tick() + expect(document.querySelector(`dialog`)).toBe(null) + + window.dispatchEvent( + new KeyboardEvent(`keydown`, { key: key_to_press, metaKey: false }), + ) + await tick() + expect(document.querySelector(`dialog`)).toBe(null) + expect(props.open).toBe(false) }, ) -test(`calls the action when an option is selected`, async () => { +test(`does not open dialog on trigger key press without metaKey`, async () => { + const props = $state({ open: false, triggers: [`k`], actions }) + mount(CmdPalette, { target: document.body, props }) + + expect(document.querySelector(`dialog`)).toBe(null) + expect(props.open).toBe(false) + + window.dispatchEvent( + new KeyboardEvent(`keydown`, { key: `k`, metaKey: false }), + ) + + expect(document.querySelector(`dialog`)).toBe(null) + expect(props.open).toBe(false) +}) + +test(`calls the action and closes dialog when an option is selected`, async () => { const spy = vi.fn() - const actions = [{ label: `action 1`, action: spy }] + const test_actions = [{ label: `action 1`, action: spy }] + const props = $state({ open: true, actions: test_actions, fade_duration: 0 }) - mount(CmdPalette, { target: document.body, props: { open: true, actions } }) + mount(CmdPalette, { target: document.body, props }) - const input = doc_query(`dialog div.multiselect input[autocomplete]`) - // press down arrow, then enter to select the first action - input.dispatchEvent( + const input_el = doc_query(`dialog div.multiselect input[autocomplete]`) + // Press down arrow, then enter to select the first action + input_el.dispatchEvent( new KeyboardEvent(`keydown`, { key: `ArrowDown`, bubbles: true }), ) - flushSync() - input.dispatchEvent( + await tick() + input_el.dispatchEvent( new KeyboardEvent(`keydown`, { key: `Enter`, bubbles: true }), ) - flushSync() + await tick() expect(spy).toHaveBeenCalledOnce() - expect(spy).toHaveBeenCalledWith(actions[0].label) + expect(spy).toHaveBeenCalledWith(test_actions[0].label) + expect(props.open).toBe(false) + expect(document.querySelector(`dialog`)).toBe(null) }) -test.each([[[`Escape`]], [[`x`]], [[`Escape`, `x`]]])( - `closes the dialog on close keys`, - async (close_keys) => { - const props = $state({ open: true, close_keys, actions }) +test.each([ + { close_keys: [`Escape`], key_to_press: `Escape` }, + { close_keys: [`x`], key_to_press: `x` }, + { close_keys: [`Escape`, `x`], key_to_press: `Escape` }, + { close_keys: [`Escape`, `x`], key_to_press: `x` }, +])( + `closes the dialog on $key_to_press (close_keys: $close_keys)`, + async ({ close_keys, key_to_press }) => { + const props = $state({ open: true, close_keys, actions, fade_duration: 0 }) + mount(CmdPalette, { target: document.body, props }) - mount(CmdPalette, { - target: document.body, - props, - }) + const init_dialog_el = doc_query(`dialog`) + expect(init_dialog_el).toBeTruthy() + expect(props.open).toBe(true) - const dialog = doc_query(`dialog`) - expect(dialog).toBeTruthy() + window.dispatchEvent(new KeyboardEvent(`keydown`, { key: key_to_press })) + await tick() // Wait for props.open to update and event to be handled - window.dispatchEvent(new KeyboardEvent(`keydown`, { key: close_keys[0] })) expect(props.open).toBe(false) - // TODO somehow dialog isn't removed from the DOM - // expect(document.querySelector(`dialog`)).toBe(null) + + const final_dialog_el = document.querySelector(`dialog`) + expect(final_dialog_el).toBe(null) }, ) -test(`closes the dialog on click outside`, async () => { - const props = $state({ open: true, actions }) +test(`closes the dialog on click outside and removes dialog from DOM`, async () => { + const props = $state({ open: true, actions, fade_duration: 0 }) + + mount(CmdPalette, { target: document.body, props }) + + const dialog = doc_query(`dialog`) + expect(dialog).toBeTruthy() + expect(props.open).toBe(true) + // Create a click event outside the dialog + const click = new MouseEvent(`click`, { bubbles: true }) + document.body.dispatchEvent(click) + await tick() // Allow props.open to update + + expect(props.open).toBe(false) + expect(document.querySelector(`dialog`)).toBe(null) +}) + +test(`displays the correct placeholder text in the input`, () => { + const placeholder_text = `Test placeholder` mount(CmdPalette, { target: document.body, - props, + props: { + open: true, + actions, + placeholder: placeholder_text, + }, + }) + + const input = doc_query(`dialog input[autocomplete]`) as HTMLInputElement + expect(input.placeholder).toBe(placeholder_text) +}) + +test.each([ + { + opens_how: `keypress`, + description: `focuses input when dialog opens via keypress`, + }, + { + opens_how: `programmatic`, + description: `focuses input when dialog opens programmatically`, + }, +])(`$description`, async ({ opens_how }) => { + const props = $state({ actions, open: false, fade_duration: 0 }) + mount(CmdPalette, { target: document.body, props }) + + if (opens_how === `keypress`) { + // Open the dialog by simulating the trigger key press + window.dispatchEvent( + new KeyboardEvent(`keydown`, { key: `k`, metaKey: true }), + ) + } else props.open = true // Open programmatically + + await tick() + + const input = document.querySelector( + `dialog input[autocomplete]`, + ) as HTMLInputElement | null + expect(input).not.toBeNull() + expect(document.activeElement).toBe(input) +}) + +test(`handles empty actions array gracefully`, () => { + mount(CmdPalette, { + target: document.body, + props: { open: true, actions: [] }, }) const dialog = doc_query(`dialog`) expect(dialog).toBeTruthy() + const options_list = document.querySelector(`dialog ul.options`) + if (options_list) { + expect(options_list.children.length).toBe(0) + } +}) - // create a click event outside the dialog - const click = new MouseEvent(`click`, { bubbles: true }) - document.body.dispatchEvent(click) +test(`applies custom style to the dialog element`, () => { + const custom_style = `border: 2px solid red; padding: 20px;` + mount(CmdPalette, { + target: document.body, + props: { open: true, actions, style: custom_style }, + }) + + const dialog = doc_query(`dialog`) as HTMLDialogElement + expect(dialog.style.border).toBe(`2px solid red`) + expect(dialog.style.padding).toBe(`20px`) +}) +test(`applies custom span_style to default option rendering`, () => { + const custom_li_style = `color: blue; font-weight: bold;` + mount(CmdPalette, { + target: document.body, + props: { open: true, actions, liOptionStyle: custom_li_style }, + }) + + const li_el = doc_query(`dialog ul.options li`) + expect(li_el).toBeTruthy() + // Note: direct style checking can be brittle if Svelte optimizes or changes how styles are applied. + // It's often better to check computed style, but for direct inline styles this can be okay. + expect(li_el.style.color).toBe(`blue`) + expect(li_el.style.fontWeight).toBe(`bold`) +}) + +test(`programmatic control via bind:open`, async () => { + const props = $state({ open: false, actions, fade_duration: 0 }) + mount(CmdPalette, { target: document.body, props }) + + expect(document.querySelector(`dialog`)).toBe(null) + + // Programmatically open + props.open = true + await tick() + expect(doc_query(`dialog`)).toBeTruthy() + expect(props.open).toBe(true) + + const input_el = doc_query( + `dialog input[autocomplete]`, + ) as HTMLInputElement | null + expect(input_el).not.toBeNull() + expect(document.activeElement).toBe(input_el) + + // Programmatically close + props.open = false + await tick() + expect(document.querySelector(`dialog`)).toBe(null) expect(props.open).toBe(false) }) + +test(`passes through ...rest props like class to Select component`, () => { + const custom_class = `my-custom-palette-class` + mount(CmdPalette, { + target: document.body, + props: { open: true, actions, class: custom_class }, + }) + + const select_wrapper = doc_query(`dialog div.multiselect`) + expect(select_wrapper).toBeTruthy() + expect(select_wrapper.classList.contains(custom_class)).toBe(true) +}) diff --git a/vite.config.ts b/vite.config.ts index 67123b9b..018121a6 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -7,6 +7,7 @@ export default defineConfig({ plugins: [sveltekit(), mdsvexamples], test: { + include: [`tests/unit/**/*.test.ts`], environment: `jsdom`, css: true, coverage: { @@ -15,11 +16,9 @@ export default defineConfig({ setupFiles: [resolve(__dirname, `tests/setup.ts`)], }, - resolve: process.env.TEST - ? { - conditions: [`browser`], - } - : undefined, + resolve: { + conditions: process.env.TEST ? [`browser`] : undefined, + }, server: { fs: { allow: [`..`] }, // needed to import from $root From 99a8439f6e08a3f786bd86be7f8a40e9ce8bb085 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sat, 17 May 2025 07:03:03 -0400 Subject: [PATCH 3/9] simplify CSS scoping --- src/lib/MultiSelect.svelte | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib/MultiSelect.svelte b/src/lib/MultiSelect.svelte index f64ba1bf..f96df74f 100644 --- a/src/lib/MultiSelect.svelte +++ b/src/lib/MultiSelect.svelte @@ -910,7 +910,7 @@ pointer-events: none; } - :is(div.multiselect > ul.options) { + ul.options { list-style: none; top: 100%; left: 0; @@ -929,29 +929,29 @@ padding: var(--sms-options-padding); margin: var(--sms-options-margin, inherit); } - :is(div.multiselect > ul.options.hidden) { + ul.options.hidden { visibility: hidden; opacity: 0; transform: translateY(50px); } - :is(div.multiselect > ul.options > li) { + ul.options > li { padding: 3pt 2ex; cursor: pointer; scroll-margin: var(--sms-options-scroll-margin, 100px); } - :is(div.multiselect > ul.options .user-msg) { + ul.options .user-msg { /* block needed so vertical padding applies to span */ display: block; padding: 3pt 2ex; } - :is(div.multiselect > ul.options > li.selected) { + ul.options > li.selected { background: var(--sms-li-selected-bg); color: var(--sms-li-selected-color); } - :is(div.multiselect > ul.options > li.active) { + ul.options > li.active { background: var(--sms-li-active-bg, var(--sms-active-color, rgba(0, 0, 0, 0.15))); } - :is(div.multiselect > ul.options > li.disabled) { + ul.options > li.disabled { cursor: not-allowed; background: var(--sms-li-disabled-bg, #f5f5f6); color: var(--sms-li-disabled-text, #b8b8b8); From a45911a4506eca0107560a4daf58381ceedf71f5 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sat, 17 May 2025 07:04:09 -0400 Subject: [PATCH 4/9] add/remove use generic T for option arg type, rename arg for clarity --- src/lib/MultiSelect.svelte | 54 +++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/lib/MultiSelect.svelte b/src/lib/MultiSelect.svelte index f96df74f..46e46a79 100644 --- a/src/lib/MultiSelect.svelte +++ b/src/lib/MultiSelect.svelte @@ -198,20 +198,20 @@ }) // add an option to selected list - function add(option: Option, event: Event) { + function add(option_to_add: T, event: Event) { event.stopPropagation() if (maxSelect && maxSelect > 1 && selected.length >= maxSelect) wiggle = true - if (!isNaN(Number(option)) && typeof selected.map(get_label)[0] === `number`) { - option = Number(option) as Option // convert to number if possible + if (!isNaN(Number(option_to_add)) && typeof selected.map(get_label)[0] === `number`) { + option_to_add = Number(option_to_add) as Option // convert to number if possible } - const is_duplicate = selected.map(key).includes(key(option)) + const is_duplicate = selected.map(key).includes(key(option_to_add)) if ( (maxSelect === null || maxSelect === 1 || selected.length < maxSelect) && (duplicates || !is_duplicate) ) { if ( - !options.includes(option) && // first check if we find option in the options list + !options.includes(option_to_add) && // first check if we find option in the options list // this has the side-effect of not allowing to user to add the same // custom option twice in append mode [true, `append`].includes(allowUserOptions) && @@ -221,32 +221,32 @@ // a new option from the user-entered text if (typeof options[0] === `object`) { // if 1st option is an object, we create new option as object to keep type homogeneity - option = { label: searchText } as Option + option_to_add = { label: searchText } as Option } else { if ( [`number`, `undefined`].includes(typeof options[0]) && !isNaN(Number(searchText)) ) { // create new option as number if it parses to a number and 1st option is also number or missing - option = Number(searchText) as Option + option_to_add = Number(searchText) as Option } else { - option = searchText as Option // else create custom option as string + option_to_add = searchText as Option // else create custom option as string } - oncreate?.({ option }) + oncreate?.({ option: option_to_add }) } - if (allowUserOptions === `append`) options = [...options, option] + if (allowUserOptions === `append`) options = [...options, option_to_add] } if (resetFilterOnAdd) searchText = `` // reset search string on selection - if ([``, undefined, null].includes(option as string | null)) { - console.error(`MultiSelect: encountered falsy option ${option}`) + if ([``, undefined, null].includes(option_to_add as string | null)) { + console.error(`MultiSelect: encountered falsy option ${option_to_add}`) return } if (maxSelect === 1) { // for maxSelect = 1 we always replace current option with new one - selected = [option] + selected = [option_to_add] } else { - selected = [...selected, option] + selected = [...selected, option_to_add] if (sortSelected === true) { selected = selected.sort((op1, op2) => { const [label1, label2] = [get_label(op1), get_label(op2)] @@ -269,8 +269,8 @@ } else if (!dropdown_should_close) { input?.focus() } - onadd?.({ option }) - onchange?.({ option, type: `add` }) + onadd?.({ option: option_to_add }) + onchange?.({ option: option_to_add, type: `add` }) invalid = false // reset error status whenever new items are selected form_input?.setCustomValidity(``) @@ -278,26 +278,26 @@ } // remove an option from selected list - function remove(to_remove: Option, event: Event) { + function remove(option_to_drop: T, event: Event) { event.stopPropagation() if (selected.length === 0) return - const idx = selected.findIndex((opt) => key(opt) === key(to_remove)) + const idx = selected.findIndex((opt) => key(opt) === key(option_to_drop)) - let [option] = selected.splice(idx, 1) // remove option from selected list + let [option_removed] = selected.splice(idx, 1) // remove option from selected list - if (option === undefined && allowUserOptions) { + if (option_removed === undefined && allowUserOptions) { // if option with label could not be found but allowUserOptions is truthy, // assume it was created by user and create corresponding option object // on the fly for use as event payload const other_ops_type = typeof options[0] - option = (other_ops_type ? { label: to_remove } : to_remove) as Option + option_removed = ( + other_ops_type ? { label: option_to_drop } : option_to_drop + ) as Option } - if (option === undefined) { + if (option_removed === undefined) { return console.error( - `Multiselect can't remove selected option ${JSON.stringify( - to_remove, - )}, not found in selected list`, + `Multiselect can't remove selected option ${JSON.stringify(option_to_drop)}, not found in selected list`, ) } @@ -305,8 +305,8 @@ invalid = false // reset error status whenever items are removed form_input?.setCustomValidity(``) - onremove?.({ option }) - onchange?.({ option, type: `remove` }) + onremove?.({ option: option_removed }) + onchange?.({ option: option_removed, type: `remove` }) } function open_dropdown(event: Event) { From 9ba20a769e0e5089efa9142ffd99713c426b67ea Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sat, 17 May 2025 10:14:57 -0400 Subject: [PATCH 5/9] minor refactor CmdPalette --- src/lib/CmdPalette.svelte | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/lib/CmdPalette.svelte b/src/lib/CmdPalette.svelte index c25715bf..e917c107 100644 --- a/src/lib/CmdPalette.svelte +++ b/src/lib/CmdPalette.svelte @@ -73,13 +73,18 @@ onadd={trigger_action_and_close} onkeydown={toggle} {...rest} + --sms-bg="var(--sms-options-bg)" + --sms-width="min(20em, 90vw)" + --sms-max-width="none" + --sms-placeholder-color="lightgray" + --sms-options-margin="1px 0" + --sms-options-border-radius="0 0 1ex 1ex" /> {/if} From 7d3ad00e1849aee2e71d283afc17d0e585ce2ffa Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sat, 17 May 2025 10:18:06 -0400 Subject: [PATCH 6/9] fix escape key navigating back to landing page from demo pages --- src/app.css | 5 +++-- src/routes/(demos)/+layout.svelte | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/app.css b/src/app.css index c7433651..5c8a2a39 100644 --- a/src/app.css +++ b/src/app.css @@ -2,6 +2,7 @@ --night: #0b0b1b; --blue: cornflowerblue; --text-color: #ccc; + --main-max-width: 50em; --sms-active-color: rgba(255, 255, 255, 0.1); --sms-focus-border: 1pt solid cornflowerblue; @@ -28,7 +29,7 @@ main { margin: auto; margin-bottom: 3em; width: 100%; - max-width: 50em; + max-width: var(--main-max-width); } button, a.btn { @@ -139,7 +140,7 @@ div.multiselect.invalid { aside.toc.desktop { position: fixed; top: 3em; - left: calc(50vw + 50em / 2); + left: calc(50vw + var(--main-max-width) / 2); max-width: 16em; } diff --git a/src/routes/(demos)/+layout.svelte b/src/routes/(demos)/+layout.svelte index 85df44f5..e084584f 100644 --- a/src/routes/(demos)/+layout.svelte +++ b/src/routes/(demos)/+layout.svelte @@ -21,10 +21,12 @@ {@render children?.()} + ({})} /> From 79264931794ae5a629c0bfc3179cfbbd22d0f1e7 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sat, 17 May 2025 10:25:58 -0400 Subject: [PATCH 7/9] add portal functionality to MultiSelect component (closes #301) to extend dropdown outside of parent component (e.g. inside a modal dialog) - new `portal` function manages dropdown rendering if portal prop is not empty - `MultiSelectProps` include optional `portal` parameter to enable and customize portal functionality - refactored the `add` function to simplify max selection logic - improved CSS for dropdown positioning and z-index management when using the portal --- src/lib/MultiSelect.svelte | 68 ++++++++++++++++++++++++++++++++++++-- src/lib/props.ts | 6 ++++ 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/src/lib/MultiSelect.svelte b/src/lib/MultiSelect.svelte index 46e46a79..56d8fa4e 100644 --- a/src/lib/MultiSelect.svelte +++ b/src/lib/MultiSelect.svelte @@ -1,4 +1,5 @@ {#if (searchText && noMatchingOptionsMsg) || options?.length > 0}
    + displays above that of another slightly below it on the page */ + /* This z-index is for the div.multiselect itself, portal has its own higher z-index */ + z-index: var(--sms-open-z-index, 4); + } ul.options.hidden { visibility: hidden; opacity: 0; diff --git a/src/lib/props.ts b/src/lib/props.ts index 9abfaf7a..cf9be437 100644 --- a/src/lib/props.ts +++ b/src/lib/props.ts @@ -71,6 +71,11 @@ export interface MultiSelectSnippets { > } +export interface PortalParams { + target_node?: HTMLElement | null + active?: boolean +} + export interface MultiSelectParameters { activeIndex?: number | null activeOption?: T | null @@ -135,6 +140,7 @@ export interface MultiSelectParameters { ulSelectedStyle?: string | null ulOptionsStyle?: string | null value?: T | T[] | null + portal?: PortalParams [key: string]: unknown } From 90849375d226a0f7b0f41a6043ff729ae19636c0 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sat, 17 May 2025 10:42:05 -0400 Subject: [PATCH 8/9] add /modal/+page.svelte demo page for portal feature --- src/lib/MultiSelect.svelte | 1 - src/routes/(demos)/modal/+page.svelte | 185 ++++++++++++++++++++++++++ tests/MultiSelect.test.ts | 12 +- 3 files changed, 187 insertions(+), 11 deletions(-) create mode 100644 src/routes/(demos)/modal/+page.svelte diff --git a/src/lib/MultiSelect.svelte b/src/lib/MultiSelect.svelte index 56d8fa4e..92708baf 100644 --- a/src/lib/MultiSelect.svelte +++ b/src/lib/MultiSelect.svelte @@ -500,7 +500,6 @@ if (!render_in_place) { document.body.appendChild(node) node.style.position = `fixed` - node.style.zIndex = `var(--sms-open-z-index, 5)` const update_position = () => { if (!target_node || !open) return (node.hidden = true) diff --git a/src/routes/(demos)/modal/+page.svelte b/src/routes/(demos)/modal/+page.svelte new file mode 100644 index 00000000..376fc847 --- /dev/null +++ b/src/routes/(demos)/modal/+page.svelte @@ -0,0 +1,185 @@ + + + + +

    MultiSelect in Modal Demo

    + +
    + + +
    + +{#if show_modal_1} +
    { + if (event.target === event.currentTarget) show_modal_1 = false + }} + onkeydown={(event) => { + if (event.key === `Enter` || event.key === ` `) show_modal_1 = false + }} + role="button" + tabindex="0" + > +
    { + if (event.key !== `Escape`) event.stopPropagation() + }} + role="dialog" + aria-modal="true" + aria-labelledby="modal-1-title" + tabindex="-1" + > + + + +

    Selected Foods: {selected_foods.map(get_label).join(`, `) || `None`}

    +

    Selected Colors: {selected_colors.map(get_label).join(`, `) || `None`}

    + +
    +
    +{/if} + +{#if show_modal_2} +
    { + if (event.target === event.currentTarget) show_modal_2 = false + }} + onkeydown={(event) => { + if (event.key === `Enter` || event.key === ` `) show_modal_2 = false + }} + role="button" + tabindex="0" + > +
    { + if (event.key !== `Escape`) event.stopPropagation() + }} + role="dialog" + aria-modal="true" + aria-labelledby="modal-2-title" + tabindex="-1" + > + +
    + + +
    +

    Selected Languages: {selected_languages.map(get_label).join(`, `) || `None`}

    +

    Selected Octicons: {selected_octicons.map(get_label).join(`, `) || `None`}

    + +
    +
    +{/if} + + diff --git a/tests/MultiSelect.test.ts b/tests/MultiSelect.test.ts index d94cea89..65f10d68 100644 --- a/tests/MultiSelect.test.ts +++ b/tests/MultiSelect.test.ts @@ -74,9 +74,7 @@ test.describe(`remove all button`, () => { await page.goto(`/ui`, { waitUntil: `networkidle` }) const input_locator = page.locator(`#foods input[autocomplete]`) - // Use a more general locator for options list that doesn't depend on .open class, - // and rely on waitFor to ensure it's visible after click. - const options_list_locator = page.locator(`#foods ul.options`) + const options_list_locator = page.locator(`ul.options[role="listbox"]`) // Select first option await input_locator.click() // Ensure dropdown is open @@ -101,9 +99,6 @@ test.describe(`remove all button`, () => { ) expect(selected_items).toHaveLength(2) - // await Promise.all([ - // await page.waitForEvent(`removeAll`), - // ]) await page.click(`button.remove-all`) expect(await page.$(`button.remove-all`)).toBeNull() // remove-all button is hidden when nothing selected @@ -208,9 +203,7 @@ test.describe(`accessibility`, () => { const invalid = await page.getAttribute( `#foods input[autocomplete]`, `aria-invalid`, - { - strict: true, - }, + { strict: true }, ) expect(invalid).toBe(`true`) }) @@ -562,7 +555,6 @@ test.describe(`snippets`, () => { }) => { await page.goto(`/snippets`, { waitUntil: `networkidle` }) - // Use a more general descendant selector for the SVG within the button const expand_icon_locator = page.locator( `#languages-1 .multiselect > input.form-control + svg`, ) From 6d8932529ab2a29f688100d09daa2acc017c7fd9 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sat, 17 May 2025 11:11:43 -0400 Subject: [PATCH 9/9] playwright test for MultiSelect portal feature --- .pre-commit-config.yaml | 2 +- tests/MultiSelect.test.ts | 64 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 08e3c7b9..75d73e4a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -38,7 +38,7 @@ repos: exclude: changelog\.md - repo: https://github.com/pre-commit/mirrors-eslint - rev: v9.26.0 + rev: v9.27.0 hooks: - id: eslint types: [file] diff --git a/tests/MultiSelect.test.ts b/tests/MultiSelect.test.ts index 65f10d68..8631cf4d 100644 --- a/tests/MultiSelect.test.ts +++ b/tests/MultiSelect.test.ts @@ -616,3 +616,67 @@ test(`dragging selected options across each other changes their order`, async ({ selected = await page.textContent(`ul.selected`) expect(selected?.trim()).toBe(`1 TypeScript 2 Python 3 C 4 Haskell`) }) + +test.describe(`portal feature`, () => { + test(`dropdown renders in document.body when portal is active in modal`, async ({ + page, + }) => { + await page.goto(`/modal`, { waitUntil: `networkidle` }) + + // Open the first modal + await page + .getByRole(`button`, { name: `Open Modal 1 (Vertical Selects)` }) + .click() + + // Locate the MultiSelect input for foods within the first modal + const foods_multiselect_input = page.locator( + `div.modal-content.modal-1 div.multiselect input[placeholder='Choose foods...']`, + ) + // Wait for the modal to be fully visible and input to be available + await foods_multiselect_input.waitFor({ state: `visible` }) + + // Click the input to open the dropdown + await foods_multiselect_input.click() + + // Identify the specific portalled options list for foods + // It should be in the body and contain the first food item + const foods_options_list_in_body = page.locator( + `body > ul.options:has(li:has-text("${foods[0]}"))`, + ) + await foods_options_list_in_body.waitFor({ state: `visible` }) + await expect(foods_options_list_in_body).toHaveAttribute( + `aria-expanded`, + `true`, + ) // Confirm it's open + + // Assert that the options list is NOT a direct child of the multiselect wrapper within the modal + const multiselect_wrapper_in_modal = page.locator( + `div.modal-content.modal-1 div.multiselect:has(input[placeholder='Choose foods...'])`, + ) + await expect( + multiselect_wrapper_in_modal.locator(`> ul.options`), + ).not.toBeAttached() + + // Select an option from this specific portalled dropdown + await foods_options_list_in_body + .locator(`li:has-text("${foods[0]}")`) + .click() + + // Assert this specific dropdown is now hidden + await expect(foods_options_list_in_body).toBeHidden({ timeout: 3000 }) + + // Explicitly wait for the selected item to appear before asserting visibility + await expect( + page.getByRole(`button`, { + name: `Remove 🍇 Grapes`, + }), + ).toBeVisible() + + // click escape to close the dropdown + await page.keyboard.press(`Escape`) + + // Close the modal + await page.getByRole(`button`, { name: `Close Modal 1` }).click() + await expect(page.locator(`div.modal-content.modal-1`)).toBeHidden() + }) +})