-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Unable to find role #835
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
Comments
To be honest I deleted the comment because I just read that you said it was working before, so the library maintainers would need to comment as to why it was changed. |
Yeah I understand now from your previous comment that it's not a valid accessibility role. I'm having a hard time targeting this because it's a button with an SVG as the content, so I can't use text within the button to target it. |
But your button *should* have an aria-label for screen readers so why not
just add one, like 'Execute' for example? Then you could do `getByRole('button', { name: 'Execute' });`
…On Thu, 19 Nov 2020 at 17:46, ngoble-phdata ***@***.***> wrote:
Yeah I understand now from your previous comment that it's not a valid
accessibility role. I'm having a hard time targeting this because it's a
button with an SVG as the content, so I can't use text within the button to
target it.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#835 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AASLMLWEGX6PPFITICY6K6TSQVKZFANCNFSM4T3WB73A>
.
|
@brendan-donegan is correct. Applying an invalid role is very bad for accessibility. Don't do it. Use a label instead. If for some reason that doesn't work, use a Thanks! |
There should still be documentation that calls out this breaking change in the library. This worked in previous versions. |
Technically all bug fixes are breaking changes for people who relied on the bug. The fact it worked previously is a bug. I'm not sure why the test ID isn't getting applied in jsdom. That will be a jsdom issue though I'm afraid. Any reason the aria-label idea isn't going to work for you? Consider that a blind user won't know what this is if it has no label. |
Turns out the reason it wasn't getting added was unrelated to this library. I had to set the jsdom window size in order to have the desktop variant shown. Thank you for your time, and updates on accessibility functionality. |
This fails for valid roles as well. It's overeager in ignoring roles. For example, when creating "listbox" roles out of lists for selects that don't use the native element, as is standard in many UI libraries like material-ui. Frankly, the opinionated nature of testing-library is highly frustrating because it edges out all kinds of valid, common use cases. I'd like to decide as the dev and designer when a role is valid or not, not a library. These decisions are basically forcing me into data-testids. |
@gabrielliwerant If you have a specific use case where we fail on valid roles, please open a different issue as your issue isn't related to this one :) |
@testing-library/react
version: 11.2.0Problem description:
Unable to find role="execute-button000"
It says it can't find that role, but as you can see in the debug print, it clearly exists in the DOM. This also fails for
screen.findByRole
This seems to be happening to any component I add a custom role to, and was working in previous versions.
The text was updated successfully, but these errors were encountered: