ember-headlessui icon indicating copy to clipboard operation
ember-headlessui copied to clipboard
verified publisher icon GavinJoyce

support listbox with multiple selection

Open miguelcobain opened this issue 3 years ago • 6 comments

I ported all the tests for the multi-select from https://github.com/tailwindlabs/headlessui/blob/main/packages/%40headlessui-react/src/components/listbox/listbox.test.tsx

Let me know your feedback, please.

miguelcobain avatar Nov 23 '22 01:11 miguelcobain

A couple more changes that this PR makes:

  • we were not setting aria-selected="false" on unselected options for listbox and combox (this differs from headlessui). This was fixed for listbox and combobox.
  • when we used assertListboxOption({ selected: false }, we were not asserting anything with regards to the selected state of the option (neither the absence of aria-selected or the presence of aria-selected="false). Not we're testing that the option has aria-selected="false". This was fixed for listbox and combobox.
  • tabindex should only be -1 if the option isn't disabled. This was fixed for listbox and combobox.

miguelcobain avatar Nov 23 '22 02:11 miguelcobain

Hey @miguelcobain :wave: I just want to let you know that, I am just in the process of simplification active / selected *guid usage in <Listbox/> component. As you already noticed (and changed!) in this PR selectedOptionGuid is not necessary to be kept in parent component, similar story is with activeOptionGuid. I think my PR will simplify a lot this one – as no need to juggle with selectedOptionIndexes.

far-fetched avatar Dec 16 '22 07:12 far-fetched

Is there anything that needs to be done on this still or is it something that can be merged? I'm in need of multi-select support for the listbox and was going to fork this repo to get it. But I would love for this to actually be merged and released.

barryofguilder avatar Jun 15 '23 20:06 barryofguilder

I would really love for this to get merged as I have a fork of this addon just for this PR. Is there any reason this can't be merged? Does it need updates before it can?

barryofguilder avatar Nov 27 '23 16:11 barryofguilder

@NullVoxPopuli Is this something that can be merged?

barryofguilder avatar Oct 16 '24 18:10 barryofguilder

There are conflicts, I'd say it's up to @GavinJoyce and @miguelcobain

Personally, my efforts are focused on not mimicking tailwind's patterns over here:

https://ember-primitives.pages.dev/

NullVoxPopuli avatar Oct 16 '24 18:10 NullVoxPopuli