feat(checks/label): support ARIA element internals properties#5170
feat(checks/label): support ARIA element internals properties#5170straker wants to merge 5 commits into
Conversation
Convert aria-describedby and aria-labelledby attribute reads in title-only, help-same-as-label, and multiple-label checks to use getResolvedRefs, hasAriaValue, and getAriaValue so that values set via ElementInternals are also detected.
chutchins25
left a comment
chutchins25
left a comment
There was a problem hiding this comment.
Nice work — clean migration to the element-internals helpers, and the new testutils-element tests cover the internals path for each check. Two low-severity questions below, neither blocking.
| const labelText = labelVirtual(virtualNode); | ||
| const title = titleText(virtualNode); | ||
| const ariaDescribedBy = virtualNode.attr('aria-describedby'); | ||
| const ariaDescribedBy = hasAriaValue(virtualNode, 'aria-describedby'); |
There was a problem hiding this comment.
Not a blocker, just a consistency question — hasAriaValue is presence-based (it returns true for an empty aria-describedby=""), whereas the previous .attr('aria-describedby') truthy check treated an empty attribute as absent. So an element with an empty aria-describedby and no title/label would now flip title-only from false to true. Such an element has no accessible name anyway, so the practical impact is negligible — was the presence-based semantics (vs. value-truthy) intended here?
There was a problem hiding this comment.
Agreed. This isn't right.
There was a problem hiding this comment.
Resolved — switched to getAriaValue and check .value, so an empty aria-describedby is treated as absent again.
| // make sure the ONE AT visible label is in the list of idRefs of aria-labelledby | ||
| const labelledby = idrefs(node, 'aria-labelledby'); | ||
| return !labelledby.includes(ATVisibleLabels[0]) ? undefined : false; | ||
| const vNode = getNodeFromTree(node); |
There was a problem hiding this comment.
Not a blocker, just a simplification — getResolvedRefs accepts a raw DOM node and calls nodeLookup internally, so you can pass node directly and drop the getNodeFromTree import/call (const labelledby = getResolvedRefs(node, 'aria-labelledby')). Up to you if the intermediate vNode reads more clearly.
There was a problem hiding this comment.
Resolved — passing node directly to getResolvedRefs and dropped the getNodeFromTree call.
…a-describedby Use getAriaValue instead of hasAriaValue so an empty aria-describedby is treated as absent, and simplify multiple-label to pass the DOM node directly to getResolvedRefs.
3 participants
Updates the
checks/labeldirectory to use the newgetAriaValue,hasAriaValue, andgetResolvedRefsfunctions where it makes sense.Here's a report of the full directory:
checks/label/help-same-as-label- aria-describedby: replacednode.getAttribute+idrefs()withhasAriaValue+getResolvedRefs; sincegetResolvedRefsreturns VirtualNodes, updatedaccessibleText(thing)toaccessibleText(thing.actualNode)checks/label/multiple-label- aria-labelledby: replacedidrefs()withgetResolvedRefs; since VirtualNodes are returned, updated.includes(domNode)to.some(ref => ref?.actualNode === domNode)checks/label/title-only- aria-describedby: replaced.attr()truthy check withhasAriaValuesince only presence is neededchecks/label/alt-space-value- checks alt attribute, not an ARIA propchecks/label/duplicate-img-label- compares accessible names, no direct ARIA attr accesschecks/label/explicit- checks label[for] association, no ARIA attr accesschecks/label/hidden-explicit-label- checks label visibility, no ARIA attr accesschecks/label/implicit- checks implicit label wrapping, no ARIA attr accesschecks/label/label-content-name-mismatch- computes accessible names, no direct ARIA attr accesschecks/label/no-implicit-explicit-label- no direct ARIA attr access (in aria directory, not label)Closes: #5146