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

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

Open
zikaari opened this issue Dec 16, 2020 · 2 comments
Open
Labels
accuracy Improves the accuracy of how behavior is simulated needs investigation Someone has to do research on this

Comments

@zikaari
Copy link

zikaari commented Dec 16, 2020

Environment

  • @testing-library/user-event version: 12.2.2
  • Testing Framework and version: jest@24.8.0 testing-library/react@11.1.2
  • DOM Environment: default

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 nickserv added the accuracy Improves the accuracy of how behavior is simulated label Dec 22, 2020
@nickserv
Copy link
Member

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

@zikaari
Copy link
Author

zikaari commented Dec 23, 2020

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.

@ph-fritsche ph-fritsche added the needs investigation Someone has to do research on this label Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accuracy Improves the accuracy of how behavior is simulated needs investigation Someone has to do research on this
Projects
None yet
Development

No branches or pull requests

3 participants