-
-
Notifications
You must be signed in to change notification settings - Fork 278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Added prop to render Radix’ Dialog.Trigger #29
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Thanks for this! I didn't even know Dialog had a Trigger now. I'm hesitant to support it through a Open to suggestions… will need to think on it |
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> |
+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. |
How about an API like
To make this work we can rely on |
# Conflicts: # cmdk/src/index.tsx
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 <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> |
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:For sure I could just add a button by myself and trigger an
changeOpen
, but Radix'Trigger
also keeps control over focus management.