shadcn-ui / ui

Beautifully designed components that you can copy and paste into your apps. Accessible. Customizable. Open Source.
https://ui.shadcn.com
MIT License
74.56k stars 4.63k forks source link

[Bug]: Command/Combobox TypeError and Unclickable/Disabled items, cmdk breaking change #2944

Closed jhnguyen521 closed 3 months ago

jhnguyen521 commented 8 months ago

Describe the bug

When utilizing a Command component that has been added/updated as of 03/07/2024 ~6PM PST, items cannot be clicked and are disabled. (Additionally some examples using command in the docs are incorrect)

Additionally, <CommandItem>s that are not surrounded by a <CommandList> will crash the application and upon opening the Command component, the error: TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator)) will be thrown.

This is due to cmdk making a new release with breaking changes: https://github.com/pacocoursey/cmdk/releases

Workaround

This can be fixed by pinning cmdk version to 0.2.1 in package.json See https://github.com/shadcn-ui/ui/issues/2944#issuecomment-1985153126

Solution

https://github.com/shadcn-ui/ui/issues/2944#issuecomment-1985153126 for disabled items fix

https://github.com/shadcn-ui/ui/issues/2944#issuecomment-1986020090 for error fix

Please see PR #2945 to fix these issues

Affected component/components

Command

How to reproduce

  1. Run npx shadcn-ui@latest add command after 03/07/2024
  2. Try to use command component

Codesandbox/StackBlitz link

https://codesandbox.io/p/devbox/shadcn-playground-forked-4xxqcw?workspaceId=2d1a1544-9936-4d36-a113-0092357e5e51

Logs

No response

System Info

cmdk v1.0.0

Before submitting

jhnguyen521 commented 8 months ago

Some notes, seems like <CommandItem> needs to be contained somewhere within a <CommandList>. <CommandGroup> alone is no longer sufficient. Upon doing this however, items are now disabled with shadcn/ui

Edit: Without a <CommandList>, the error is: TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator))

abdasis commented 8 months ago

I'm face same issue

matowang commented 8 months ago

+1 Onclick interactions doesn't work

jhnguyen521 commented 8 months ago

Figured out the issue, in cmdk changenotes: The aria-disabled and aria-selected props will now be set to false, instead of being undefined in https://github.com/pacocoursey/cmdk/commit/c57e6b7f81a5796395c7a016d6b1b2aac9591973

It seems that the data-disabled prop is also set to false whereas before the property would not exist at all until being disabled, and data-[disabled]:pointer-events-none will apply even for data-disabled="false".

The fix is to replace data-[disabled] with data-[disabled='true']. This should be backwards compatible too! :) I see that shadcn/ui uses data-[disabled] in numerous places, may be worth fixing this behavior everywhere. Going to make a PR.

collinversluis commented 8 months ago

I encountered a similar problem when using the Combobox, but in my situation, the issue wasn't that it was unclickable or "disabled"; rather, the application would crash whenever the Combobox was opened. This is what I ran into for anyone googling this error:

TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator))

This is also solved by pinning cmdk version to 0.2.1 in package.json for now. Thanks @jhnguyen521

jhnguyen521 commented 8 months ago

I encountered a similar problem when using the Combobox, but in my situation, the issue wasn't that it was unclickable or "disabled"; rather, the application would crash whenever the Combobox was opened. This is what I ran into for anyone googling this error:

TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator))

This is also solved by pinning cmdk version to 0.2.1 in package.json for now. Thanks @jhnguyen521

Hey, I forgot to put in the error, but this is likely because you don't have a <CommandList> enclosing your <CommandItems> as I mentioned above, the cmdk version seems to have changed this behavior.

seems like <CommandItem> needs to be contained somewhere within a <CommandList>. <CommandGroup> alone is no longer sufficient. Upon doing this however, items are now disabled with shadcn/ui

Some of the examples for the Command component are incorrect (which I also copy-pastaed (: ) and don't use a <CommandList> which I've also updated in the PR.

tl;dr Try just putting a <CommandList> around your existing <CommandGroup>s like under the Usage example here: https://ui.shadcn.com/docs/components/command

collinversluis commented 8 months ago

Hey, I forgot to put in the error, but this is likely because you don't have a <CommandList> enclosing your <CommandItems> as I mentioned above, the cmdk version seems to have changed this behavior.

Try just putting a <CommandList> around your existing <CommandGroup>s under the Usage example here: https://ui.shadcn.com/docs/components/command

This is totally the issue, adding the <CommandList> around your <CommandItem>'s resolves the TypeError.

Any thought on if we should be wrapping <CommandList> above <CommandGroup> or below?

Updating the examples will resolve the crash issue for future users, but the "aria-disabled" breaking change will still need to be updated so anyone with this issue should keep 0.2.1 pinned in their package.json until https://github.com/shadcn-ui/ui/pull/2945 is merged by changing

"cmdk": "^1.0.0" or similar

to

"cmdk": "0.2.1"

Thanks again @jhnguyen521!

shainegordon commented 8 months ago

I also had to remove data-[disabled]:pointer-events-none data-[disabled]:opacity-50 from here

const CommandItem = React.forwardRef<
    React.ElementRef<typeof CommandPrimitive.Item>,
    React.ComponentPropsWithoutRef<typeof CommandPrimitive.Item>
>(({ className, ...props }, ref) => (
    <CommandPrimitive.Item
        ref={ref}
        className={cn(
            'relative flex cursor-default select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none aria-selected:bg-accent aria-selected:text-accent-foreground data-[disabled]:pointer-events-none data-[disabled]:opacity-50',
            className,
        )}
        {...props}
    />
))

Which makes me then wonder, what was it doing in the first place :)

