feat(ByRole): Add support for fallback roles #418

iAziz786 · 2019-12-13T21:54:38Z

What: Adding support for *ByRole query to query multiple roles.

Why:

The *ByRole query should support querying elements that have multiple role values defined. the ARIA spec defines the role attribute as a whitespace delimited list of values, of which the engine will choose the first supported value. See w3.org/TR/wai-aria-1.2/#introroles. Querying by role when testing should take this into account and find roles matching any of the items in the list rather than only the entire role attribute.

Closes: #411

How: With the help of a flag named queryFallbacks, *ByRole queries can query multiple roles. If the flag is missing and you're trying to query a role that is located on the right side, then this will throw an error. The working is same as mentioned in (#411 (comment))

Checklist:

Documentation added to the
docs site
I've prepared a PR for types targeting
DefinitelyTyped (I'm not sure whether it's needed or not)
Tests
Ready to be merged

codesandbox-ci · 2019-12-13T21:55:26Z

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 97141f7:

Sandbox	Source
happy-kilby-lmtt6	Configuration

codecov · 2019-12-13T21:57:58Z

Codecov Report

Merging #418 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #418   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          22     22           
  Lines         374    381    +7     
  Branches       87     89    +2     
=====================================
+ Hits          374    381    +7

Impacted Files	Coverage Δ
src/queries/role.js	`100% <100%> (ø)`	⬆️
src/events.js	`100% <0%> (ø)`	⬆️

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 a2c3472...97141f7. Read the comment docs.

iAziz786 · 2019-12-13T22:00:10Z

@kentcdodds Since this is my first PR I need some guidance to merge it ASAP. I am aware of that I have to update the docs. I'm not sure whether to create the PR for it before the merge of the changes or after it

Please let me know if I missed something.

kentcdodds

Thanks! Great job! Just a few comments. Let me know if you have any questions.

kentcdodds · 2020-01-21T18:38:21Z

src/__tests__/element-queries.js

