testing-library / dom-testing-library

🐙 Simple and complete DOM testing utilities that encourage good testing practices.
https://testing-library.com/dom
MIT License
3.25k stars 460 forks source link

Support abstract roles #1282

Open Dreamsorcerer opened 6 months ago

Dreamsorcerer commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

@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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

Thanks, I've added a comment to that PR covering the later comments that seemed to be against it: https://github.com/testing-library/dom-testing-library/pull/675#discussion_r1438315218

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

Dreamsorcerer commented 6 months ago

@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 commented 6 months ago

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?

Dreamsorcerer commented 6 months ago

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 commented 6 months ago

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.