jhnguyen521 commented 8 months ago

Any thought on if we should be wrapping <CommandList> above <CommandGroup> or below?

I believe it should be above <CommandGroup> according to all the examples, additionally enclosing <CommandEmpty> but I think it works either way 🤷

"aria-disabled" breaking change will still need to be updated

I don't think so, the Command component doesn't do any special handling around aria-disabled and aria-selected. That should all be handled by the command primitives. Only place I saw that stuff referenced was in the Calendar component.

Edit: nvm, it has attributes for the aria-x. It's just styling, so it doesn't really matter, but I can just update this in my PR

shainegordon commented 8 months ago

I also had to remove data-[disabled]:pointer-events-none data-[disabled]:opacity-50 from here

const CommandItem = React.forwardRef<
    React.ElementRef<typeof CommandPrimitive.Item>,
    React.ComponentPropsWithoutRef<typeof CommandPrimitive.Item>
>(({ className, ...props }, ref) => (
    <CommandPrimitive.Item
        ref={ref}
        className={cn(
            'relative flex cursor-default select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none aria-selected:bg-accent aria-selected:text-accent-foreground data-[disabled]:pointer-events-none data-[disabled]:opacity-50',
            className,
        )}
        {...props}
    />
))

Which makes me then wonder, what was it doing in the first place :)

Ignore this, I missed the post about changing to data-[disabled='true'], so I just replaced those classes accordingly

I''ve also moved the position of <CommandList> as per the examples, and my combo-box is working again as it should

abolajibisiriyu commented 8 months ago

like @shainegordon said, I had to adjust the styles to fix this

const CommandItem = React.forwardRef<
  React.ElementRef<typeof CommandPrimitive.Item>,
  React.ComponentPropsWithoutRef<typeof CommandPrimitive.Item>
>(({ className, ...props }, ref) => (
  <CommandPrimitive.Item
    ref={ref}
    className={cn(
      "relative flex cursor-default select-none items-center rounded-sm px-2 py-1.5 text-base outline-none aria-selected:bg-accent aria-selected:text-accent-foreground data-[disabled='true']:pointer-events-none data-[disabled='true']:opacity-50",
      className
    )}
    {...props}
  />
));

note the change data-[disabled='true']

tommerty commented 8 months ago

+1 Onclick interactions doesn't work

@matowang onClick isn't working for me either, but looking at the PR I figured out to do onSelect instead! Helps me with my scenario, hopefully can do the same for you! For example I was trying to navigate with it, and got it working with something like this:

            <CommandDialog open={open} onOpenChange={setOpen}>
                <CommandInput placeholder="Search..." />
                <CommandList>
                    <CommandEmpty>No results found.</CommandEmpty>
                    <CommandGroup heading="Features">
                        {featureItems.map((item) => (
                            <CommandItem
                                key={item.url}
                                onSelect={() =>
                                    router.push(
                                        item.url.replaceAll(
                                            "$username",
                                            user.username
                                        )
                                    )
                                }
                            >
                                <IconArrowRight className="mr-2 h-4 w-4" />
                                <span>{item.title}</span>
                            </CommandItem>
                        ))}
etc etc

Taking into account the data-[disabled='true'] instead of data-[disabled] fix in the /ui/command component too!

lloydrichards commented 8 months ago

This should be addressed with PR #2945 🤞

benjamin-guibert commented 8 months ago

I had the same issue when updating to cmdk v1. I resolved the issue following your advices:

Thank you! 🙏🏻

itxtoledo commented 8 months ago

shadcn should lock the dependency version to avoid these bugs due to updates

faintblack commented 8 months ago

+1 Onclick interactions doesn't work

Maybe you don't change all the data-[disabled] to data-[disabled='true']. I'm facing the same issues, but when I see it again, I just change one data-[disabled], but it actually have two. When I change it all, it works fine.

Kqan1 commented 8 months ago

