pacocoursey / cmdk

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

[WIP] Added prop to render Radix’ Dialog.Trigger #29

Open gopeter opened 2 years ago

gopeter commented 2 years ago

This PR is WIP since it lacks documentation and tests. But before writing these things I wanted to ensure that the maintainers are OK with this idea/change :-)

Since cmdk is built around Radix' dialog, it would be great if we could use their built-in <Trigger /> functionality. Then we could use it like:

<Command.Dialog
  open={open}
  onOpenChange={setOpen}
  trigger={
    <button>
      <SearchIcon />
      Start searching by pressing CMD-K
    </button>
  }
>
  ...
</Command.Dialog>

For sure I could just add a button by myself and trigger an changeOpen, but Radix' Trigger also keeps control over focus management.

vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
cmdk-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2023 2:21pm
pacocoursey commented 2 years ago

Thanks for this! I didn't even know Dialog had a Trigger now. I'm hesitant to support it through a trigger prop instead of a component, but the alternative is destructuring Dialog a bit (which makes it harder to drop-in and use).

Open to suggestions… will need to think on it

gopeter commented 2 years ago

... but the alternative is destructuring Dialog a bit (which makes it harder to drop-in and use).

Yes, that's why I thought a prop would fit better. But if you don't want this, we could also extract the Portal component and guide users to do something like this:

import * as RadixDialog from '@radix-ui/react-dialog'
import { Command } from 'cmdk'

<RadixDialog.Root open={open} onOpenChange={onOpenChange}>
  <RadixDialog.Trigger />
  <Command.DialogPortal>
    <Command.Input />
    <Command.List>
      <Command.Empty>No results found.</Command.Empty>

      <Command.Group heading="Letters">
        <Command.Item>a</Command.Item>
        <Command.Item>b</Command.Item>
        <Command.Separator />
        <Command.Item>c</Command.Item>
      </Command.Group>

      <Command.Item>Apple</Command.Item>
    </Command.List>
  </Command.DialogPortal>
</RadixDialog.Root>
alexcarpenter commented 1 year ago

+1 here, I have an option to trigger the dialog and would like to restore focus to the trigger after the dialog is closed as Radix Dialog does.

joaom00 commented 1 year ago

How about an API like

<Command.Dialog>
  <Command.DialogTrigger />
  <Command.Input />
  <Command.List />
</Command.Dialog>

To make this work we can rely on React.Children to render the parts in the right place

gopeter commented 1 year ago

... but the alternative is destructuring Dialog a bit (which makes it harder to drop-in and use).

Yes, that's why I thought a prop would fit better. But if you don't want this, we could also extract the Portal component and guide users to do something like this:

import * as RadixDialog from '@radix-ui/react-dialog'
import { Command } from 'cmdk'

<RadixDialog.Root open={open} onOpenChange={onOpenChange}>
  <RadixDialog.Trigger />
  <Command.DialogPortal>
    <Command.Input />
    <Command.List>
      <Command.Empty>No results found.</Command.Empty>

      <Command.Group heading="Letters">
        <Command.Item>a</Command.Item>
        <Command.Item>b</Command.Item>
        <Command.Separator />
        <Command.Item>c</Command.Item>
      </Command.Group>

      <Command.Item>Apple</Command.Item>
    </Command.List>
  </Command.DialogPortal>
</RadixDialog.Root>

I've updated the PR to make the mentioned API working. I'm using it in production like that

joaom00 commented 1 year ago

I've updated the PR to make the mentioned API working. I'm using it in production like that

It's kind of weird passing Command props in Command.DialogPortal. How about extracting the Command too:

<RadixDialog.Root open={open} onOpenChange={onOpenChange}>
  <RadixDialog.Trigger />
  <Command.DialogPortal>
    <Command>
      <Command.Input />
      <Command.List>
        <Command.Empty>No results found.</Command.Empty>

        <Command.Group heading="Letters">
          <Command.Item>a</Command.Item>
          <Command.Item>b</Command.Item>
          <Command.Separator />
          <Command.Item>c</Command.Item>
        </Command.Group>

        <Command.Item>Apple</Command.Item>
      </Command.List>
    </Command>
  </Command.DialogPortal>
</RadixDialog.Root>