fix: suggest hidden:true with getByRole if element is inaccessible #745
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 a5dfd75:
|
eps1lon
left a comment
There was a problem hiding this comment.
It shouldn't just suggest it but rather say that the/a element is not part of the a11y tree.
The element being removed from the a11y tree has strong implications for users relying on screen readers. It's equivalent to the element being invisible for a sighted user.
I fear people will just randomly add aria-hidden and query for it if we explicitly recommend it.
|
Yes it was something to discuss. I think that we should add this to testing playground too. Maybe we should do a console.warn? While on testing playground show a text ? |
|
what do you think of this ? p.s. I have added some types to diff --git a/src/suggestions.js b/src/suggestions.js
index f584e7b..0d23434 100644
--- a/src/suggestions.js
+++ b/src/suggestions.js
@@ -39,6 +39,8 @@ function getRegExpMatcher(string) {
}
function makeSuggestion(queryName, element, content, {variant, name}) {
+ let warning = ''
+
const queryArgs = [
queryName === 'Role' || queryName === 'TestId'
? content
@@ -51,6 +53,10 @@ function makeSuggestion(queryName, element, content, {variant, name}) {
if (queryName === 'Role' && isInaccessible(element)) {
queryArgs.push({hidden: true})
+ warning = `Element is inaccessible. This means that the element and all his children are invisible to screen readers.
+ If you are using aria-hidden prop make sure this is the right choice for your case.
+ `
+ console.warn(warning)
}
const queryMethod = `${variant}By${queryName}`
@@ -60,6 +66,7 @@ function makeSuggestion(queryName, element, content, {variant, name}) {
queryMethod,
queryArgs,
variant,
+ warning,
toString() {
let [text, options] = queryArgs
diff --git a/types/suggestions.d.ts b/types/suggestions.d.ts
index 17e48d6..8c1e68c 100644
--- a/types/suggestions.d.ts
+++ b/types/suggestions.d.ts
@@ -1,5 +1,13 @@
+export type QueryArg = {
+ [key: string]: RegExp | boolean
+}
+
export interface Suggestion {
queryName: string
+ queryMethod: string
+ queryArgs: QueryArg[]
+ variant: string
+ warning: string
toString(): string
} |
("his" -> "its", "using aria-hidden prop" -> "using the aria-hidden prop", "prop make" -> "prop, make") |
👍 That is way clearer. There's always some risk involved but I hope this won't cause too much damage. |
|
I have added the message as we discussed above 👍 |
Codecov Report
@@ Coverage Diff @@
## master #745 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 24 24
Lines 657 665 +8
Branches 173 175 +2
=========================================
+ Hits 657 665 +8
Continue to review full report at Codecov.
|
|
Looks solid. Thanks! |
|
🎉 This PR is included in version 7.22.5 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
What: If an element is inaccessible
getSuggestedQueryshould suggest to usehidden:truewithgetByRoleWhy: Answering to this issue #744, I wanted to share a link of
testing-playgroundbut the query was not suggested as I expected.How: If the element is inaccessible and the query is
getByRoleI have added toqueryArgsanhidden:trueChecklist:
docs site