The solution I use:

In @/components/ui/command.tsx file, you can change the classname on line 120 as follows: "relative flex cursor-default select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none aria-selected: bg-accent aria-selected:text-accent-foreground data-[disabled=true]:pointer-events-none data-[disabled=true]:opacity-50"

Babailan commented 8 months ago

The component was built on version 0.2.1, but it automatically updated to version 1.0.0, indicating a major release. This could be due to the automatic incrementation of major versions when installing shadcn

Liberty34 commented 7 months ago

I had the same issue and fixed with adding CommandList in the tree (check shadcn docs) I replaced data-[disabled] by data-[disabled='true']. I just don't like that but it's working

Babailan commented 7 months ago

I had the same issue and fixed with adding CommandList in the tree (check shadcn docs) I replaced data-[disabled] by data-[disabled='true']. I just don't like that but it's working

you can downgrade the cmdk into previous version 0.2.1

ZubriQ commented 7 months ago

I had the same issue and fixed with adding CommandList in the tree (check shadcn docs) I replaced data-[disabled] by data-[disabled='true']. I just don't like that but it's working

This. Just wrapped my CommandGroup with CommandList and it works now...

craftogrammer commented 7 months ago

I also had to remove data-[disabled]:pointer-events-none data-[disabled]:opacity-50 from here

const CommandItem = React.forwardRef<
    React.ElementRef<typeof CommandPrimitive.Item>,
    React.ComponentPropsWithoutRef<typeof CommandPrimitive.Item>
>(({ className, ...props }, ref) => (
    <CommandPrimitive.Item
        ref={ref}
        className={cn(
            'relative flex cursor-default select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none aria-selected:bg-accent aria-selected:text-accent-foreground data-[disabled]:pointer-events-none data-[disabled]:opacity-50',
            className,
        )}
        {...props}
    />
))

Which makes me then wonder, what was it doing in the first place :)

thanks for the solution, I also wonder this now 😂😂😂

Malachite40 commented 6 months ago

Still getting this issue, the fixes listed above work - however clicking on a CommandItem doesn't close the popover, leaving a meh UX

joeblau commented 6 months ago

I also had to remove data-[disabled]:pointer-events-none data-[disabled]:opacity-50 from here

const CommandItem = React.forwardRef<
    React.ElementRef<typeof CommandPrimitive.Item>,
    React.ComponentPropsWithoutRef<typeof CommandPrimitive.Item>
>(({ className, ...props }, ref) => (
    <CommandPrimitive.Item
        ref={ref}
        className={cn(
            'relative flex cursor-default select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none aria-selected:bg-accent aria-selected:text-accent-foreground data-[disabled]:pointer-events-none data-[disabled]:opacity-50',
            className,
        )}
        {...props}
    />
))

Which makes me then wonder, what was it doing in the first place :)

thanks for the solution, I also wonder this now 😂😂😂

This solved the issue for me.

samofoke commented 6 months ago

Thanks for the solutions it works after removing data-[disabled]:pointer-events-none data-[disabled]:opacity-50 I wonder if this will be fixed soon.

KhoaLee commented 6 months ago

Thanks for the solutions it works after removing data-[disabled]:pointer-events-none data-[disabled]:opacity-50 I wonder if this will be fixed soon.

This is correct and I hope we will have an official fix soon in next version

Armadillidiid commented 6 months ago

I've used to wrap both and , but I encountered an unusual issue. Upon inspecting my console, I've noticed a multiple warnings indicating that children within the components share the same key.

Warning: Encountered two children with the same key, 44. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version.

This warning isn't isolated to just one item; it applies to all . It appears that attempts to render twice with identical keys. I resolved this by appending the map index to my unique ID. The fact that this solution works confirms that the component mounts twice simultaneously.

That aside, there's a huge lag when the button is clicked to the popover appearing, further confirming my thesis.

<PopoverContent className="w-[300px] p-0">
  <Command>
    <CommandInput placeholder="Search country..." />
    <CommandList>
      <CommandEmpty>No country found.</CommandEmpty>
      <CommandGroup>
        {countries.map((country, idx) => (
          <CommandItem
            key={`${country.value}-${idx.toString()}`} // without `idx`, you'll get duplicate key error
            value={country.label}
            onSelect={() => doStuff()}
            }}
          >
            {country.label}
          </CommandItem>
        ))}
      </CommandGroup>
    </CommandList>
  </Command>
</PopoverContent>
Sholamide commented 6 months ago

0.2.1

clicking on a CommandItem doesn't close the popover

baxsm commented 6 months ago

I've used ^0.2.0 in different projects, works great for me

Sholamide commented 6 months ago

I've used ^0.2.0 in different projects, works great for me

Did you make any modifications to the CommandItem classnames?, here's mine;

