Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow createOptionMsg=null to suppress console error when allowUserOptions thruthy #227

Merged
merged 4 commits into from
May 15, 2023
Merged

Allow createOptionMsg=null to suppress console error when allowUserOptions thruthy #227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
73d796d
allow suppressing createOptionMsg is falsy console error by setting c…
janosh May 15, 2023
1198d22
readme document null option for createOptionMsg and expected develope…
janosh May 15, 2023
8eb7892
silence console error on /allow-user-options demo page
janosh May 15, 2023
11e6af3
add unit test checking no console error when createOptionMsg=null
janosh May 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ Full list of props/bindable variables for this component. The `Option` type you
Currently active option, i.e. the one the user currently hovers or navigated to with arrow keys.

1. ```ts
createOptionMsg: string = `Create this option...`
createOptionMsg: string | null = `Create this option...`
```

Message shown to users after entering text when no options match their query and `allowUserOptions` is truthy.
The message shown to users when `allowUserOptions` is truthy and they entered text that doesn't match any existing options to suggest creating a new option from the entered text. Emits `console.error` if `allowUserOptions` is `true` or `'append'` and `createOptionMsg=''` to since users might be unaware they can create new option. The error can be silenced by setting `createOptionMsg=null` indicating developer intent is to e.g. use MultiSelect as a tagging component where a user message might be unwanted.

1. ```ts
allowEmpty: boolean = false
Expand Down
6 changes: 3 additions & 3 deletions src/lib/MultiSelect.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

export let activeIndex: number | null = null
export let activeOption: Option | null = null
export let createOptionMsg: string = `Create this option...`
export let createOptionMsg: string | null = `Create this option...`
export let allowUserOptions: boolean | 'append' = false
export let allowEmpty: boolean = false // added for https://github.com/janosh/svelte-multiselect/issues/192
export let autocomplete: string = `off`
Expand Down Expand Up @@ -128,9 +128,9 @@
`user re-orderings of selected options will be undone by sortSelected on component re-renders.`
)
}
if (allowUserOptions && !createOptionMsg) {
if (allowUserOptions && !createOptionMsg && createOptionMsg !== null) {
console.error(
`MultiSelect's allowUserOptions=${allowUserOptions} but createOptionMsg=${createOptionMsg} is falsy. ` +
`MultiSelect has allowUserOptions=${allowUserOptions} but createOptionMsg=${createOptionMsg} is falsy. ` +
`This prevents the "Add option" <span> from showing up, resulting in a confusing user experience.`
)
}
Expand Down
4 changes: 2 additions & 2 deletions src/routes/(demos)/allow-user-options/+page.svx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@

## Start empty

You can start with no options and let users populate MultiSelect from scratch. In this case MultiSelect acts more like a tagging component.
You can start with no options and let users populate MultiSelect from scratch. In this case, MultiSelect acts more like a tagging component.

```svelte example stackblitz id="no-default-options"
<script>
Expand All @@ -72,6 +72,6 @@ You can start with no options and let users populate MultiSelect from scratch. I
allowUserOptions="append"
bind:selected
noMatchingOptionsMsg=""
createOptionMsg=""
createOptionMsg={null}
/>
```
6 changes: 3 additions & 3 deletions tests/unit/MultiSelect.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ test.each([[true], [false]])(
)

describe.each([[true], [false]])(`allowUserOptions=%s`, (allowUserOptions) => {
test.each([[`create option`], [``]])(
test.each([[`create option`], [``], [null]])(
`console.error when allowUserOptions is truthy but createOptionMsg is falsy`,
async (createOptionMsg) => {
console.error = vi.fn()
Expand All @@ -935,10 +935,10 @@ describe.each([[true], [false]])(`allowUserOptions=%s`, (allowUserOptions) => {
props: { options: [1, 2, 3], createOptionMsg, allowUserOptions },
})

if (allowUserOptions && !createOptionMsg) {
if (allowUserOptions && !createOptionMsg && createOptionMsg !== null) {
expect(console.error).toHaveBeenCalledTimes(1)
expect(console.error).toHaveBeenCalledWith(
`MultiSelect's allowUserOptions=${allowUserOptions} but createOptionMsg=${createOptionMsg} is falsy. ` +
`MultiSelect has allowUserOptions=${allowUserOptions} but createOptionMsg=${createOptionMsg} is falsy. ` +
`This prevents the "Add option" <span> from showing up, resulting in a confusing user experience.`
)
} else {
Expand Down