test required but empty MultiSelect fails form validity check (i.e. c…

…auses unsubmittable form) and filled one passes it

src/components/Examples.svelte

@@ -114,10 +114,11 @@
       <ColorSlot let:idx {idx} let:option {option} slot="selected" />
       <ColorSlot let:idx {idx} let:option {option} slot="option" />
     </MultiSelect>
-    <button style="border: none; border-radius: 2pt; margin: 5pt 5pt 8pt 0;">
+    <button style="border: none; border-radius: 1pt; margin: 5pt 5pt 8pt 0;">
       submit
     </button>
-    (form submission will abort if Multiselect is empty)
+    (due to passing <code>required={true}</code> here, form submission will abort if
+    Multiselect is empty)
     <p>
       Also sets
       <code>allowUserOptions="append"</code> to allow adding custom colors.

src/lib/MultiSelect.svelte

@@ -1,5 +1,5 @@
 <script lang="ts">
-  import { createEventDispatcher } from 'svelte'
+  import { createEventDispatcher, tick } from 'svelte'
   import type { DispatchEvents, MultiSelectEvents, ObjectOption, Option } from './'
   import { get_label, get_value } from './'
   import CircleSpinner from './CircleSpinner.svelte'
@@ -98,8 +98,8 @@
 
   // formValue binds to input.form-control to prevent form submission if required
   // prop is true and no options are selected
-  $: formValue = _selectedValues.join(`,`)
-  $: if (formValue) invalid = false // reset error status whenever component state changes
+  $: form_value = _selectedValues.join(`,`)
+  $: if (form_value) invalid = false // reset error status whenever component state changes
 
   // options matching the current search text
   $: matchingOptions = options.filter(
@@ -260,14 +260,13 @@
 
       if (autoScroll) {
         // TODO This ugly timeout hack is needed to properly scroll element into view when wrapping
-        // around start/end of option list. Find a better solution than waiting 10 ms to.
-        setTimeout(() => {
-          const li = document.querySelector(`ul.options > li.active`)
-          if (li) {
-            li.parentNode?.scrollIntoView({ block: `center` })
-            li.scrollIntoViewIfNeeded()
-          }
-        }, 10)
+        // around start/end of option list. Find a better solution than waiting 10 ms.
+        await tick()
+        const li = document.querySelector(`ul.options > li.active`)
+        if (li) {
+          li.parentNode?.scrollIntoView?.({ block: `center` })
+          li.scrollIntoViewIfNeeded?.()
+        }
       }
     }
     // on backspace key: remove last selected option
@@ -318,9 +317,10 @@
   title={disabled ? disabledInputTitle : null}
   aria-disabled={disabled ? `true` : null}
 >
+  <!-- formValue binds to input.form-control to prevent form submission if required prop is true and no options are selected -->
   <input
     {required}
-    bind:value={formValue}
+    bind:value={form_value}
     tabindex="-1"
     aria-hidden="true"
     aria-label="ignore this, used only to prevent form submission if select is required but empty"

src/lib/index.ts