<CommandPrimitive.Item ref={ref} className={cn( "relative flex cursor-default select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none aria-[selected='true']:bg-accent aria-[selected='true']:text-accent-foreground data-[disabled='true']:pointer-events-none data-[disabled='true']:opacity-50", className )} {...props} />

baxsm commented 6 months ago

This is what I have

const CommandItem = React.forwardRef<
  React.ElementRef<typeof CommandPrimitive.Item>,
  React.ComponentPropsWithoutRef<typeof CommandPrimitive.Item>
>(({ className, ...props }, ref) => (
  <CommandPrimitive.Item
    ref={ref}
    className={cn(
      "relative flex cursor-pointer select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none aria-selected:bg-accent aria-selected:text-accent-foreground data-[disabled]:pointer-events-none data-[disabled]:opacity-50",
      className
    )}
    {...props}
  />
));

As mentioned above by folks, the difference might be data-[disabled='true'] as I only have data-[disabled] for this version. (^0.2.0)

youneszahzouh commented 6 months ago

Figured out the issue, in cmdk changenotes: The aria-disabled and aria-selected props will now be set to false, instead of being undefined in https://github.com/pacocoursey/cmdk/commit/c57e6b7f81a5796395c7a016d6b1b2aac9591973

It seems that the data-disabled prop is also set to false whereas before the property would not exist at all until being disabled, and data-[disabled]:pointer-events-none will apply even for data-disabled="false".

The fix is to replace data-[disabled] with data-[disabled='true']. This should be backwards compatible too! :) I see that shadcn/ui uses data-[disabled] in numerous places, may be worth fixing this behavior everywhere. Going to make a PR.

This works like a charm

Danilo-A commented 6 months ago

Figured out the issue, in cmdk changenotes: The aria-disabled and aria-selected props will now be set to false, instead of being undefined in https://github.com/pacocoursey/cmdk/commit/c57e6b7f81a5796395c7a016d6b1b2aac9591973

It seems that the data-disabled prop is also set to false whereas before the property would not exist at all until being disabled, and data-[disabled]:pointer-events-none will apply even for data-disabled="false".

The fix is to replace data-[disabled] with data-[disabled='true']. This should be backwards compatible too! :) I see that shadcn/ui uses data-[disabled] in numerous places, may be worth fixing this behavior everywhere. Going to make a PR.

Legend! Thank you 🙏

ARiyou2000 commented 6 months ago

replace data-[disabled]: with data-[disabled=true]: in components/ui/command.tsx

https://stackoverflow.com/a/78308802/11438881

zahidiqbalnbs commented 6 months ago

@jhnguyen521 you are doing great job, such a motivation. I am working on creating Multiselect component in this fashion using Popover (radix-ui/shadcn) with cmdk 1.0.0 but facing the same TypeError issue. I tried using multiple solution from above thread but none worked and issue is randomly occuring. Can you please have a look?

Following is the code.

