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: add value prop to commandItem to fix command search #1522

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ahmadghoniem
Copy link

@ahmadghoniem ahmadghoniem commented Sep 14, 2023

This commit aims to fix all the search functionality problems occuring in all of the examples in ui.shadcn.com that have a command (cmdk) component used in it along with users installing new command component from the CLI or manually installing them by copying.
similar to #631 #1450 #173 but more general

live examples from ui.shadcn.com

command.mp4
combobox.mp4
playground-model-selection.mp4
cards-invite.mp4

REASON:

- With the value prop
the way the search functionality works is that if a value prop is provided to CommandItem
<CommandItem value="Calendar">Calendar</CommandItem>
the value prop value's gets stringified and passed to the implicit filter prop callback function as the value argument while search is the current value of the search input on Command

 <Command
  filter={(value, search) => {
    if (value.includes(search)) return 1
    return 0
  }}
/>

see here cmdk-filterProp

backspacing triggers the filter callback function and everything works fine however

- Without the value prop
when omitting the value prop on <CommandItem/>
the value prop takes the textContent of the CommandItem DOM element (see here: cmdk-textContent) and passes it to the filter function and it works TILL you try to use the backspace
backspacing doesn' t trigger the filter function and it produces the bug that you already have seen on the videos above

here's a brief video of solving the combobox example

solving-combobox.mp4

@vercel
Copy link

vercel bot commented Sep 14, 2023

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

A member of the Team first needs to authorize it.

@ahmadghoniem
Copy link
Author

@SoyDiego hey, would love if you can take a look

@vercel
Copy link

vercel bot commented Sep 20, 2023

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

Name Status Preview Comments Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 20, 2023 3:14am

@ahmadghoniem
Copy link
Author

is there anything i can help with so this can get merged
i can see that the error is because of formatting using prettier can you give it another try?
@shadcn

@ahmadghoniem
Copy link
Author

@shadcn @elliot would please take a look
would really love to get this PR merged

@ahmadghoniem
Copy link
Author

If there's something that can be done from my end please let me know
I'm not that great with version control yet

@ahmadghoniem ahmadghoniem changed the title fix: add value prop to commandItem to fix search fix: add value prop to commandItem to fix command search Nov 12, 2023
leonlarsson added a commit to leonlarsson/the-finals-leaderboard that referenced this pull request Dec 22, 2023
@aryanranderiya
Copy link

using shouldFilter={false} in <Command> resolves the issue.

Originally posted by @kulterryan in #1849 (comment)

@CrisAlejo26
Copy link

CrisAlejo26 commented Dec 27, 2024

Hello, sorry, I have a problem with the search in the combobox, it turns out that I not only want to search by the value property, I also want to search by subtitle, but it doesn't work, it doesn't do the search correctly, I thank you if you help me, I want to have several values ​​within an item and search for all of them at the same time, thank you very much:

import { Check, ChevronsUpDown } from 'lucide-react';

import { cn } from '@/lib/utils';
import { useState } from 'react';
import {
	Button,
	Command,
	CommandEmpty,
	CommandGroup,
	CommandInput,
	CommandItem,
	CommandList,
	Popover,
	PopoverContent,
	PopoverTrigger,
} from '../ui';

const frameworks = [
	{
		value: 'next.js',
		label: 'Next.js',
		subtitle: 'React framework',
	},
	{
		value: 'sveltekit',
		label: 'SvelteKit',
		subtitle: 'Svelte framework',
	},
	{
		value: 'nuxt.js',
		label: 'Nuxt.js',
		subtitle: 'Vue framework',
	},
	{
		value: 'remix',
		label: 'Remix',
		subtitle: 'React framework',
	},
	{
		value: 'astro',
		label: 'Astro',
		subtitle: 'Static site generator',
	},
];

export function Combobox() {
	const [open, setOpen] = useState(false);
	const [value, setValue] = useState('');
	const [searchTerm, setSearchTerm] = useState('');


	// Filtrar los frameworks según el término de búsqueda
	const filteredFrameworks = frameworks.filter(
		framework =>
			framework.label.toLowerCase().includes(searchTerm.toLowerCase()) ||
			framework.subtitle.toLowerCase().includes(searchTerm.toLowerCase()),
	);

	return (
		<Popover open={open} onOpenChange={setOpen}>
			<PopoverTrigger asChild>
				<Button
					variant="outline"
					role="combobox"
					aria-expanded={open}
					className="w-[200px] justify-between">
					{value
						? frameworks.find(framework => framework.value === value)?.label
						: 'Select framework...'}
					<ChevronsUpDown className="opacity-50" />
				</Button>
			</PopoverTrigger>
			<PopoverContent className="w-[300px] p-0">
				<Command>
					<CommandInput
						placeholder="Search framework..."
						onValueChange={e => setSearchTerm(e)}
					/>
					<CommandList>
						<CommandEmpty>No framework found.</CommandEmpty>
						<CommandGroup>
							{filteredFrameworks.map(framework => (
								<CommandItem
									key={framework.value}
									value={framework.value}
									onSelect={currentValue => {
										setValue(currentValue === value ? '' : currentValue);
										setOpen(false);
									}}>
									<div>
										<div>{framework.label}</div>
										<div className="text-sm text-muted-foreground">
											{framework.subtitle}
										</div>
									</div>
									<Check
										className={cn(
											'ml-auto',
											value === framework.value ? 'opacity-100' : 'opacity-0',
										)}
									/>
								</CommandItem>
							))}
						</CommandGroup>
					</CommandList>
				</Command>
			</PopoverContent>
		</Popover>
	);
}

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.

4 participants