pacocoursey / cmdk

Fast, unstyled command menu React component.
https://cmdk.paco.me
MIT License
9.04k stars 259 forks source link

Generated items not showing up when erasing #172

Closed Joel-Jensen closed 10 months ago

Joel-Jensen commented 10 months ago

Looks like everything works fine when items are hardcoded but when they are generated from an array or similar it does not work.

Fastest way to replicate is to:

  1. Go to https://cmdk.paco.me/.
  2. Search for "caste" in raycast, erase one letter at a time and watch how programs pop up.
  3. Now go to Vercel
  4. Search for "projecte", you now need to erase everything before you can see anything at all.

And if you print the values

filter={(value, search) => {
  console.log(value, search)
  if (value.includes(search)) return 1
  return 0
}}

You can see that it's not executed anymore once something is filtered out. So seems like the internal data structure doesn't know what options there are when it's not hardcoded.

dschlabach commented 10 months ago

Having this issue as well

dschlabach commented 10 months ago

Did some investigating to see what was going on under the hood. This bug only seems to happen when "value" is not explicitly set. (same as https://github.com/pacocoursey/cmdk/issues/40)

In such cases, useValue attempts to set the value to the textContent of the rendered component.

function useValue(id: string, ref: React.RefObject<HTMLElement>, deps: (string | React.ReactNode | React.RefObject<HTMLElement>)[]) {
  const valueRef = React.useRef<string>()
  const context = useCommand()

  useLayoutEffect(() => {
    const value = (() => {
      for (const part of deps) {
        if (typeof part === 'string') {
          return part.trim().toLowerCase()
        }

        if (typeof part === 'object' && 'current' in part && part.current) {
          return part.current.textContent?.trim().toLowerCase()
        }
      }
    })()

    context.value(id, value)
    ref.current?.setAttribute(VALUE_ATTR, value)
    valueRef.current = value
  })

  return valueRef
}

But once an option is filtered out of the list, it is dropped from the DOM. Thus, there is no textContent to match against, until the input is "" and this reset is triggered:

  function filterItems() {
    if (
      !state.current.search ||
      // Explicitly false, because true | undefined is the default
      propsRef.current.shouldFilter === false
    ) {
      state.current.filtered.count = allItems.current.size
      // Do nothing, each item will know to show itself because search is empty
      return
    }
HasanHaghniya commented 10 months ago

same issue here , any updates?

dschlabach commented 10 months ago

Wasn't able to fix this myself due to time constraints. I'd suggest setting value explicitly for now until there's a workaround

Joel-Jensen commented 10 months ago

Works now, thank you @dschlabach and @WITS! 🙌

mauliksoni110 commented 9 months ago

@Joel-Jensen How to use the fix? I see it's not released yet.

Joel-Jensen commented 9 months ago

@mauliksoni110 Sorry not sure since it's a subdir. Only tested it on the https://cmdk.paco.me/ page and currently waiting for the official release build.

dschlabach commented 9 months ago

I just copied the contents of /src into my repo for now rather than importing from the library

nacho-villanueva commented 9 months ago

Is there a preview version where we can use this fix?