@@ -433,6 +434,7 @@ test('queryAllByRole returns semantic html elements', () => {
  expect(queryAllByRole(/rowgroup/i)).toHaveLength(2)
  expect(queryAllByRole(/(table)|(textbox)/i)).toHaveLength(3)
  expect(queryAllByRole(/img/i)).toHaveLength(1)
+  expect(queryAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)


Suggested change

expect(queryAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)

expect(queryAllByRole('meter')).toHaveLength(1)

expect(queryAllByRole('progressbar')).toHaveLength(0)

expect(queryAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)

kentcdodds · 2020-01-21T18:38:40Z

src/__tests__/element-queries.js

@@ -482,6 +485,7 @@ test('getAll* matchers return an array', () => {
  expect(getAllByDisplayValue(/cars$/)).toHaveLength(2)
  expect(getAllByText(/^where/i)).toHaveLength(1)
  expect(getAllByRole(/container/i)).toHaveLength(1)
+  expect(getAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)


Suggested change

expect(getAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)

expect(getAllByRole('meter')).toHaveLength(1)

expect(getAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)

kentcdodds · 2020-01-21T18:41:07Z

src/__tests__/queries.find.js

+  await expect(
+    findByRole('progressbar', {queryFallbacks: true}),
+  ).resolves.toBeTruthy()
+  await expect(
+    findAllByRole('progressbar', {queryFallbacks: true}),
+  ).resolves.toHaveLength(1)


I can't make a suggestion in GitHub for this, but I would suggest this change to:

await expect(findByRole('meter')).resolves.toBeTruthy() await expect(findAllByRole('meter')).resolves.toHaveLength(1) await expect(findByRole('progressbar', {queryFallbacks: true})).resolves.toBeTruthy() await expect(findAllByRole('progressbar', {queryFallbacks: true})).resolves.toHaveLength(1)

Thanks!

kentcdodds · 2020-01-21T18:42:58Z

Also, sorry that it took me so long to get to this. I took a little break 😅

Thanks for your work!

iAziz786 · 2020-01-22T19:27:03Z

Welcome back from holidays, Kent. I hope it went well.

Thanks for the review. I have lost some track on this, but I'll push the suggested changes ASAP.

iAziz786 · 2020-01-25T08:55:32Z

@kentcdodds I have made the review changes.

kentcdodds · 2020-01-25T15:39:24Z

src/queries/role.js

+            .some(text => matcher(text, node, role, matchNormalizer))
+        }
+        // if a custom normalizer is passed then let normalizer handle the role value
+        if (normalizer) {


Do we have a test specifically covering this case? We should.

I thought it is covered in text-matcher.js.

If this is not the test case you're talking about then can you please guide me where to add the test case for it? That will be very helpful :)

@kentcdodds ping you to remind again.

Gotcha. That's fine. Thank you!

kentcdodds

Looking good! Could you add one test case?

kentcdodds

This is great! Thank you for your help on this @iAziz786!

kentcdodds · 2020-01-28T22:25:51Z

Could you make a pull request to add this to the docs please? https://github.com/testing-library/testing-library-docs

kentcdodds · 2020-01-28T22:25:57Z

@all-contributors please add @iAziz786 code and tests

allcontributors · 2020-01-28T22:26:06Z

@kentcdodds

I've put up a pull request to add @iAziz786! 🎉

kentcdodds · 2020-01-29T04:34:38Z

🎉 This PR is included in version 6.12.0 🎉

The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot 📦🚀

iAziz786 · 2020-01-29T12:12:46Z

Yes, @kentcdodds, I'll surely add this to the docs. I need a little time.

kentcdodds reviewed Jan 21, 2020

View reviewed changes

iAziz786 added 2 commits January 25, 2020 14:23

feat(ByRole): Add support for fallback roles

e2a5f1a

feat(ByRole) first word should match for multiple roles

97141f7

kentcdodds reviewed Jan 25, 2020

View reviewed changes

kentcdodds approved these changes Jan 28, 2020

View reviewed changes

kentcdodds merged commit 91d05c1 into testing-library:master Jan 28, 2020

allcontributors bot mentioned this pull request Jan 28, 2020

docs: add iAziz786 as a contributor #434

Merged

kentcdodds added the released label Jan 29, 2020

iAziz786 deleted the pr/getbyrole-fallback-roles branch February 28, 2020 17:48

iAziz786 mentioned this pull request Feb 29, 2020

Add docs for queryFallbacks for *ByRole in dom-testing-library testing-library/testing-library-docs#386

Closed

	expect(getAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)
	expect(getAllByRole('meter')).toHaveLength(1)
	expect(getAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)

feat(ByRole): Add support for fallback roles #418

feat(ByRole): Add support for fallback roles #418

Uh oh!

Conversation

iAziz786 commented Dec 13, 2019

Uh oh!

codesandbox-ci bot commented Dec 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

iAziz786 commented Dec 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kentcdodds left a comment

Choose a reason for hiding this comment

Uh oh!

kentcdodds Jan 21, 2020

Choose a reason for hiding this comment

Uh oh!

kentcdodds Jan 21, 2020

Choose a reason for hiding this comment

Uh oh!

kentcdodds Jan 21, 2020

Choose a reason for hiding this comment

Uh oh!

kentcdodds commented Jan 21, 2020

Uh oh!

iAziz786 commented Jan 22, 2020

Uh oh!

iAziz786 commented Jan 25, 2020

Uh oh!

kentcdodds Jan 25, 2020

Choose a reason for hiding this comment

Uh oh!

iAziz786 Jan 25, 2020

Choose a reason for hiding this comment

Uh oh!

iAziz786 Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

kentcdodds Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

kentcdodds left a comment

Choose a reason for hiding this comment

Uh oh!

kentcdodds left a comment

Choose a reason for hiding this comment

Uh oh!

kentcdodds commented Jan 28, 2020

Uh oh!

kentcdodds commented Jan 28, 2020

Uh oh!

allcontributors bot commented Jan 28, 2020

Uh oh!

kentcdodds commented Jan 29, 2020

Uh oh!

iAziz786 commented Jan 29, 2020

Uh oh!

Uh oh!

codesandbox-ci bot commented Dec 13, 2019 •

edited

Loading

codecov bot commented Dec 13, 2019 •

edited

Loading

iAziz786 commented Dec 13, 2019 •

edited

Loading