Skip to content
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

fix: update cmdk breaking changes #3268

Closed

Conversation

ObjectJosh
Copy link

Overview

cmdk v1.0.0 released some breaking changes
This PR fixes @shadcn's usage of cmdk's Command component for the shadcn Combobox component.

  • Updated cmdk v0.2.1 -> v1.0.0
  • Fixed new selector of data-[selected=true] (see release notes above)
  • Wrapped CommandItems in a CommandList (see release notes above)

Fixes #3213 #3051 #3047 #3024 #3021 #3012 #2944 #3256

Before:

After running npx shadcn-ui@latest add combobox there is a
breaking change incompatibility:

Screen Shot 2024-03-29 at 3 41 27 PM

Only wrapping in CommandList fixes the above error, but it's defaulted to disabled (unclickable):

Screen Shot 2024-03-29 at 3 42 12 PM

After:

⭐ Adding both fixes, works as expected!

Screen Shot 2024-03-29 at 3 42 26 PM

Copy link

vercel bot commented Mar 29, 2024

@ObjectJosh is attempting to deploy a commit to the shadcn-pro Team on Vercel.

A member of the Team first needs to authorize it.

@capaj
Copy link

capaj commented Apr 4, 2024

thanks you @ObjectJosh. Tried your proposed changes on my little project, works nicely for my usecase.

@@ -81,26 +82,28 @@ export function ComboboxDemo() {
<Command>
<CommandInput placeholder="Search framework..." />
<CommandEmpty>No framework found.</CommandEmpty>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think CommandEmpty should be inside CommandList

@@ -64,26 +65,28 @@ export default function ComboboxDemo() {
<Command>
<CommandInput placeholder="Search framework..." />
<CommandEmpty>No framework found.</CommandEmpty>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

@coolbaluk
Copy link

👋

Thanks for making the changes!
Any movement on this PR ?

@ObjectJosh
Copy link
Author

👋

Thanks for making the changes! Any movement on this PR ?

@coolbaluk Currently waiting for it to get merged. Not sure if the proposed changes above by @darrenswhite ar e correct. Would like a second opinion, thanks.

@shadcn
Copy link
Collaborator

shadcn commented Aug 5, 2024

Fixed in #2626. (also a duplicate of #2945). If not, feel free to re-open. Thank you.

@shadcn shadcn closed this Aug 5, 2024
@nullnullsieben
Copy link

nullnullsieben commented Aug 5, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug]: combobox search do not work with cmdk 1.0.0
7 participants