Skip to content

Commit

Permalink
MultiSelect fix form validation not resetting when required prop …
Browse files Browse the repository at this point in the history
…changes (#286)

* update deps except vite

vite 5.2+ seems to break mdsvexamples

* migrate eslint to v9 flat config

* CSS.highlights no longer eslint undef

* MultiSelect fix form validation reset when required prop changes

* only run unit tests in CI, playwright been broken long time

* add test for submittable form after toggling required prop
  • Loading branch information
janosh committed Apr 8, 2024
1 parent 1b04c15 commit 22d51cf
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 55 deletions.
27 changes: 0 additions & 27 deletions .eslintrc.yml

This file was deleted.

2 changes: 2 additions & 0 deletions .github/workflows/gh-pages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ on:
jobs:
build:
uses: janosh/workflows/.github/workflows/nodejs-gh-pages.yml@main
with:
install-cmd: npm install --force
3 changes: 2 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ jobs:
tests:
uses: janosh/workflows/.github/workflows/npm-test-release.yml@main
with:
install-cmd: npm install -f
install-cmd: npm install --force
test-cmd: npm run test:unit
12 changes: 6 additions & 6 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
ci:
autoupdate_schedule: quarterly
skip: [eslint]

default_stages: [commit]

default_install_hook_types: [pre-commit, commit-msg]

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.5.0
rev: v4.6.0
hooks:
- id: check-case-conflict
- id: check-symlinks
Expand Down Expand Up @@ -37,20 +38,19 @@ repos:
exclude: changelog\.md

- repo: https://github.com/pre-commit/mirrors-eslint
rev: v9.0.0-beta.2
rev: v9.0.0
hooks:
- id: eslint
types: [file]
args: [--fix]
files: \.(js|ts|svelte)$
additional_dependencies:
- 'typescript-eslint'
- eslint
- eslint-plugin-svelte
- globals
- svelte
- typescript
- eslint-plugin-svelte
- '@typescript-eslint/eslint-plugin'
- '@typescript-eslint/parser'
- svelte-eslint-parser

- repo: local
hooks:
Expand Down
41 changes: 41 additions & 0 deletions eslint.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import svelte from 'eslint-plugin-svelte'
import globals from 'globals'
import tslint from 'typescript-eslint'

/** @type { import("eslint").Linter.FlatConfig[] } */
export default [
...tslint.configs.recommended,
...svelte.configs[`flat/recommended`],
{
rules: {
'@typescript-eslint/no-unused-vars': [
`error`,
{ argsIgnorePattern: `^_`, varsIgnorePattern: `^_` },
],
'@typescript-eslint/quotes': [`error`, `backtick`, { avoidEscape: true }],
'svelte/no-at-html-tags': `off`,
},
},
{
languageOptions: {
ecmaVersion: 2020,
globals: {
...globals.browser,
...globals.es2017,
...globals.node,
$$Generic: false,
},
},
},
{
files: [`**/*.svelte`],
languageOptions: {
parserOptions: {
parser: tslint.parser,
},
},
},
{
ignores: [`build/`, `.svelte-kit/`, `package/`, `vite.config.ts.*`],
},
]
26 changes: 13 additions & 13 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,15 @@
},
"devDependencies": {
"@iconify/svelte": "^3.1.6",
"@playwright/test": "^1.42.1",
"@playwright/test": "^1.43.0",
"@sveltejs/adapter-static": "^3.0.1",
"@sveltejs/kit": "^2.5.3",
"@sveltejs/package": "2.3.0",
"@sveltejs/kit": "^2.5.5",
"@sveltejs/package": "2.3.1",
"@sveltejs/vite-plugin-svelte": "3.0.2",
"@typescript-eslint/eslint-plugin": "^7.1.1",
"@typescript-eslint/parser": "^7.1.1",
"@vitest/coverage-v8": "^1.3.1",
"eslint": "^8.57.0",
"eslint-plugin-svelte": "^2.35.1",
"@vitest/coverage-v8": "^1.4.0",
"eslint": "^9.0.0",
"eslint-plugin-svelte": "^2.36.0",
"globals": "^15.0.0",
"hastscript": "^9.0.0",
"highlight.js": "^11.9.0",
"jsdom": "^24.0.0",
Expand All @@ -46,15 +45,16 @@
"prettier-plugin-svelte": "^3.2.2",
"rehype-autolink-headings": "^7.1.0",
"rehype-slug": "^6.0.0",
"svelte-check": "^3.6.6",
"svelte-check": "^3.6.9",
"svelte-multiselect": "^10.2.0",
"svelte-preprocess": "^5.1.3",
"svelte-toc": "^0.5.7",
"svelte-toc": "^0.5.8",
"svelte-zoo": "^0.4.10",
"svelte2tsx": "^0.7.3",
"typescript": "5.4.2",
"svelte2tsx": "^0.7.6",
"typescript": "5.4.4",
"typescript-eslint": "^7.5.0",
"vite": "^5.1.5",
"vitest": "^1.3.1"
"vitest": "^1.4.0"
},
"keywords": [
"svelte",
Expand Down
7 changes: 5 additions & 2 deletions src/lib/MultiSelect.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -457,9 +457,12 @@
})
// create Highlight object from ranges and add to registry
// eslint-disable-next-line no-undef
CSS.highlights.set(`sms-search-matches`, new Highlight(...ranges.flat()))
}
// reset form validation when required prop changes
// https://github.com/janosh/svelte-multiselect/issues/285
$: required, form_input?.setCustomValidity(``)
</script>

<svelte:window
Expand All @@ -486,7 +489,7 @@
<!-- bind:value={selected} prevents form submission if required prop is true and no options are selected -->
<input
{name}
required={Boolean(required)}
{required}
value={selected.length >= Number(required) ? JSON.stringify(selected) : null}
tabindex="-1"
aria-hidden="true"
Expand Down
8 changes: 4 additions & 4 deletions src/routes/(demos)/form/+page.svx
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ This example shows the JavaScript way of handling MultiSelect fields in form sub
<ColorSlot {idx} {option} />
</MultiSelect>
<button>Submit</button>
<small
>select some options, then click submit to see what data MultiSelect sends to a form
submit handler</small
>
<small>
select some options, then click submit to see what data MultiSelect sends to a form
submit handler
</small>
</form>

{#if form_data}
Expand Down
20 changes: 20 additions & 0 deletions tests/unit/MultiSelect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ describe.each([
props: { options: [1, 2, 3], required, selected, maxSelect },
})

// form should be valid if MultiSelect not required or n_selected >= n_required and <= maxSelect
const form_valid =
!required ||
(selected.length >= Number(required) &&
Expand Down Expand Up @@ -423,6 +424,25 @@ test.each([
},
)

test(`toggling required after invalid form submission allows submitting`, async () => {
// https://github.com/janosh/svelte-multiselect/issues/285
const form = document.createElement(`form`)
document.body.appendChild(form)

const select = new MultiSelect({
target: form,
props: { options: [1, 2, 3], required: true },
})

// form should not be submittable due to missing required input
expect(form.checkValidity()).toBe(false)

// toggle required to false
select.required = false
// form should now be submittable
expect(form.checkValidity()).toBe(true)
})

test(`invalid=true gives top-level div class 'invalid' and input attribute of 'aria-invalid'`, async () => {
new MultiSelect({
target: document.body,
Expand Down
4 changes: 2 additions & 2 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@
"sourceMap": true,

"forceConsistentCasingInFileNames": true,
"resolveJsonModule": true,
},
"resolveJsonModule": true
}
}

0 comments on commit 22d51cf

Please sign in to comment.