-
Notifications
You must be signed in to change notification settings - Fork 35
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
When title is missing, should aria-hidden be "false" or default? #294
When title is missing, should aria-hidden be "false" or default? #294
Comments
Hey, thanks for opening this. Definitely happy to improve this, but I'm unsure what the problem is.
They shouldn't be read by screen readers now. If there's a title, then hidden will be false and the label will be announced I believe, and if the title is falsy, then hidden will be true, which seems correct. Have I misunderstood? I'm a little confused by the migration issue too, why does setting false cause a migration warning? Isn't false a valid setting here? I don't actually use Vue any more, and I don't recall the migration process. |
Thanks for taking a look at this even though you are not using vue anymore!
This is the scenario I think we can have undesirable behavior. If hidden is false, then it will announce it unconditionally. If hidden is undefined, it will announce it if it is rendered. The behavior I am concerned about is announcing the title for icons that are not rendered. When migrating from vue 2 to vue 3, you can use a migration build that will issue warnings for behavior that has changed. The behavior that changed that is relevant here is this: https://v3-migration.vuejs.org/breaking-changes/attribute-coercion.html Previously, if you bound an attribute to a false value, the attribute would be removed. In vue 3, it literally sets it to false. Thus, we used to set hidden to undefined rather than false. |
I understand the difference
What I don't understand is why / how this would occur. I think your change probably makes sense anyway, but I'm still not following if its a real issue or just a theoretical one. Sorry for my brain slowness, I am not a clever man. |
I don't have a screen reader and am not an accessibility expert, so it's theoretical for me. But I have a theory of a practical scenario it might matter in. Suppose a page has several modals with icons. The modals are in the DOM but hidden while not active with something like
That makes two of us, so thanks for joining me in bumbling through this :) |
Ah, I see what you mean now, thank you. Yes, I think it would be wise to merge your PR. I should be able to get to it Friday evening / Saturday morning; I'll do a release of updated icons etc at the same time |
Fixes: #294 Co-Authored-By: skylerberg
Fixes: #294 Co-Authored-By: skylerberg Signed-off-by: Rob Cresswell <[email protected]>
Currently, this library sets the aria-hidden attribute based on whether or not title is falsy.
vue-material-design-icons/build.ts
Lines 15 to 16 in 4b79bae
In the change between Vue2 and Vue3, the default behavior for
v-bind
attributes with the valuefalse
changed. Specifically:This leads to a subtle change in behavior in this package. Instead of omitting
aria-hidden
when title is unset, it will includearia-hidden="false"
. According to MDN, here are the different settings:I'm no expert in accessibility, but it seems like the default behavior would be more desirable. That way non-rendered icons won't be read by screen readers.
I'll put up a PR in case you also think we should change this.
P.S., I found this because it was causing a lot of noise as warnings on a migration build of Vue, so changing this will improve the migration experience a bit.
The text was updated successfully, but these errors were encountered: