rob-balfre / svelte-select

Svelte Select. A select component for Svelte
https://svelte-select-examples.vercel.app
Other
1.25k stars 175 forks source link

Selected items in dropdown not filtered when the selected values are an Array #557

Closed josdejong closed 1 year ago

josdejong commented 1 year ago

I have a case where the values of my Select box are arrays containing paths. When the selected values are not the same Array instance as the paths in the options of the select box (they aren't by default, the data is loaded from somewhere), then the dropdown does not filter the already selected values from the list:

afbeelding

https://svelte.dev/repl/3a6a75691ba84aec9ba0aef21c072c48?version=3.55.1

<script>
  import Select from 'svelte-select'

  export let values = [
    ['path', 'one'],
    ['path', 'two']
  ]

  function toOption(value) {
    return {
      value,
      label: value.join(' ')
    }
  }

  const allValues = [
    ['path', 'one'],
    ['path', 'two'],
    ['path', 'three']
  ]
  let options = allValues.map(toOption)

  // The following introduces an issue: 
  // the dropdown still includes the values that are already selected
  // this is because the values are arrays: they are deep equal, 
  // but not the same Array instance, so === doesn't work, and you need an isDeepEqual comparison
  let selectedOptions = values.map(toOption)

  // A solution is getting the actual value from the option, so the Array instance is the same:
  //   const isDeepEqual = (a, b) => JSON.stringify(a) === JSON.stringify(b)
  //   let selectedOptions = values
  //     .map(value => {
  //       return options.find(option => isDeepEqual(option.value, value))
  //     })
  //    .filter(option => !!option)

  $: values = selectedOptions.map(option => option.value)
</script>

<Select multiple items={options} bind:value={selectedOptions}/>

{#if values}
  <p>
    Selected values: {JSON.stringify(values)}
  </p>
{/if}
rob-balfre commented 1 year ago

https://svelte.dev/repl/899766049f49450ca21ef481d449273b?version=3.55.1

josdejong commented 1 year ago

Ehh, yeah, that is a workaround. So you basically transform the path into a string and back.

I think there are two options here:

  1. Change the documentation to clearly state that value must be a string. So in that case I just have to do the plumbing for my non-string values, converting back-and-forth.
  2. Make <Select /> fully working with value: any as currently documented. I have the feeling it works 99% and it may be little work to fix the few issues it has.

I simply think thatsvelte-select would be much more powerful and user-friendly if it supports non-string values, and more importantly, has two way binding for justValue (#555). Would you be open to PR's improving svelte-select in that direction? Would it involve much work?

rob-balfre commented 1 year ago

@josdejong thanks for the feedback.

value is documented as any because it can be initialised as a string, object, simple array or collection depending on the setup (single vs multi). On initialisation value will get converted to { [itemId]: ... [label] ... } or [ { [itemId]: ... [label] ... } ].

Regarding justValue, having two sources of truth you can bind to wouldn't work in practice. I suggest to wrapping Select in your component to implement the justValue logic you want.

I appreciate your suggestions but there are many many MANY ways people setup these things, currently it's at a nice sweet spot where I can REPL pretty much any setup that is requested.

That said I'm always open to PRs. Changes will need tests and be non-breaking.

josdejong commented 1 year ago

Definitely agree on the single source of truth. I only opt for that to keep value backward compatible.

Just to be clear: my ideal version of the library would be to drop justValue and change value such that it holds the selected value(s) and not the selected option(s).

Do you have a use case where it is actually handy to get back the selected options instead of the values? I've seen more Select libraries taking a similar approach, but I've never needed that myself (often having to do some plumbing from value to option and back. I don't really understand why that would be useful in the first place, and I would like to understand that.

rob-balfre commented 1 year ago

Do you have a use case where it is actually handy to get back the selected options instead of the values? I've seen more Select libraries taking a similar approach, but I've never needed that myself (often having to do some plumbing from value to option and back. I don't really understand why that would be useful in the first place, and I would like to understand that.

Yep. Any metadata you need to send back to the server. We do it all the time in my day job.

Thanks, gonna close the issue but feel free to continue discussing :)

josdejong commented 1 year ago

Thanks for your explanation Rob. Still curious, can you give an example of what meta data you send along the selected value?

davidroeca commented 1 year ago

This is a small complaint about a great library -- and I'll keep using it. But I figure I'd chime in here since I agree.


My biggest problem is when I want to assign value from outside of the select component via bind, most frustratingly when multiple is true:

<script lang="ts">
  export let initialValue: string[] | undefined = undefined
  let data = [{ id: 1, text: 'One', meta: 'hey' }, { id: 2, text: 'Two', meta: 'sup' }]
  let items = data.map(({ id, text }) => ({ value: id, label: text }))
  let value = initialValue?.map(value => ({ value, label: '' })) || []
</script>
<Select
  bind:value
  multiple
  {items}
/>

If I ever wanted to reassign value, I'd have to continue to go through the exercise of adding the label.

justValue is nice, but unfortunately it's read-only so doesn't help in this case.

The metadata that I'd need can easily be accessed in another data structure or constructed like so:

const dataMap = new Map(data.map((item) => [item.id, item]))
for (const selectItem of value) {
  const data = dataMap.get(selectItem.value)
  console.log(data?.meta)
}
joelmukuthu commented 1 year ago

Hi @rob-balfre, I have a similar issue as the others have mentioned and I suppose a way to handle both use cases would be to make justValue writable, so it can be used to set the initial value. Do you have any plans of doing that or would you be willing to accept a PR for that?