-
Notifications
You must be signed in to change notification settings - Fork 779
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(label-title-only): allow hidden labels #3183
Conversation
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.
I'm wondering if we should instead update the labelVirtual
code to no check if the aria-labelledby
element is visible. I'll get back to you once I've had a chance to talk to @WilcoFiers about it.
I tried that as an alternative, however, the build failed when trying to use arialabelledbyText inside labelVirtual |
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.
Alright, talked to Wilco and we decided that aria-hidden
labels should pass, but visually hidden labels should not. That means we can just remove the true
argument in the visualVirtual
call in labelVirtual.
return vNode ? visibleVirtual(vNode) : '';
In conjunction with that change, we should update the description to the label-title-only rule to state what the rule is really looking for: that the element has a visual label.
"description": "Ensures that every form element has a visible label and is not solely labeled using hidden labels, or the title or aria-describedby attributes",
Ok, will update the PR soon |
@straker updated the PR according to your suggestions, please take a look :) |
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.
Great! Thanks for the pr and the updates.
Reviewed for security |
take into account labels with
aria-hidden="true"
when evaluating thelabel-title-only
rule.Closes issue:
fixes #3174