huntabyte / cmdk-sv

cmdk, but for Svelte ✨
https://cmdk-sv.com
MIT License
410 stars 17 forks source link

"Search Key" Property #66

Closed MH1702 closed 3 months ago

MH1702 commented 3 months ago

Currently the "value" property is used both for searching and in the onSelect callback. This is problematic for situations where you have an object with an id and a label and want to search for the label, but still want the value given in the onSelect to be the id.

To work around this you have to do something like this:

<script>
  const options = [
    {
      id: 1,
      label: "A",
    },
    {
      id: 2,
      label: "B",
    },
   {
      id: 3,
      label: "C",
    },
  ]
</script>
<Command.Root>
  <Command.Input />
  <Command.Group>
  {#each options as option}
    <Command.Item
          value={option.label}
          onSelect={(label) => {
             // Find the id of the option by finding the option object through its label and accessing the id
             // In a big list or even a tree of options this could be inefficient and complex 
             console.log(options.find(o => o.label === label).id);
          }}
    >
      {option.label}
    </Command.Item>
  {/each}
  </Command.Group>
</Command.Root>

So instead I propose something like this:

<script>
  const options = [
    {
      id: 1,
      label: "A",
    },
    {
      id: 2,
      label: "B",
    },
   {
      id: 3,
      label: "C",
    },
  ]
</script>
<Command.Root>
  <Command.Input />
  <Command.Group>
  {#each options as option}
    <Command.Item
          value={option.id}
          searchKey={option.label}
          onSelect={(id) => {
             // No find required. Problem solved!
             console.log(id);
          }}
    >
      {option.label}
    </Command.Item>
  {/each}
  </Command.Group>
</Command.Root>

The "searchKey" property is then used when searching for an option instead of the "value" property which is however still provided in the onSelect callback. When not provided the search would still continue to use the "value" property, which makes this completely backwards compatible. It would also allow "value" to be anything (number, object, array, etc.) and not just a string.

niemyjski commented 3 months ago

Yes, you could also have duplicate labels. I want to filter and search on both id and value. Would be nice to be able to set extra data on an item. Then pass all of this information to onSelect

huntabyte commented 3 months ago

You have a reference to the entire object you're iterating over, there's no need to find.

{#each options as option}
    <Command.Item
        value={option.label}
        onSelect={() => {
            doSomething(option.label)
        }}
    >
        {option.label}
    </Command.Item>
{/each}

If the extra data isn't being used by the library then it doesn't make sense for it to go into it. The problem with the whole id thing is that it would need to be explicitly passed, whereas right now it can detect that value via the text content of the item.

Closing as not planned for now - may revisit in the future but right now I don't see anything that can't be accomplished already and doesn't add more bloat to the library itself.

MH1702 commented 3 months ago

Fair enough, somehow had tunnel vision on this I suppose and didnt consider capturing from the "each" context into the onSelect function. I suppose in some highly abstracted contexts this might become an issue. But this works for me for now.