`import { Command as CommandPrimitive, useCommandState } from 'cmdk' import { X } from 'lucide-react' import * as React from 'react' import { forwardRef, useEffect } from 'react'

import { Badge } from '@/components/shadcn/Badge' import { Command, CommandGroup, CommandItem, CommandList } from '@/components/shadcn/Command' import { cn } from '@/utils'

import { Popover, PopoverContent, PopoverTrigger } from './Popover'

export interface Option { value: string label: string disable?: boolean /* fixed option that can't be removed. / fixed?: boolean /* Group the options by providing key. / key: string: string | boolean | undefined }

interface GroupOption {

}

interface MultiSelectProps { values?: string[] defaultOptions?: Option[] / manually controlled options */ options?: Option[] placeholder?: string /* Loading component. / loadingIndicator?: React.ReactNode / Empty component. */ emptyIndicator?: React.ReactNode / Debounce time for async search. Only work with onSearch. */ delay?: number triggerSearchOnFocus?: boolean /* async search / onSearch?: (value: string) => Promise<Option[]> onChange?: (values: string[]) => void / Limit the maximum number of selected options. */ maxSelected?: number / When the number of selected options exceeds the limit, the onMaxSelected will be called. */ onMaxSelected?: (maxLimit: number) => void /* Hide the placeholder when there are options selected. / hidePlaceholderWhenSelected?: boolean disabled?: boolean groupBy?: string className?: string badgeClassName?: string selectFirstItem?: boolean / Allow user to create option when there is no option matched. */ creatable?: boolean /* Props of Command / commandProps?: React.ComponentPropsWithoutRef /* Props of CommandInput / inputProps?: Omit<React.ComponentPropsWithoutRef, 'value' | 'placeholder' | 'disabled'> }

export interface MultiSelectRef { selectedValue: Option[] input: HTMLInputElement focus: () => void }

export const useDebounce = <T,>(value: T, delay?: number): T => { const [debouncedValue, setDebouncedValue] = React.useState(value)

useEffect(() => { const timer = setTimeout(() => setDebouncedValue(value), delay || 500)

return () => {
  clearTimeout(timer)
}

}, [value, delay])

return debouncedValue }

const transToGroupOption = (options: Option[], groupBy?: string) => { if (options.length === 0) { return {} } if (!groupBy) { return { '': options, } }

const groupOption: GroupOption = {} options.forEach((option) => { const key = (option[groupBy] as string) || '' if (!groupOption[key]) { groupOption[key] = [] } groupOption[key].push(option) }) return groupOption }

const removePickedOption = (groupOption: GroupOption, picked: Option[]) => { const cloneOption = JSON.parse(JSON.stringify(groupOption)) as GroupOption

for (const [key, value] of Object.entries(cloneOption)) { cloneOption[key] = value.filter((val) => !picked.find((p) => p.value === val.value)) } return cloneOption }

const CommandEmpty = forwardRef<HTMLDivElement, React.ComponentProps>( ({ className, ...props }, forwardedRef) => { const render = useCommandState((state) => state.filtered.count === 0)

if (!render) return null

return (
  <div
    ref={forwardedRef}
    className={cn('py-3 text-center text-sm', className)}
    // eslint-disable-next-line react/no-unknown-property
    cmdk-empty=""
    role="presentation"
    onFocus={() => alert()}
    {...props}
  />
)

}, )

CommandEmpty.displayName = 'CommandEmpty'

export const MultiSelect = React.forwardRef<MultiSelectRef, MultiSelectProps>( ( { values, onChange, placeholder, defaultOptions: arrayDefaultOptions = [], options: arrayOptions, delay, onSearch, loadingIndicator, emptyIndicator, maxSelected = Number.MAX_SAFE_INTEGER, onMaxSelected, hidePlaceholderWhenSelected, disabled, groupBy, className, badgeClassName, selectFirstItem = true, creatable = false, triggerSearchOnFocus = false, commandProps, inputProps, }: MultiSelectProps, ref: React.Ref, ) => { const inputRef = React.useRef(null) const [open, setOpen] = React.useState(false) const [isLoading, setIsLoading] = React.useState(false)

const getSelectedOptions = () =>
  (arrayDefaultOptions || arrayOptions).filter((option) => values?.includes(option.value))

const [selected, setSelected] = React.useState<Option[]>(getSelectedOptions() || [])
const [options, setOptions] = React.useState<GroupOption>(transToGroupOption(arrayDefaultOptions, groupBy))
const [inputValue, setInputValue] = React.useState('')
const debouncedSearchTerm = useDebounce(inputValue, delay || 500)

React.useImperativeHandle(
  ref,
  () => ({
    selectedValue: [...selected],
    input: inputRef.current as HTMLInputElement,
    focus: () => inputRef.current?.focus(),
  }),
  [selected],
)

const handleUnselect = React.useCallback(
  (option: Option) => {
    const newOptions = selected.filter((s) => s.value !== option.value)
    setSelected(newOptions)
    onChange?.(newOptions.map(({ value }) => value))
  },
  [onChange, selected],
)

const handleKeyDown = React.useCallback(
  (e: React.KeyboardEvent<HTMLDivElement>) => {
    const input = inputRef.current
    if (input) {
      if (e.key === 'Delete' || e.key === 'Backspace') {
        if (input.value === '' && selected.length > 0) {
          handleUnselect(selected[selected.length - 1])
        }
      }
      // This is not a default behaviour of the <input /> field
      if (e.key === 'Escape') {
        input.blur()
      }
    }
  },
  [handleUnselect, selected],
)

useEffect(() => {
  if (values) {
    setSelected(getSelectedOptions())
  }
}, [values])

useEffect(() => {
  /** If `onSearch` is provided, do not trigger options updated. */
  if (!arrayOptions || onSearch) {
    return
  }
  const newOption = transToGroupOption(arrayOptions || [], groupBy)
  if (JSON.stringify(newOption) !== JSON.stringify(options)) {
    setOptions(newOption)
  }
}, [arrayDefaultOptions, arrayOptions, groupBy, onSearch, options])

useEffect(() => {
  const doSearch = async () => {
    setIsLoading(true)
    const res = await onSearch?.(debouncedSearchTerm)
    setOptions(transToGroupOption(res || [], groupBy))
    setIsLoading(false)
  }

  const exec = async () => {
    if (!onSearch || !open) return

    if (triggerSearchOnFocus) {
      await doSearch()
    }

    if (debouncedSearchTerm) {
      await doSearch()
    }
  }

  void exec()
}, [debouncedSearchTerm, groupBy, open, triggerSearchOnFocus])

const CreatableItem = () => {
  if (!creatable) return undefined

  const Item = (
    <CommandItem
      value={inputValue}
      className="cursor-pointer"
      onMouseDown={(e) => {
        e.preventDefault()
        e.stopPropagation()
      }}
      onSelect={(value: string) => {
        if (selected.length >= maxSelected) {
          onMaxSelected?.(selected.length)
          return
        }
        setInputValue('')
        const newOptions = [...selected, { value, label: value }]
        setSelected(newOptions)
        onChange?.(newOptions.map(({ value }) => value))
      }}
    >{`Create "${inputValue}"`}</CommandItem>
  )

  // For normal creatable
  if (!onSearch && inputValue.length > 0) {
    return Item
  }

  // For async search creatable. avoid showing creatable item before loading at first.
  if (onSearch && debouncedSearchTerm.length > 0 && !isLoading) {
    return Item
  }

  return undefined
}

const EmptyItem = React.useCallback(() => {
  if (!emptyIndicator) return undefined

  // For async search that showing emptyIndicator
  if (onSearch && !creatable && Object.keys(options).length === 0) {
    return (
      <CommandItem value="-" disabled>
        {emptyIndicator}
      </CommandItem>
    )
  }

  return <CommandEmpty>{emptyIndicator}</CommandEmpty>
}, [creatable, emptyIndicator, onSearch, options])

const selectables = React.useMemo<GroupOption>(() => removePickedOption(options, selected), [options, selected])

/** Avoid Creatable Selector freezing or lagging when paste a long string. */
const commandFilter = React.useCallback(() => {
  if (commandProps?.filter) {
    return commandProps.filter
  }

  if (creatable) {
    return (value: string, search: string) => {
      return value.toLowerCase().includes(search.toLowerCase()) ? 1 : -1
    }
  }
  // Using default filter in `cmdk`. We don't have to provide it.
  return undefined
}, [creatable, commandProps?.filter])

return (
  <Popover open={open} onOpenChange={setOpen}>
    <Command
      {...commandProps}
      onKeyDown={(e) => {
        handleKeyDown(e)
        commandProps?.onKeyDown?.(e)
      }}
      className={cn('overflow-visible bg-transparent', commandProps?.className)}
      shouldFilter={commandProps?.shouldFilter !== undefined ? commandProps.shouldFilter : !onSearch} // When onSearch is provided, we don't want to filter the options. You can still override it.
      filter={commandFilter()}
    >
      <PopoverTrigger onFocus={() => inputRef.current?.focus()}>
        <div
          className={cn(
            'group rounded-md border border-input p-[5px] text-base ring-offset-background focus-within:border-blue-700 focus-within:border-2',
            className,
          )}
        >
          <div className="flex flex-wrap gap-1 items-center">
            {selected.map((option) => {
              return (
                <Badge
                  key={option.value}
                  className={cn(badgeClassName)}
                  data-fixed={option.fixed}
                  data-disabled={disabled}
                  variant="subtle"
                >
                  {option.label}
                  <button
                    className={cn(
                      'ml-1 rounded-full outline-none ring-offset-background focus:ring-2 focus:ring-ring focus:ring-offset-2',
                      (disabled || option.fixed) && 'hidden',
                    )}
                    onKeyDown={(e) => {
                      if (e.key === 'Enter') {
                        handleUnselect(option)
                      }
                    }}
                    onMouseDown={(e) => {
                      e.preventDefault()
                      e.stopPropagation()
                    }}
                    onClick={() => handleUnselect(option)}
                    type="button"
                  >
                    <X className="h-3 w-3 text-muted-foreground hover:text-foreground" />
                  </button>
                </Badge>
              )
            })}
            {/* Avoid having the "Search" Icon */}

            <CommandPrimitive.Input
              {...inputProps}
              ref={inputRef}
              value={inputValue}
              disabled={disabled}
              onValueChange={(value) => {
                setInputValue(value)
                inputProps?.onValueChange?.(value)
              }}
              onBlur={(event) => {
                setOpen(false)
                inputProps?.onBlur?.(event)
              }}
              onFocus={(event) => {
                setOpen(true)
                triggerSearchOnFocus && onSearch?.(debouncedSearchTerm)
                inputProps?.onFocus?.(event)
              }}
              placeholder={hidePlaceholderWhenSelected && selected.length !== 0 ? '' : placeholder}
              className={cn(
                'flex-1 bg-transparent outline-none placeholder:text-muted-foreground',
                inputProps?.className,
              )}
            />
          </div>
        </div>
      </PopoverTrigger>
      <PopoverContent className="p-0">
        <CommandGroup className="w-full rounded-md border bg-popover text-popover-foreground shadow-md outline-none animate-in">
          {isLoading ? (
            <>{loadingIndicator}</>
          ) : (
            <>
              {EmptyItem()}
              {CreatableItem()}
              {!selectFirstItem && <CommandItem value="-" className="hidden" />}
              {Object.entries(selectables).map(([key, dropdowns]) => (
                <CommandList key={key} className="h-full overflow-auto">
                  <>
                    {dropdowns.map((option) => {
                      return (
                        <CommandItem
                          key={option.value}
                          value={option.value}
                          disabled={option.disable}
                          onMouseDown={(e) => {
                            e.preventDefault()
                            e.stopPropagation()
                          }}
                          onSelect={() => {
                            if (selected.length >= maxSelected) {
                              onMaxSelected?.(selected.length)
                              return
                            }
                            setInputValue('')
                            const newOptions = [...selected, option]
                            setSelected(newOptions)
                            onChange?.(newOptions.map(({ value }) => value))
                          }}
                          className={cn('cursor-pointer', option.disable && 'cursor-default text-muted-foreground')}
                        >
                          {option.label}
                        </CommandItem>
                      )
                    })}
                  </>
                </CommandList>
              ))}
            </>
          )}
        </CommandGroup>
      </PopoverContent>
    </Command>
  </Popover>
)

}, )

MultiSelect.displayName = 'MultiSelect' `

