feat(byRole): Add description filter

[PaquitoSoft]

What:

Add a new description option to ByRole queries.
This helps finding elements which are aria described.

Why:

We were implementing a React component to display the usual popup notifications and, when reading about accessibility considerations, we found we should use ariadialog role and we also should use aria-describedby to reference the message (spec documentation).

When writing some tests we wanted to have one where we could close an specific notification when a list of them were rendered in a view.

ByRole queries currently support filtering results by accessible name, but no filtering by accessible description.

How:

Since this project is already using the dom-accessibility-api dependency to resolve the accessible name and that library also expose a function to resolve the accessible description, I would like to introduce this proposal where we implement filtering by description when using ByRole queries.

Given some HTML markup like this one:

<ul>
  <li role="alertdialog" aria-describedby="notification-id-1">
    <div><button>Close</button></div>
    <div id="notification-id-1">You have unread emails</div>
  </li>
  <li role="alertdialog" aria-describedby="notification-id-2">
    <div><button>Close</button></div>
    <div id="notification-id-2">Your session is about to expire!</div>
  </li>
</ul>

Now we can find alertdialog elements with this kind of query:

const notification = getByRole('alertdialog', {
  description: 'Your session is about to expire!',
});

I used this PR (when namefiltering was added to ByRole queries) to guide my proposed implementation.

I just wanted to create the proposal with some actual code so maybe it's easier to reason about and maybe there are other thing to take into account that we might discuss as a follow-up of this starting point.

Checklist:

• Documentation added to the
  docs site
• Tests
• TypeScript definitions updated
• Ready to be merged
  

References
These are the documents I used as reference for this proposal:

• RTL docs
• alertdialog spec
• What is an accessible name?
• Accessible Name and Description spec

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2c18fc5:

Sandbox	Source
react-testing-library-examples	Configuration

[codecov]

Codecov Report

Merging #1120 (2c18fc5) into main (6b99a7e) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main     #1120   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        25           
  Lines          970       985   +15     
  Branches       316       322    +6     
=========================================
+ Hits           970       985   +15     

Flag	Coverage Δ
node-12	100.00% <100.00%> (ø)
node-14	100.00% <100.00%> (ø)
node-16	100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/queries/role.js	100.00% <100.00%> (ø)
src/role-helpers.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b99a7e...2c18fc5. Read the comment docs.

[eps1lon]

Very nice, thanks!

We need documentation in https://github.com/testing-library/testing-library-docs/edit/main/docs/queries/byrole.mdx (under "Options") for this to be mergable.

[PaquitoSoft]

Very nice, thanks!

We need documentation in https://github.com/testing-library/testing-library-docs/edit/main/docs/queries/byrole.mdx (under "Options") for this to be mergable.

Hey @eps1lon, you mention to add the new option under the Options section but I noticed the name property is not there (it is explained before that section) so I added on that.

Should I move that explanation to the Options section?
Should another PR (maybe out of current's scope) do the same for name prop?

Thanks!

[eps1lon]

Hey @eps1lon, you mention to add the new option under the Options section but I noticed the name property is not there (it is explained before that section) so testing-library/testing-library-docs#1035.

We should move the name section to options as well. But let's do that in a follow-up

[timdeschryver]

@eps1lon not related to this PR, but should we also do a includeName?
It seems like we're already providing the property here but we're not doing anything with it.

[eps1lon]

I think we originally decide to always include it to help people discover the name option (would have to check the original PR though).

Though I don't think this necessarily applies to description since it's

1. more likely to not be set
2. quite noisy (concerning length) if you don't care about it

[timdeschryver]

I'll take a look at the history.
If we indeed decided to always include it (which makes sense imho), I'll remove the code where we now add the includeName argument.

[eps1lon]

Very well crafted PR. Thanks!

[github-actions]

🎉 This PR is included in version 8.13.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[PaquitoSoft]

Thanks for accepting this little feature and for your help 👍

[vctormb]

I had some tests like this:

<button title="this is the title">click me</button>

screen.getByRole('button', { name: /this is the title/i }) // <- before it was getting by its title

now it only gets by its children

<button title="this is the title">click me</button>

screen.getByRole('button', { name: /this is the title/i }) // <- failing
screen.getByRole('button', { name: /click me/i }) // <- OK

is this problem related to this change?

thanks!