@@ -55,7 +55,8 @@ export const get_value = (op: Option) =>
 // https://github.com/nuxodin/lazyfill/blob/a8e63/polyfills/Element/prototype/scrollIntoViewIfNeeded.js
 if (
   typeof Element !== `undefined` &&
-  !Element.prototype?.scrollIntoViewIfNeeded
+  !Element.prototype?.scrollIntoViewIfNeeded &&
+  typeof IntersectionObserver !== `undefined`
 ) {
   Element.prototype.scrollIntoViewIfNeeded = function (centerIfNeeded = true) {
     const el = this as Element

tests/unit/multiselect.test.ts

@@ -13,11 +13,11 @@ describe(`MultiSelect`, () => {
   test(`defaultDisabledTitle and custom per-option disabled titles are applied correctly`, () => {
     const defaultDisabledTitle = `Not selectable`
     const special_disabled_title = `Special disabled title`
-    const options = [0, 1, 2].map((el) => ({
+    const options = [1, 2, 3].map((el) => ({
       label: el,
       value: el,
       disabled: true,
-      disabledTitle: el ? undefined : special_disabled_title,
+      disabledTitle: el > 1 ? undefined : special_disabled_title,
     }))
 
     new MultiSelect({
@@ -39,15 +39,11 @@ describe(`MultiSelect`, () => {
   test(`removeAllTitle and removeBtnTitle are applied correctly`, () => {
     const removeAllTitle = `Custom remove all title`
     const removeBtnTitle = `Custom remove button title`
-    const options = [1, 2, 3].map((itm) => ({
-      label: itm,
-      value: itm,
-      preselected: true,
-    }))
+    const options = [1, 2, 3]
 
     new MultiSelect({
       target: document.body,
-      props: { removeAllTitle, removeBtnTitle, options },
+      props: { removeAllTitle, removeBtnTitle, options, selected: options },
     })
     const remove_all_btn = document.querySelector(
       `div.multiselect button.remove-all`
@@ -58,12 +54,11 @@ describe(`MultiSelect`, () => {
 
     expect(remove_all_btn.title).toBe(removeAllTitle)
     expect([...remove_btns].map((btn) => btn.title)).toEqual(
-      options.map((op) => `${removeBtnTitle} ${op.label}`)
+      options.map((op) => `${removeBtnTitle} ${op}`)
     )
   })
 
   test(`applies DOM attributes to input node`, () => {
-    const options = [1, 2, 3]
     const searchText = `1`
     const id = `fancy-id`
     const autocomplete = `on`
@@ -75,7 +70,7 @@ describe(`MultiSelect`, () => {
     new MultiSelect({
       target: document.body,
       props: {
-        options,
+        options: [1, 2, 3],
         searchText,
         id,
         autocomplete,
@@ -104,11 +99,6 @@ describe(`MultiSelect`, () => {
   })
 
   test(`applies custom classes for styling through CSS frameworks`, () => {
-    const options = [1, 2, 3].map((itm, idx) => ({
-      label: itm,
-      value: itm,
-      preselected: Boolean(idx),
-    }))
     const classes = Object.fromEntries(
       `input liOption liSelected outerDiv ulOptions ulSelected`
         .split(` `)
@@ -117,7 +107,8 @@ describe(`MultiSelect`, () => {
 
     new MultiSelect({
       target: document.body,
-      props: { options, ...classes },
+      // select 1 to make sure the selected list is rendered
+      props: { options: [1, 2, 3], ...classes, selected: [1] },
     })
 
     // TODO also test liActiveOptionClass once figured out how to make an option active
@@ -135,9 +126,10 @@ describe(`MultiSelect`, () => {
 
   // https://github.com/janosh/svelte-multiselect/issues/111
   test(`arrow down makes first option active`, async () => {
-    const options = [`1`, `2`, `3`]
-
-    new MultiSelect({ target: document.body, props: { options, open: true } })
+    new MultiSelect({
+      target: document.body,
+      props: { options: [1, 2, 3], open: true },
+    })
 
     const input = document.querySelector(`div.multiselect ul.selected input`)
     if (!input) throw new Error(`input not found`)
@@ -155,9 +147,7 @@ describe(`MultiSelect`, () => {
 
   // https://github.com/janosh/svelte-multiselect/issues/112
   test(`can select 1st and last option with arrow and enter key`, async () => {
-    const options = [1, 2, 3]
-
-    new MultiSelect({ target: document.body, props: { options } })
+    new MultiSelect({ target: document.body, props: { options: [1, 2, 3] } })
 
     const input = document.querySelector(`div.multiselect ul.selected input`)
 
@@ -215,15 +205,11 @@ describe(`MultiSelect`, () => {
   })
 
   test(`selected is a single option (not length-1 array) when maxSelect=1`, async () => {
-    const options = [1, 2, 3].map((itm) => ({
-      label: itm,
-      value: itm,
-      preselected: true,
-    }))
+    const options = [1, 2, 3]
 
     const instance = new MultiSelect({
       target: document.body,
-      props: { options, maxSelect: 1 },
+      props: { options, maxSelect: 1, selected: options },
     })
 
     const selected = instance.$$.ctx[instance.$$.props.selected]
@@ -243,7 +229,8 @@ describe(`MultiSelect`, () => {
     expect(selected).toBe(null)
   })
 
-  test(`selected is array of options when maxSelect=2`, async () => {
+  test(`selected is array of first two options when maxSelect=2`, async () => {
+    // even though all options have preselected=true
     const options = [1, 2, 3].map((itm) => ({
       label: itm,
       value: itm,
@@ -259,4 +246,34 @@ describe(`MultiSelect`, () => {
 
     expect(selected).toEqual(options.slice(0, 2))
   })
+
+  test.each([null, [], undefined])(
+    `required but empty MultiSelect makes form not pass validity check`,
+    async (selected) => {
+      // not passing validity check means form won't submit (but dispatching
+      // submit and checking event.defaultPrevented is true seems harder to test)
+
+      const form = document.createElement(`form`)
+      document.body.appendChild(form)
+
+      new MultiSelect({
+        target: form,
+        props: { options: [1, 2, 3], required: true, selected },
+      })
+
+      expect(form.checkValidity()).toBe(false)
+    }
+  )
+
+  test(`required and non-empty MultiSelect makes form pass validity check`, async () => {
+    const form = document.createElement(`form`)
+    document.body.appendChild(form)
+
+    new MultiSelect({
+      target: form,
+      props: { options: [1, 2, 3], required: true, selected: [1] },
+    })
+
+    expect(form.checkValidity()).toBe(true)
+  })
 })