Error appears randomly not every time [TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator)) which is very annoying and cannot go to production with cmdk 1.0.0.

Moreover I wrote a test case which always gets failed using this MultiSelect component. This test case for sure failed every time with error [TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator))] that catches the issue very easily.

 it('handles keyboard interactions', async () => {
    const onChange = vi.fn()
    const { getByRole } = render(
      <MultiSelect
        options={[
          { value: '1', label: 'Option 1' },
          { value: '2', label: 'Option 2' },
        ]}
        onChange={onChange}
      />,
    )
    const multiselectElement = getByRole('combobox')
    fireEvent.keyDown(multiselectElement, { key: 'ArrowDown' })
    fireEvent.keyDown(multiselectElement, { key: 'Enter' })
  }) 

Any help would be appreciated. Here is sandbox link Issue rarely happens but yes it happens, usually reproduced on backspace/enter pressed in combobox. But test case is failing that's catching the issue every time.

vienpt commented 5 months ago

This is work for me

<FormField
  control={form.control}
  name="language"
  render={({ field, fieldState }) => (
    <FormItem className="flex flex-col">
      <FormLabelValidate
        label={'Language'}
        htmlFor="language"
        invalid={fieldState.invalid}
      />
      <Popover open={open} onOpenChange={setOpen}>
        <PopoverTrigger asChild>
          <FormControl>
            <Button
              variant="outline"
              role="combobox"
              className={cn(
                'w-[200px] justify-between',
                !field.value && 'text-muted-foreground',
              )}
            >
              {field.value
                ? languages.find(
                    (language) => language.value === field.value,
                  )?.label
                : 'Select language'}
              <CaretSortIcon className="ml-2 size-4 shrink-0 opacity-50" />
            </Button>
          </FormControl>
        </PopoverTrigger>
        <PopoverContent className="w-[200px] p-0">
          <Command>
            <CommandInput
              placeholder="Search language..."
              className="h-9"
            />
            <CommandList>
              <CommandEmpty>No language found.</CommandEmpty>
              <CommandGroup>
                {languages.map((language) => (
                  <CommandItem
                    value={language.label}
                    key={language.value}
                    onSelect={() => {
                      form.clearErrors('language')
                      form.setValue('language', language.value)
                      setOpen(false)
                    }}
                  >
                    {language.label}
                    <CheckIcon
                      className={cn(
                        'ml-auto size-4',
                        language.value === field.value
                          ? 'opacity-100'
                          : 'opacity-0',
                      )}
                    />
                  </CommandItem>
                ))}
              </CommandGroup>
            </CommandList>
          </Command>
        </PopoverContent>
      </Popover>
      <FormMessageValidate />
    </FormItem>
  )}
