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

feat: adding multi-select component #1616

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

Conversation

dinogit
Copy link

@dinogit dinogit commented Sep 27, 2023

This PR creates a multi-select component from existing shadcn-ui components.

@vercel
Copy link

vercel bot commented Sep 27, 2023

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

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Oct 21, 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 Oct 23, 2023 6:30am

@izakfilmalter
Copy link

Looking forward to this.

  • Seems that clicking on the x on the badge toggles the menu closed
    CleanShot 2023-10-21 at 08 44 53

  • The hover state on the badge on dark mode is the same color as the input causing them to disappear.
    CleanShot 2023-10-21 at 08 44 33

  • Badge alignment within the input is off as well.
    CleanShot 2023-10-21 at 08 47 46

  • Would be sick if backspace on empty input deleted

@shadcn
Copy link
Collaborator

shadcn commented Oct 21, 2023

This looks great. Adding it to the roadmap.

@izakfilmalter has some feedback if you have a chance to look at it.

@shadcn shadcn added new component area: roadmap This looks great. We'll add it to the roadmap, review and merge. postpone: more info or changes requested maintainers asked a question or needs more info labels Oct 21, 2023
@izakfilmalter
Copy link

@dinogit Looking a lot better. This interaction is still really awkward:
CleanShot 2023-10-23 at 07 23 28

I would probably disable the deselection of items while the dropdown is closed.

@izakfilmalter
Copy link

Backspace to remove is really wonky. It needs to check if the input is empty:
CleanShot 2023-10-23 at 07 25 59

@dinogit
Copy link
Author

dinogit commented Oct 23, 2023

Hi @izakfilmalter, thank you for your time and feedback.

regarding the comment above, the update is that deselection is removed from the Badge and only available on the Close button when the dropdown is open. Close button in the Badge also apears and disapears when dropdown is open/closed.

Delete will remove badge if Search is not populated, othervise it will delete letters in search input.

Let me know what do you think

@nemanja-alloy
Copy link

nemanja-alloy commented Nov 28, 2023

For me this multi select has major bug.

image

Having multiple fields in form will cause deletion of multi select tags once you modify other input fileds.

Property is MultiSelect

Using backspace to edit email / username will delete each selected tag one by one on every Backspace hit.

This is inside Sheet component.

<Command className={className}>
<CommandInput
onValueChange={(item) => {
console.log("dd", item)

Choose a reason for hiding this comment

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

Remove console.log

export type OptionType = Record<"value" | "label", string>

interface MultiSelectProps {
options: Record<"value" | "label", string>[]

Choose a reason for hiding this comment

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

Why not use the OptionType here that is defined just above?

Comment on lines +42 to +46
React.useEffect(() => {
const handleKeyDown = (e: KeyboardEvent) => {
if (e.key === "Backspace" && query === "" && selected.length > 0) {
onChange(selected.filter((_, index) => index !== selected.length - 1))
}

Choose a reason for hiding this comment

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

This backspace act as soon as the component is loaded it should not remove an item if I am using backspace to remove text in another input field

Copy link

Choose a reason for hiding this comment

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

This should do the trick (works for me)

Suggested change
React.useEffect(() => {
const handleKeyDown = (e: KeyboardEvent) => {
if (e.key === "Backspace" && query === "" && selected.length > 0) {
onChange(selected.filter((_, index) => index !== selected.length - 1))
}
React.useEffect(() => {
const handleKeyDown = (e: KeyboardEvent) => {
if (document.activeElement === commandInputRef.current) {
if (e.key === 'Backspace' && query === '' && selected.length > 0) {
onChange(selected.filter((_, index) => index !== selected.length - 1));
}
}

Plus declaring commandInputRef

const commandInputRef = React.useRef<HTMLInputElement>(null);

and adding ref={commandInputRef}

<CommandInput
  ref={commandInputRef}
  onValueChange={(item) => {
    setQuery(item);
  }}
  placeholder="Search ..."
/>

{item.label}
{open && (
<Button
asChild
Copy link

@jonatan-kruse jonatan-kruse Dec 3, 2023

Choose a reason for hiding this comment

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

I don't know why but this asChild crashed my website with this error message:

Uncaught Error: React.Children.only expected to receive a single React element child.
    at Object.onlyChild [as only] (react.development.js:1229:11)
    at Slot.tsx:70:62
    at renderWithHooks (react-dom.development.js:16305:18)
    at updateForwardRef (react-dom.development.js:19226:20)
    ...

role="combobox"
aria-expanded={open}
className={`group w-full justify-between ${
selected.length > 1 ? "h-full" : "h-10"

Choose a reason for hiding this comment

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

This resulted in quite a buggy ui for me, i would sugest

min-h-[2.5rem] h-fit

@SaadBazaz
Copy link

SaadBazaz commented Dec 5, 2023

Just asking the community to see what they'd like...

Would you like an AutoComplete with Multiselect + FreeSolo capabilities in Shad, like the following?

Screen.Recording.2023-12-06.at.00.08.03.mov

(You can test this out at https://app.listen.dev)

If yes, we can try making this generic enough to merge into shadcn mainnet. I think with some tweaks, it can be moulded to do whatever a user wants to within multiselect/freesolo/autocomplete as they want. Kinda like MUI's AutoComplete.

@simicvm
Copy link

simicvm commented Dec 29, 2023

I am getting the following Issue in Chrome when using the provided example multi-select-form.tsx.

Incorrect use of <label for=FORM_ELEMENT>
The label's for attribute doesn't match any element id. This might prevent the browser from correctly autofilling the form and accessibility tools from working correctly.

To fix this issue, make sure the label's for attribute references the correct id of a form field.

The issue is that <label> gets the for attribute, but none of the form elements get the matching id attribute (or any id attribute for that matter).

For example, on the Input component, both the <label> and <input> elements get the matching id and no issue is raised.

My website still works fine though and values are passed properly.

variant="outline"
size="icon"
className={`border-none duration-300 ${
open ? "opacity-100 ease-in" : "opacity-0 ease-out"
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary conditional, open value is always truthy
because button only visible when open is true

Suggested change
open ? "opacity-100 ease-in" : "opacity-0 ease-out"
opacity-100 ease-in

@shadcn
Copy link
Collaborator

shadcn commented Mar 4, 2024

fyi: we have two multi-select components that we're looking into: #2773

@sersavan
Copy link

Hi everyone,
Thank you for developing such a wonderful library—it's truly the best! In relation to this topic, I would like to share an example of a multi-select component that I've assembled using shadcn's components. I appreciate your feedback.
https://shadcn-multi-select-component.vercel.app/

? selected.filter((item) => item.value !== option.value)
: [...selected, option]
)
setOpen(true)

Choose a reason for hiding this comment

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

set it to open?
if you are clicking it, the popover is already open and visible, am i missing something?

<Command className={className}>
<CommandInput
onValueChange={(item) => {
console.log("dd", item)

Choose a reason for hiding this comment

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

remove console.log

@ilbertt ilbertt mentioned this pull request Dec 17, 2024
@sovetski
Copy link

Any updates please? a lot of developer are waiting for this..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: roadmap This looks great. We'll add it to the roadmap, review and merge. new component postpone: more info or changes requested maintainers asked a question or needs more info
Projects
None yet
Development

Successfully merging this pull request may close these issues.