fix(ByRole): filter by name or description when hidden #1161
Conversation
|
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 64e243b:
|
src/__tests__/role.js
Outdated
| @@ -26,7 +26,7 @@ test('by default logs accessible roles when it fails', () => { | |||
| }) | |||
|
|
|||
| test('when hidden: true logs available roles when it fails', () => { | |||
| const {getByRole} = render(`<div hidden><h1>Hi</h1></div>`) | |||
| const {getByRole} = render(`<h1 hidden>Hi</h1>`) | |||
There was a problem hiding this comment.
Let's keep this test. I don't think it's relevant to the change. If there is something important here, lets's add a new test instead so that we can better track what changed
There was a problem hiding this comment.
The idea was to test the case where an element had a name available to be filtered but is hidden, before that change the name was always empty. May we can add a test specifically for that case? What do you think?
There was a problem hiding this comment.
Always add test instead of re-purposing existing tests. Existing tests should only change their assertions if they relied on buggy behavior.
There was a problem hiding this comment.
Thank you! I'm learning about all that tests concepts and patterns.
There was a problem hiding this comment.
@eps1lon can you check this again? :3
69b1b96 to
71fcffb
Compare
71fcffb to
57aa847
Compare
Codecov Report
@@ Coverage Diff @@
## main #1161 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 24 24
Lines 998 998
Branches 326 326
=========================================
Hits 998 998
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
The problem is that the accessible name is different if the element is hidden. See #846 (comment) So this adds a footgun where you just flip |
What: Fixes the ByRole methods to accept filter hidden elements using name and description. Also fixes the logs to show the accessible name in case of not found error. Related to #846
Why: If we have two hidden elements with the same role we can't filter that using the name. Also when we have hidden elements on logs they don't have the name filled, they are always equal
""How: By adding the hidden option when use
computeAccessibleDescriptionandcomputeAccessibleNameto get the accessible description and nameChecklist:
docs site N/A