Skip to content

Support option grouping #179

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

Closed
wants to merge 4 commits into from
Closed

Conversation

greendrake
Copy link

This seems to work except for the TS checker ranting about Object.groupBy:

Property 'groupBy' does not exist on type 'ObjectConstructor'.

Not sure how to make it happy.

Copy link

vercel bot commented Jan 28, 2025

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

Name Status Preview Comments Updated (UTC)
vue3-select-component ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 31, 2025 11:21pm

@TotomInc
Copy link
Owner

TotomInc commented Jan 28, 2025

Hey @greendrake, thanks for your contribution.

I've pushed some fixes & improvements on your branch:

  • Added ES2024.Object to TS lib, because Object.groupBy is somewhat recent and not enabled by default on the official Vue TSConfigs.
  • Minor improvements & renaming to keep things consistent.
  • Added a <slot name="group-name" /> to allow customization of the group-name UI

However, the current implementation has an issue with the way navigation works (with arrow keys & focused option state) when we have at least 2 groups.

We are using an index to determine the selected option. This index is used to display the focused state of an option or to determiner which option should be focused next (when navigating with arrow keys).

Since we have a v-for on each option group availableOptionsGrouped, then the index used for the focused option isn't unique anymore. It is the same index for all the available groups.

Here's what we still need to do:

  • Fix focusedOption index so it is unique across all groups
  • Add documentation on the new prop & slots
  • Add a documentation demo page for the groups
  • Add unit-tests to ensure groups are working as expected (test multi-group navigation, option focus, unfocus)

I am a bit busy and away from open-source since a few months (only answering issues). Might pick it up once I'll get more free time.

Sorry, something went wrong.

@greendrake
Copy link
Author

Hey @TotomInc , thank you for quick review. Nice catch re indices. I think I've got it working, though I am not too happy that the new method getFlatIndex is called so many times. Maybe it's better to maintain a Map of Options to their flat indices?

I also tried another approach which is simply to maintain prev and next references on the Options themselves and use them for key navigation instead of indices. But I got stalled by TypeScript woes, I'm not too strong in TS.

@TotomInc
Copy link
Owner

TotomInc commented Apr 9, 2025

There's been a lot of changes in the code recently. I am closing this pull request for the moment.

We might add the option grouping feature later, with proper TypeScript support. I like to see what react-select had done with their types to handle options + groups.

@TotomInc TotomInc closed this Apr 9, 2025
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.

None yet

2 participants