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

refactor: ui experience adjusts and shadcn aligns #9

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

LucasHang
Copy link

Thanks for the great component, saved me a lot of time!

While testing the component in a more complex form context I saw somethings that could be improved.
I see that some changes may be too personal, so I will discribe them here so you can decide better what really matters to you:

  • Fixed selectables filtering. It is currently comparing objects, it works for static objects like in the component example, but when using states the object comparison gets buggy.

  • Adjusted the Input minimun height to match shadcn Input component

  • Add maximun height + overflow to selected options box. So that with large lists of selected options it has a better presentation

  • Moved the selectables box all inside ternary and changed margin top by translate (just like shadcn does for the Select component). In the current way, with margin top it produces align errors, look:
    multi-select-disalign

  • Moved the z-index property to selectables parent box (and increased it to the same value shadcn uses in Select component). For me it was not working with z-index on the child, look:
    multi-select-z-index

  • Add maximun height to CommandGroup box, so large lists of options have a better presentation

  • Removed unused "value" prop in CommandItem onSelect() function

That's all. Let me know if made any stupidity.
Thanks again.

@vercel
Copy link

vercel bot commented Jul 22, 2023

Someone is attempting to deploy a commit to a Personal Account owned by @mxkaske on Vercel.

@mxkaske first needs to authorize it.

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.

1 participant