testing-library / user-event

🐕 Simulate user events
https://testing-library.com/user-event
MIT License
2.19k stars 248 forks source link

`selectOptions` doesn't work when 'listbox' lives outside of select element #522

Open zikaari opened 3 years ago

zikaari commented 3 years ago

Environment

Problem description

selectOptions and deselectOptions does not work if the dropdown menu is a child of some other element, say document.body for example. It's because the current logic looks for options inside the given select element, and in the given scenario, it can't find any and it throws the Value [X] not found in options error.

Suggested solution

Instead of looking for options "inside" the select element straight up, we could first check if the given select element is "pseudo-select" by checking the aria-haspopup attribute and if it has a linked listbox through a shared labelledby pointer. downshift for one, has been tested to work with the proposed setup below.

If both the assertions are positive, then we could handle the selection and deselection like so:

Notice the call signature of selectOptions and deselectOptions; it's 100% compliant and backwards compatible with current signature

const getOptions = (selectToggle: HTMLElement, value: Matcher | Matcher[]): HTMLElement[] => {
  userEvent.click(selectToggle);
  const labelledByIds = selectToggle.getAttribute('aria-labelledby')?.split(/\s/) || [];
  const listboxes = labelledByIds.map(labelledById => document.querySelector(`[role="listbox"][aria-labelledby="${labelledById}"]`));
  const matchingListbox = listboxes.find(listbox => !!listbox) as HTMLElement;
  const values = arrify(value) as Matcher[];
  return values.map(v => within(matchingListbox).getByText(v, { selector: '[role="option"]' }));
};

export const selectOptions = (selectToggle: HTMLElement, value: Matcher | Matcher[]): void =>
  getOptions(selectToggle, value)
    .filter(option => option.getAttribute('aria-selected') !== 'true')
    .forEach(option => option.isConnected && userEvent.click(option));

export const deselectOptions = (selectToggle: HTMLElement, value: Matcher | Matcher[]): void =>
  getOptions(selectToggle, value)
    .filter(option => option.getAttribute('aria-selected') === 'true')
    .forEach(option => option.isConnected && userEvent.click(option));
nickserv commented 3 years ago

This seems like a good approach, can you please submit a PR?

zikaari commented 3 years ago

I might be able to take a shot (will confirm later). However, I want to discuss some problems surrounding this.

The core problem is that useSelect from downshift, and from react-aria, both employ aria-selected attribute very differently.

While downshift uses aria-selected="true" for expressing visual highlight AND for when the item is literally selected, react-aria however DOES NOT mix both concepts, it achieves the visual highlight without setting aria-selected="true" on the item and will only use to express the literal selection of an item.

This issue has been brough up before over at downshift-js/downshift and is still open - [useSelect]: highlightedIndex vs selectedItem. A comment from @silviuaavram suggests that downshifts behaviour is like what w3.org recommends.

This is a problem because now we need to decide whether selectOptions should filter out the items which have aria-selected="true" on them or should it not. Not filtering the ones with aria-selected="true" could end up in deselection in case of react-aria and filtering them could mean they would not get selected in case of downshift because it was just a visual highlight.

I hope this thread can be used as meeting ground between all parties and perhaps result in a more consistent ecosystem altogether:

I don't know how who to cc for anyone outside of React, it'd nice to see what's happening in the world of Angular, Vue etc. because user-event library is framework agonostic.