search

skeletonlabs / skeleton

A complete design system and component solution, built on Tailwind.
https://skeleton.dev
MIT License
5.15k stars 321 forks source link

Combobox does not show results with `multiple` #2963

Open MrVauxs opened 1 week ago

MrVauxs commented 1 week ago

Current Behavior

Combobox does not fill the input with the selected choices if multiple is enabled true.

This also inadvertently causes required to always cause the input to fail the check.

Expected Behavior

Combobox filling the input with the given array, would be nice to be able to provide a function for the separator used (so you can have something like x, y, and z instead of plain .join(", ").

Steps To Reproduce

No response

Link to Reproduction / Stackblitz

No response

More Information

Earlier Discord Report for History: https://discord.com/channels/1003691521280856084/1191790107867488316/1303464712981184584

endigo9740 commented 1 week ago

FYI I noticed this too when testing the feature originally. It's always possible I was doing something wrong. But if not, then that indicates it may be a bug on Zag's end and would need to be reported upstream. Let's review first to confirm, but just a heads up!

MrVauxs commented 1 week ago

FYI I noticed this too when testing the feature originally. It's always possible I was doing something wrong. But if not, then that indicates it may be a bug on Zag's end and would need to be reported upstream. Let's review first to confirm, but just a heads up!

I checked, the official React Zag StackBlitz instance works with multiple once you add it to the controls. https://zagjs.com/components/react/combobox https://stackblitz.com/run?file=src%2FApp.tsx,src%2FCombobox.tsx&showSidebar=0

endigo9740 commented 1 week ago

@MrVauxs can you expand on what you mean by "controls". Unfortunately your Stackblitz is non-functional for me. Just links to their homepage.

MrVauxs commented 1 week ago

@MrVauxs can you expand on what you mean by "controls". Unfortunately your Stackblitz is non-functional for me. Just links to their homepage.

Oh, the stackblitz is supposed to be what is linked on the Zag page.

And what I meant is that the StackBlitz example Zag has, just doesnt have multiple (amongst many other stuff) as an available ComboboxProps property.

endigo9740 commented 1 week ago

I'm still completely lost here. multiple is a feature that Zag supports. Here it is in their documentation: https://zagjs.com/components/react/combobox#selecting-multiple-values

But when implemented on our component, it's not working. Which is why this ticket was all about, correct? Maybe put Stackblitz aside and share whatever code you're referring to here please.

MrVauxs commented 1 week ago

FYI I noticed this too when testing the feature originally. It's always possible I was doing something wrong. But if not, then that indicates it may be a bug on Zag's end and would need to be reported upstream. Let's review first to confirm, but just a heads up!

You weren't sure if this was a bug in Skeleton, or a bug in Zag. I was confirming that it is Skeletons, with an added comment about Zag's official example having outdated types but nonetheless working when you add the multiple property to their component.