pacocoursey / cmdk

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

The first item is not selected by default when `Command.List` content is dynamic #280

Open jaxxreal opened 4 months ago

jaxxreal commented 4 months ago

Issue description

When content of Command.List is dynamic eg rendering result of a search API endpoint results the first item is not always selected by default.

In case I am doing something wrong, please let me know.

Setup

shadcn's Command component, cmdk@1.0.0

Repro

Repro steps for stackblitz template I drafted up.

Expected:

  1. Type "air" in the input
  2. When results appear, the first item is highlighted
  3. When no results and Add <query> item appears - it's highlighted since it's the first item in the list

Actual:

  1. Type "air" in the input
  2. When results appear, the first item is NOT highlighted
  3. When no results appear and Add <query> item appears - it's NOT highlighted

EDIT: bringing code sample here, just in case

import { Command, CommandInput, CommandItem, CommandList } from './Command';

export default function App() {
  const [items, setItems] = React.useState([]);
  const [isLoading, setIsLoading] = React.useState(false);
  const [q, setQ] = React.useState('');

  const hasSearchResults = items.length > 0;

  React.useEffect(() => {
    if (!q) {
      return;
    }

    setItems([]);
    setIsLoading(true);

    setTimeout(() => {
      setItems(
        dataArray
          .filter((v) => v.includes(q.toLocaleLowerCase()))
          .map((v, idx) => ({
            name: v,
            id: v + idx,
            onSelect: (value: string) => {
              console.log('Selection was made:', value);
            },
          }))
      );
      setIsLoading(false);
    }, 2000);
  }, [q]);

  return (
    <>
      <h1 className="text-3xl font-bold underline">
        First item default selection issue
      </h1>
      <div>
        <Command
          className="rounded-lg border shadow-md"
          shouldFilter={false}
          loop
        >
          <CommandInput placeholder={'Search...'} onValueChange={setQ} />
          <CommandList className="h-fit">
            {isLoading && (
              <CommandItem key={'spinner'} forceMount>
                Searching...
              </CommandItem>
            )}

            {hasSearchResults &&
              items.map((item) => {
                return <RecordCommandItem key={item.id} {...item} />;
              })}

            {!hasSearchResults && !isLoading && q.length > 0 && (
              <CommandItem
                forceMount
                key="create-new-record"
                value={q}
                onMouseDown={(e) => {
                  e.preventDefault();
                  e.stopPropagation();
                }}
                onSelect={(value: string) => {
                  setQ('');
                  console.log('Created!');
                }}
              >
                {`Add "${q}"`}
              </CommandItem>
            )}
          </CommandList>
        </Command>
      </div>
    </>
  );
}

interface RecordCommandItemProps {
  name: string;
  description: string;
  id: string;
  onSelect: (value: string) => void;
}

function RecordCommandItem({
  name,
  description,
  id,
  onSelect,
}: RecordCommandItemProps) {
  return (
    <CommandItem value={id} onSelect={onSelect}>
      <div className="flex flex-row gap-2 text-sm">
        <div className="font-medium">{name}</div>
        <div className="capitalize text-gray-500">{description}</div>
        <span hidden>{id}</span>
      </div>
    </CommandItem>
  );
}
costaluu commented 4 months ago

Maybe exposing the store object to control the value stored can be a solution, +1 checked this on chrome and firefox

thomaslaberge commented 4 months ago

Same issue for me did you find a solution ?

thomaslaberge commented 3 months ago

Found a workaround, you can add a Key to the CommandList. When the key changes, the component is fully rerendered.

For me i added the query as the key. Everytime the query changes, the command is fully rerendered with the right selected first item.

jaxxreal commented 3 months ago

Found a workaround, you can add a Key to the CommandList. When the key changes, the component is fully rerendered.

For me i added the query as the key. Everytime the query changes, the command is fully rerendered with the right selected first item.

Hello! I just applied changes to the example I've attached but I see no changes in behavior. Am I doing something wrong?

thomaslaberge commented 3 months ago

For your case II add to put the key at the command level : <Command className="rounded-lg border shadow-md" shouldFilter={false} loop key={isLoading.toString().concat(q)}

the general idea is that id a key element is changed, the element will be fuly rerender after a state change. In my case the results list are in a children component. Rerendering this component was enought for my use case. You need to also include the isloading with Q because you want a full rerender when is transition from loading to not loading. This is a woraround, not a best practice

stuartmemo commented 2 months ago

If it helps, I had to add autoFocus to the <CommandInput> to stop input losing focus on re-render.

senadir commented 1 month ago

Adding a key does kinda mess up the performance and behavior of the list, especially when switching from no results to exists results.

Adding the key to Command.List is slightly more stable but not as good.

loggerhead commented 4 weeks ago

Any solution? Still encountering the problem.

xixixao commented 2 days ago

I suspect others like me are trying to use this component to build a nice "typeahead" kind of experience, and this bug totally breaks it :(

xixixao commented 1 day ago

The problem is that the component is used uncontrolled. To fix this, control the Command instance like this:

  const items = ... // result of async loading
  const [activeItem, setActiveItem] = useState<string | undefined>();

  // this is just a classic direct rerender, you could avoid it if you can call
  // setActiveItem from where you load items
  const lastItems = useRef(items);
  if (items !== lastItems.current) {
    lastItems.current = items;
    setActiveItem(items?.[0]?.id ?? undefined);
  }

  return (
    <Command
      shouldFilter={false}
      value={activeItem}
      onValueChange={(itemId) => {
        setActiveItem(itemId);
      }}
    >
       ....
           <CommandItem
            key={item.id}
            value={item.id}