/>
abdulApp commented 5 months ago

CommandList

I had the same issue but after adding CommandList around CommandItem solved the problem. However, I had to change the disabled to equal true ("data-[disabled=true]:pointer-events-none data-[disabled=true]:opacity-50" instead of "data-[disabled]:pointer-events-none data-[disabled]:opacity-50") so I can select the value I want.

(I hope this was a helpful 🙌)

myudak commented 4 months ago

so, are there any other solutions besides a hacky workaround?

kyusungpark commented 4 months ago

This is work for me

<FormField
  control={form.control}
  name="language"
  render={({ field, fieldState }) => (
    <FormItem className="flex flex-col">
      <FormLabelValidate
        label={'Language'}
        htmlFor="language"
        invalid={fieldState.invalid}
      />
      <Popover open={open} onOpenChange={setOpen}>
        <PopoverTrigger asChild>
          <FormControl>
            <Button
              variant="outline"
              role="combobox"
              className={cn(
                'w-[200px] justify-between',
                !field.value && 'text-muted-foreground',
              )}
            >
              {field.value
                ? languages.find(
                    (language) => language.value === field.value,
                  )?.label
                : 'Select language'}
              <CaretSortIcon className="ml-2 size-4 shrink-0 opacity-50" />
            </Button>
          </FormControl>
        </PopoverTrigger>
        <PopoverContent className="w-[200px] p-0">
          <Command>
            <CommandInput
              placeholder="Search language..."
              className="h-9"
            />
            <CommandList>
              <CommandEmpty>No language found.</CommandEmpty>
              <CommandGroup>
                {languages.map((language) => (
                  <CommandItem
                    value={language.label}
                    key={language.value}
                    onSelect={() => {
                      form.clearErrors('language')
                      form.setValue('language', language.value)
                      setOpen(false)
                    }}
                  >
                    {language.label}
                    <CheckIcon
                      className={cn(
                        'ml-auto size-4',
                        language.value === field.value
                          ? 'opacity-100'
                          : 'opacity-0',
                      )}
                    />
                  </CommandItem>
                ))}
              </CommandGroup>
            </CommandList>
          </Command>
        </PopoverContent>
      </Popover>
      <FormMessageValidate />
    </FormItem>
  )}
