ui
ui copied to clipboard
shadcn-ui
fix: Command/Combobox TypeError and Unclickable/Disabled items + Documentation
Fixes #2944 due to latest cmdk update with breaking changes for the Command component. Until this fix is merged, all new command components that install [email protected] will be broken and items will not be selectable. People who also have existing components and update to cmdk 1.00 and don’t have a <CommandList> will experience crashes upon opening up the component. See issue above for more details.
- Additionally handling the data-disabled property better in all components, should be backwards compatible
^ Happy to not do this for all other components, but I figure it's better to just do this preventatively if it's backwards compatible. Either way, things will break for data-disabled="false" for all existing
data-[disabled] - Applied same fix for aria-selected
- Fixing broken examples for command component
@jhnguyen521 is attempting to deploy a commit to the shadcn-pro Team on Vercel.
A member of the Team first needs to authorize it.
![vercel[bot] avatar](https://avatars.githubusercontent.com/in/8329?v=4 "Published by a pub.dev verified publisher"){kind=small}
This PR handles the proper styling via data attributes + dependancy bump + update registry.
https://github.com/shadcn-ui/ui/pull/2626
Fixed crlf line endings to lf. May be worth documenting this in contributing, but pnpm seems to output files with \r\n instead of just \r on windows machines, ignoring tsconfig compiler options
The component specific issues have already been fixed in https://github.com/shadcn-ui/ui/pull/2626 🤔
The component specific issues have already been fixed in #2626 🤔
After you later added the bug fix from this and the other issue afterwards in a later commit, then yes if that counts as "already" :) Also, aria is already set in cmdk and we can be relatively certain that will always be there for accessibility reasons, not sure how opinionated we should be about using their aria/data attribute. Personally, I think it's fine to leave as is but what do I know.
Also, this PR updates necessary documentation to handle other breaking changes in cmdk.
I am not too sure why you are hinting that I have "copied" your work or something?
With all due respect, the original #2626 PR was opened in January as a follow up from #1980 that was opened in November 2023. I have been monitoring this via an issue opened in the cmdk repo itself.
After my initial comment on this PR, I noticed that you made some extra commits, specifically, extra changes that were made in #2626 that had not been made in this PR. I see that these commits have now been wiped out as you cleaned the commit history...
You also commented on my PR which you have since deleted...I don't understand
After you later added the bug fix from this and the other issue afterwards in a later commit
Here is the commit you speak of...https://github.com/shadcn-ui/ui/pull/2626/commits/430c1b3a2f5fe2bddd4e40086a16ffd7b31dc285
After my initial comment on this PR, I noticed that you made some extra commits, specifically, extra changes that were made in https://github.com/shadcn-ui/ui/pull/2626 that had not been made in this PR. I see that these commits have now been wiped out as you cleaned the commit history...
Yes, the change I made were applying the bug fix to the aria-selected attributes, which is the purpose of this PR? (which I might add, was also discussed and brought to my attention by someone else in #2944, so not sure what you are trying to re-imply? lol) Whereas your PR was originally just changing aria-selected -> data-selected and doing a version bump. Huh? Cleaned the commit history? Probably because just did a rebase after some new stuff was merged in, but all commits should be as they have...
You also commented on my PR which you have since deleted...I don't understand
My comment was on being opinionated about aria-selected vs data-selected, but I realized that I was incorrect about some assumptions I was saying that made my comment irrelevant so I just deleted it :)
All I'm saying is that, it's not very cool to come in saying "already been fixed here 🤔", after applying a fix that wasn't there in the original commit an doesn't have any changes around the documentation for this component, when the purposes of our PRs are different, especially when I've been discussing with others in #2944 to workaround the issue.
Either way, I was initially annoyed, but if maintainers just want to use the earlier commit, happy to close mine and you can merge in the doc changes or I just make another PR with that. 🤷
For many components which are not from cmdk the applied changes do not work and cause the disable state to not work anymore. Means the fix: data-[disabled='true'] does not work for all components.
Good catch, thanks! I'll probably just revert changes to all other components since I'd probably have to make a custom class in css to get this to work for all while still being in the classNames. Should make for a simpler PR as well.
Same Issue on the Example Filter Function, that dont works anymore too.
@jhnguyen521 thank you! I am pretty new to shadcn and figured it will lead to a crash if I follow the official example in ui/apps/www/app/(app)/examples/forms/account/account-form.tsx for the language selector.
I had to wrap CommandGroup with CommandList to avoid the crash. However, none of the selection was selectable.
Hopefully this fix will be merged into the main branch soon! kudos to you.
Copied the code from the official website and found out it's not working with [email protected], maybe we can lock the package version with the install instruction?
@shadcn check out #2626 which is a follow up of #1980. Both pending for a long time addressing this issue.
https://github.com/shadcn-ui/ui/pull/2945#issuecomment-1988984058
@jhnguyen521 Any chance you can fix the conflicts here please? Let's merge.
@shadcn Conflicts are fixed :)
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| ui | ✅ Ready (Inspect) | Visit Preview | Aug 5, 2024 11:23am |
@kevinmitch14 I merged https://github.com/shadcn-ui/ui/pull/2626. Anything else we're missing? \cc @jhnguyen521
@shadcn it looks like some changes made in #2626 were reverted in https://github.com/shadcn-ui/ui/pull/4181/ by accident. (command.tsx, model-selector.tsx, cmdk package was reverted back to v0.2.0 files). I don't think this PR bumps it back to latest