ui icon indicating copy to clipboard operation
ui copied to clipboard

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

Open jhnguyen521 opened this issue 3 months ago • 35 comments

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

  • [X] I've made research efforts and searched the documentation
  • [X] I've searched for existing issues

jhnguyen521 avatar Mar 08 '24 06:03 jhnguyen521

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))

jhnguyen521 avatar Mar 08 '24 06:03 jhnguyen521

I'm face same issue

abdasis avatar Mar 08 '24 06:03 abdasis

+1 Onclick interactions doesn't work

matowang avatar Mar 08 '24 07:03 matowang

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.

jhnguyen521 avatar Mar 08 '24 07:03 jhnguyen521

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

collinversluis avatar Mar 08 '24 16:03 collinversluis

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

jhnguyen521 avatar Mar 08 '24 16:03 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.

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!

collinversluis avatar Mar 08 '24 16:03 collinversluis

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 :)

shainegordon avatar Mar 08 '24 16:03 shainegordon

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

jhnguyen521 avatar Mar 08 '24 16:03 jhnguyen521

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

shainegordon avatar Mar 08 '24 17:03 shainegordon

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']

abolajibisiriyu avatar Mar 09 '24 21:03 abolajibisiriyu

+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!

tommerty avatar Mar 11 '24 10:03 tommerty

This should be addressed with PR #2945 🤞

lloydrichards avatar Mar 11 '24 12:03 lloydrichards

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

  • I added CommandList in the tree;
  • I replaced data-[disabled] by data-[disabled='true'].

Thank you! 🙏🏻

benjamin-guibert avatar Mar 14 '24 19:03 benjamin-guibert

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

itxtoledo avatar Mar 16 '24 02:03 itxtoledo

+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.

faintblack avatar Mar 16 '24 07:03 faintblack

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"

Kqan1 avatar Mar 16 '24 20:03 Kqan1

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

Babailan avatar Mar 18 '24 19:03 Babailan

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

Liberty34 avatar Mar 23 '24 06:03 Liberty34

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

Babailan avatar Mar 23 '24 13:03 Babailan

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...

ZubriQ avatar Mar 29 '24 16:03 ZubriQ

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 😂😂😂

optimbro avatar Apr 06 '24 13:04 optimbro

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

Malachite40 avatar Apr 21 '24 00:04 Malachite40

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.

joeblau avatar Apr 22 '24 18:04 joeblau

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.

samofoke avatar Apr 23 '24 08:04 samofoke

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

KhoaLee avatar Apr 28 '24 11:04 KhoaLee

I've used <CommandList /> to wrap both <CommandEmpty /> and <CommandGroup />, but I encountered an unusual issue. Upon inspecting my console, I've noticed a multiple warnings indicating that children within the <CommandItem /> 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 <CommandItem />. It appears that <CommandItem /> 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>

Armadillidiid avatar Apr 28 '24 15:04 Armadillidiid

0.2.1

clicking on a CommandItem doesn't close the popover

Sholamide avatar May 03 '24 16:05 Sholamide

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

baxsm avatar May 03 '24 16:05 baxsm

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} />

Sholamide avatar May 03 '24 16:05 Sholamide