/>

does scroll work for you?

apethree commented 4 months ago

For anyone new to community to resolve this you need to change this in: components/ui/command.tsx Where you installed the components not in the component you are building 🗡️ . Hopefully this helps someone.

replace data-[disabled]: with data-[disabled=true]: in components/ui/command.tsx
bowen0110 commented 4 months ago

the scrollbar doesn't work

iamisti commented 3 months ago

I also had to remove data-[disabled]:pointer-events-none data-[disabled]:opacity-50 from here

const CommandItem = React.forwardRef<
    React.ElementRef<typeof CommandPrimitive.Item>,
    React.ComponentPropsWithoutRef<typeof CommandPrimitive.Item>
>(({ className, ...props }, ref) => (
    <CommandPrimitive.Item
        ref={ref}
        className={cn(
            'relative flex cursor-default select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none aria-selected:bg-accent aria-selected:text-accent-foreground data-[disabled]:pointer-events-none data-[disabled]:opacity-50',
            className,
        )}
        {...props}
    />
))

Which makes me then wonder, what was it doing in the first place :)

This is very interesting, because in tailwind, you cannot add multiple classes to different states, like hover/active/focus/disabled

paulofabiano commented 3 months ago

Nothing is working for me: scroll, field focus, items mouse over. It is just static and I've applied all workaround suggested in here. I'm using react 18 and nextjs 14.2.4

tjhoo commented 3 months ago

I'm on the same boat as @paulofabiano

May I know what else need to be done beside downgrading cmdk to 0.2.0?

tjhoo commented 3 months ago

I'm on the same boat as @paulofabiano

May I know what else need to be done beside downgrading cmdk to 0.2It.0?

It should be working by downgrading cmdk to 0.2.0?

<Popover>
  <PopoverTrigger asChild>
    <Button>OK</Button>
  </PopoverTrigger>
  <PopoverContent>
    <Command>
      <CommandInput></CommandInput>
      <CommandList>
        <CommandItem key="1" onSelect={() => console.log("first")}>
          123
        </CommandItem>
        <CommandItem key="2" onSelect={() => console.log("second")}>
          456
        </CommandItem>
      </CommandList>
    </Command>
  </PopoverContent>
tuanvt commented 3 months ago

replace data-[disabled]: with data-[disabled=true]: in components/ui/command.tsx

https://stackoverflow.com/a/78308802/11438881

This works well for me. data-[disabled=true]:pointer-events-none data-[disabled=true]:opacity-50

othman316 commented 3 months ago

Figured out the issue, in cmdk changenotes: The aria-disabled and aria-selected props will now be set to false, instead of being undefined in https://github.com/pacocoursey/cmdk/commit/c57e6b7f81a5796395c7a016d6b1b2aac9591973

It seems that the data-disabled prop is also set to false whereas before the property would not exist at all until being disabled, and data-[disabled]:pointer-events-none will apply even for data-disabled="false".

The fix is to replace data-[disabled] with data-[disabled='true']. This should be backwards compatible too! :) I see that shadcn/ui uses data-[disabled] in numerous places, may be worth fixing this behavior everywhere. Going to make a PR.

Works perfectly. Thank you!

televators commented 3 months ago

Been hearing about Shadcn for a while, finally trying it out and one of the first components I try is buggy. Surprised this is still not fixed as of August. Thanks for the fix, though!

RanitManik commented 2 months ago
form.clearErrors('language')

saved me a lot of time. Thanks!

OmarKhattab commented 1 month ago

Seems like an issue still