Skip to content

Should this case be supported by getByLabelText()? #198

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
ValentinH opened this issue Jan 30, 2019 · 7 comments
Closed

Should this case be supported by getByLabelText()? #198

ValentinH opened this issue Jan 30, 2019 · 7 comments

Comments

@ValentinH
Copy link

It's a question but not a support one, more a feature request 😋

In our app, we are using the material-ui Chip to display radio like buttons. If I'm not mistaking, we set the right aria attributes and roles so they are accessible.

In the following case, I was expecting the getByLabelText('open') to return the div containing the aria-checked:

<div
  aria-checked="false"
  role="radio"
  tabindex="0"
>
  <span>
    open
  </span>
</div>

What do you think?

@kentcdodds
Copy link
Member

I'm not 100% certain, but I think that HTML wont association the span with the div radio button. I'm honestly not sure. If you can find some documentation around the actual standard then maybe we can add that standard to our queries.

@alexkrolick
Copy link
Collaborator

<div
  aria-checked="false"
  aria-labelledby="xyz"
  role="radio"
  tabindex="0"
>
  <span id="xyz">
    open
  </span>
</div>

@sompylasar
Copy link
Collaborator

sompylasar commented Jan 30, 2019

This and the previous discussion about element visibility and interactivity surfaces quite an interesting problem with this library: it does not make a difference between how the app is used with assistive technologies versus with the user eyes. In most cases these UX patterns match 1:1 but in some cases where labels get hidden due to being superfluous for an eye-equipped user they are different and may even require separate testing approaches.

As this library attempts to follow the way screen readers read the control-label associations, I think your example should also be supported (but should be based on role=radio rather than aria-checked).

EDIT: Of course, only if the example is actually accessible and the assistive technologies associate the span with the wrapping div[role=radio]

@kentcdodds
Copy link
Member

@sompylasar is correct. This library is limited by not actually being able to see what (most) users see. In a perfect world that would be how we write our tests (not sure how you'd write tests like that... not even browser-based solutions are capable of writing tests like that AFAIK). So we use the next-best-thing which is the same stuff screen readers rely on. That gets us close enough to be very useful in most situations and it also helps to ensure that your code is accessible which is a big benefit.

I'm happy to encode the standard aria spec in this library (or perhaps another library that this one uses?) but that does seem like quite a time-consuming challenge and my bandwidth wont allow that kind of time 😬

@ValentinH
Copy link
Author

@alexkrolick I thought your suggestion was the key but unfortunately it's not working. Should it?

Based on https://www.w3.org/TR/2016/WD-wai-aria-practices-1.1-20160317/examples/radio/radio.html, I've also tried using the following markup without success (even if it cannot be done with Material-UI as-is):

<div
  aria-checked="true"
  role="checkbox"
  tabindex="0"
>
  Chip 1
</div>

@alexkrolick
Copy link
Collaborator

It should work yes - `getByLabelText("open") should find the span and then find the div that points at its id ("xyz").

We certainly haven't encoded the other aria roles yet, so they won't work, but as Kent said I'm certainly in favor of doing so over time or even as a separate semantic HTML parsing library.

@kentcdodds
Copy link
Member

Closing this for inactivity. If someone wants to work on it in the future feel free to open a new issue. Thanks!

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

No branches or pull requests

4 participants