Closed skylerberg closed 10 months ago
Hey, thanks for opening this. Definitely happy to improve this, but I'm unsure what the problem is.
That way non-rendered icons won't be read by screen readers
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!
If there's a title, then hidden will be false and the label will be announced
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.
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
The behavior I am concerned about is announcing the title for icons that are not rendered.
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.
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.
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 display: none
in CSS. My theory is that by setting aria-hidden="false"
we would cause a screen reader to announce the icons in the modals even though they are not rendered.
Sorry for my brain slowness, I am not a clever man.
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
Currently, this library sets the aria-hidden attribute based on whether or not title is falsy.
https://github.com/robcresswell/vue-material-design-icons/blob/4b79bae4808a2f5a34d9784299836211f4374720/build.ts#L15-L16
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.