Skip to content
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

Allow empty alt text on the Image component #623

Open
blittle opened this issue Sep 8, 2022 · 7 comments
Open

Allow empty alt text on the Image component #623

blittle opened this issue Sep 8, 2022 · 7 comments
Labels

Comments

@blittle
Copy link
Contributor

blittle commented Sep 8, 2022

Describe the bug
The Image component currently throws warnings when there is no alt text. Sometimes, for presentational images, there should not be alt text. Should an explicit empty string passed as a prop avoid the warning? cc @WillsonSmith

@blittle blittle added the Bug Something isn't working label Sep 8, 2022
@frehner
Copy link
Contributor

frehner commented Sep 8, 2022

Sounds reasonable to me

@davidhousedev
Copy link
Contributor

For presentational images, it's important to set role="presentation" so the image is removed from the accessibility tree. Should some guidance around how to use the attribute be included in the warning? Maybe an error could still be thrown (in development?) when alt text is empty and role !== 'presentation'?

@frehner
Copy link
Contributor

frehner commented Sep 12, 2022

That's a great suggestion, thanks!

@dduupp
Copy link

dduupp commented Sep 14, 2022

Having both alt="" and role="presentation" attributes seems overkill. Images with empty alt attributes are already ignored by screen readers. Adding a presentation role would excluded the image from the accessibility tree altogether which could be an issue for anything that's not a screen reader but still uses the accessibility tree API.

IMO the best solution is to default alt to an empty string, and to remove the warning altogether.

@davidhousedev
Copy link
Contributor

Images with empty alt attributes are already ignored by screen readers.

Based on my experience, MacOS's screen reader will still read out the presence of an image with empty alt text, unless you set another attribute to remove it from the accessibility tree (e.g. visibility: hidden or role="presentation"). That said, I don't have any experience with NVDA. Could you elaborate @dduupp ?

@dduupp
Copy link

dduupp commented Sep 14, 2022

This video demonstrates how an image with empty alt attribute gets skipped by VoiceOver. Are you sure it's different for your version? @davidhousedev

See this StackOverflow thread.

To answer your last point, the biggest reason to not include all three is that it is complete overkill. With an empty alt attribute, the element will already be ignored by screen readers. One potential downside with including role=presentation and aria-hidden is that both of these will remove the element from the accessibility API, which could have negative effects on users that use the API with tools that are not screen readers.

Also found this W3C page on decorative images.

Screen readers also allow the use of WAI-ARIA to hide elements by using role="presentation". However, currently, this feature is not as widely supported as using a null alt attribute.

I guess I'd rather not be forced to use role="presentation" (which may not even do anything) everywhere. My guess is like 90% of all imagery tends to be decorative anyways. May I suggest we keep things DRY and just stick with the widely supported empty alt attribute?

@davidhousedev
Copy link
Contributor

Cool! I didn't realize that VO behaved differently on an image without alt set, versus an image that had alt="". Thanks @dduupp

As someone who has worked pretty hard to bring a11y into my organization, this is now super clear to me. However, I'd be concerned about alt="" being the expected default because it'd be very easy for folks to hide images that should otherwise be given an accessible name.

Respectfully, I don't share your concern about needing to set either role="presentation" or aria-hidden="true". We make heavy use of images on our sites, but I can only think of a handful of cases where the image is strictly presentational and communicates no information about the page.

All that said, I defer to the core team here, we'd be happy to adopt whatever convention is supported by the framework 👍🏻

@elisenyang elisenyang transferred this issue from Shopify/hydrogen-v1 Oct 25, 2022
@frehner frehner transferred this issue from Shopify/hydrogen-react Mar 3, 2023
@blittle blittle added the SEV-3 label Jul 18, 2024
@michenly michenly added SEV-4 and removed SEV-3 labels Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants