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

Support abstract roles #1282

Open
Dreamsorcerer opened this issue Dec 24, 2023 · 9 comments
Open

Support abstract roles #1282

Dreamsorcerer opened this issue Dec 24, 2023 · 9 comments

Comments

@Dreamsorcerer
Copy link

Describe the feature you'd like:

It would be useful if we could, for example, get all inputs with getAllByRole('input').
This seems to be one of the abstract roles, and as far as I can tell, these are not supported currently:
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles#6._abstract_roles
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/input_role

Expected result is that <input type="text"> would have both textbox and input roles (as input is a superclass of textbox).

This would make testing a lot more flexible.

@timdeschryver
Copy link
Member

As described in the referenced link, abstract roles should not be used.

We want our test to resemble how users would use the application:

You want to write maintainable tests that give you high confidence that your components are working for your users

While abstract roles don't add any value:

Doing so will not result in any meaningful information being conveyed to assistive technologies or to users.

@Dreamsorcerer
Copy link
Author

Dreamsorcerer commented Dec 28, 2023

@timdeschryver I think you've misread that. It says a developer must not use abstract roles in content. That means you should never put role='input' into your HTML.

That has nothing to do with querying them. It says they are 'use[ed] by browsers to help organize and streamline a document'. This is exactly what we want when testing code. Supporting abstract roles like a browser makes it easier to organise a document in a way that facilitates testing.

jsdom is the browser in our case, and we should be able to test using these abstract roles. It's why they exist...

@Dreamsorcerer
Copy link
Author

Dreamsorcerer commented Dec 28, 2023

The second link also says similarly: It is not to be used by web authors.
Which again only indicates you shouldn't be putting the role in when writing HTML.

We want our test to resemble how users would use the application:

A user would navigate through all the inputs in a form. The browser can jump through all the inputs in the form by using the input role. Requiring a developer to select checkbox, radio, text etc. separately and then figure out the order of those elements in relation to each other is not how a user would use the application.

@timdeschryver
Copy link
Member

Based on the feedback I revisited my answer.
But then I also found an older issue that requested this feature, after some comments we decided not to go through with this change.
For more context see

@Dreamsorcerer
Copy link
Author

Thanks, I've added a comment to that PR covering the later comments that seemed to be against it:
#675 (comment)

I still think it would be beneficial, and adding abstract roles only would still be backwards-compatible.

@Dreamsorcerer
Copy link
Author

@timdeschryver Seems the main issue with the PR was backwards-compatibility (caused by supporting all class hierarchies), and there is agreement now that adding abstract roles (only) would be beneficial. Can we reopen this?

@timdeschryver
Copy link
Member

I'll reopen this so other maintainers can also give their opinion.
If we support this, how would this look like and how will it affect existing tests?

@timdeschryver timdeschryver reopened this Jan 2, 2024
@Dreamsorcerer
Copy link
Author

how would this look like

Internally, components with certain roles would also be assigned an abstract role (i.e. they have multiple roles). So, a textbox or whatever would automatically get assigned the input role. Then querying the abstract role should work and find all components matching that role. e.g. <input type="text"> should be found with both queryByRole('textbox') and queryByRole('input').

I've not looked at that linked PR in detail, so maybe it's just a case of removing the non-abstract roles from the superclass hierarchy, and it might be mergeble like that?

and how will it affect existing tests?

As long as it's only abstract roles, then they've never been supported before, so it doesn't affect any currently working tests (other than providing a chance to refactor them).

The closed PR tried to implement the full class hierarchy, which might be desirable eventually, but would cause backwards-compatibility issues (as some queries would start returning more components than previously).

@Dreamsorcerer
Copy link
Author

Dreamsorcerer commented Jan 2, 2024

I've not looked at that linked PR in detail, so maybe it's just a case of removing the non-abstract roles from the superclass hierarchy, and it might be mergeble like that?

Appears to get a mapping from aria-query:
https://github.com/testing-library/dom-testing-library/pull/675/files#diff-2f6e527aa3ca12c294aca55992bdf1a7089d75ab13f294d1114639a70cfe298dR1

So, probably just need to replace that with our own map, or else filter out the values in that map to a whitelist of abstract roles.

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

2 participants