-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement aria-tooltip-name
rule
#35
Conversation
Co-authored-by: Michiel Pauw <[email protected]> Co-authored-by: onkar75 <[email protected]>
/** | ||
* Make sure that a elements text is "visible" to a screenreader user. | ||
* | ||
* - Inner text that is discernible to screen reader users. | ||
* - Non-empty aria-label attribute. | ||
* - aria-labelledby pointing to element with text which is discernible to screen reader users. | ||
*/ | ||
function hasAccessibleText(el: Element): boolean { | ||
if (el.hasAttribute("aria-label")) { | ||
return el.getAttribute("aria-label")!.trim() !== ""; | ||
} | ||
|
||
if (!labelledByIsValid(el)) return false; | ||
|
||
if (el.getAttribute("title")) { | ||
return el.getAttribute("title")!.trim() !== ""; | ||
} | ||
|
||
if (el.textContent) { | ||
return el.textContent.trim() !== ""; | ||
} | ||
|
||
return true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could probably be extracted out as a utility as I imagine that other rules implement this pattern often as well.
// Metadata | ||
const id = "aria-tooltip-name"; | ||
const text = "ARIA tooltip must have an accessible name"; | ||
const url = `https://dequeuniversity.com/rules/axe/4.4/${id}?application=RuleDescription`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point we should standardise how this metadata is recorded and exposed.
const tooltips = querySelectorAll("[role=tooltip]", el); | ||
if (el.matches("[role=tooltip]")) tooltips.push(el); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pattern is pretty universal and easy to forget to match the element that's passed into the rule. Maybe we can abstract that away?
LCOV of commit
|
const passes = [ | ||
`<div role="tooltip" id="al" aria-label="Name"></div>`, | ||
`<div> | ||
<div role="tooltip" id="alb" aria-labelledby="labeldiv"></div> | ||
<div id="labeldiv">Hello world!</div> | ||
</div>`, | ||
`<div role="tooltip" id="combo" aria-label="Aria Name">Name</div>`, | ||
`<div role="tooltip" id="title" title="Title"></div>`, | ||
]; | ||
|
||
const violations = [ | ||
`<div role="tooltip" id="empty"></div>`, | ||
`<div role="tooltip" id="alempty" aria-label=""></div>`, | ||
`<div | ||
role="tooltip" | ||
id="albmissing" | ||
aria-labelledby="nonexistent" | ||
></div>`, | ||
`<div> | ||
<div role="tooltip" id="albempty" aria-labelledby="emptydiv"></div> | ||
<div id="emptydiv"></div> | ||
</div>`, | ||
]; | ||
|
||
describe("aria-tooltip-name", async function () { | ||
for (const markup of passes) { | ||
const el = await fixture(html`${markup}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should change all the other rules to use this pattern so that the markup is correct for each rule. Otherwise it's all rendered into memory and then wiped from the DOM after the first test runs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, standardising this whole suite might be the way to go. A developer should then just provide the test cases.
No description provided.