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

Cover Block: Allow alt text for images #30505

Closed
kjellr opened this issue Apr 5, 2021 · 14 comments · Fixed by #33226
Closed

Cover Block: Allow alt text for images #30505

kjellr opened this issue Apr 5, 2021 · 14 comments · Fixed by #33226
Assignees
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement.

Comments

@kjellr
Copy link
Contributor

kjellr commented Apr 5, 2021

Cover block markup includes an attribute for alt text, but there's no place to edit this in the block UI. If you attempt to edit this via the block markup, the editor throws an invalid content error.

Cover images are often decorative (and thus don't technically need alt text), but we can't assume that'll be the case all the time. Gutenberg should allow users to enter alt text here when it's needed.

Screenshots:

Example with default Markup:

<!-- wp:cover {"url":"https://cldup.com/y7iKtyPFbw.jpg","id":2651} -->
<div class="wp-block-cover has-background-dim"><img class="wp-block-cover__image-background wp-image-2651" alt="" src="https://cldup.com/y7iKtyPFbw.jpg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size"></p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->

Screen Shot 2021-04-05 at 10 24 13 AM

Example markup with alt text:

<!-- wp:cover {"url":"https://cldup.com/y7iKtyPFbw.jpg","id":2651} -->
<div class="wp-block-cover has-background-dim"><img class="wp-block-cover__image-background wp-image-2651" alt="Photo of mountains" src="https://cldup.com/y7iKtyPFbw.jpg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size"></p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->

Screen Shot 2021-04-05 at 10 24 39 AM

@kjellr kjellr added [Type] Enhancement A suggestion for improvement. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Block] Cover Affects the Cover Block - used to display content laid over a background image labels Apr 5, 2021
@anarieldesign
Copy link

I'm just finishing our new WordPress theme based on block patterns and I noticed the same problem today. I added alt text to the cover block to pass the theme check and it breaks the pattern, it throws an invalid content error. Theme check shows empty alt so it would be useful if you can find the solution. Thanks

@torbenkorb
Copy link

Same problem here: with the new embedded image in the cover block (which is generally a good move) the alt text is empty.

In the JavaScript files packages/block-library/src/cover/edit.js on line 631 and in packages/block-library/src/cover/save.js on line 112 both it's set as empty string. Would be really great to have a field in block options to set or at least take over alt text if it is set in media library.

I've never contributed to WordPress before and am not sure how to extend fields or where in the scope the alt text can be found. Maybe I'll have a look into contributor guides the next days. Hopefully someone else is faster. Thanks.

@luisceladita
Copy link

It would be nice if the "alt" attribute could be added optionally to the cover block.

@cngodles
Copy link

To note, adding the attribute combination of role="presentation" and alt="" will suffice for accessibility testing. But I agree with the above, it would be nice to be able to add alternative text to the cover block.

Setting the Alternative Text attribute in the media library does not set this as well.

@eric-michel
Copy link

Definitely seems like a missing piece. And I agree that having role="presentation" and alt="" as the default makes sense. Right now it just outputs alt (with no value) which I believe is not sufficient for telling screen readers that the alt attribute is null

@cbirdsong
Copy link

cbirdsong commented May 27, 2021

It seems like the best implementation for this would be to use the alt text set on the image in the media library, but allow the cover block image to be set as decorative to remove it.

@jayseventwo
Copy link

I have come across this issue today as well - If I manually add the alt text in the code editor and then save, the alt text is removed. I have alt text applied to the image in the media library, but it is not pulling this into the cover image.

@rofsky
Copy link

rofsky commented Jun 22, 2021

I have come across this issue today as well - If I manually add the alt text in the code editor and then save, the alt text is removed. I have alt text applied to the image in the media library, but it is not pulling this into the cover image.

Same for me. Latest version. I tried all methods above and have the same results.

@sophiegyo
Copy link

I tested this too - it doesn't seem to pull the alt text from the media library at all. I actually hadn't noticed cover blocks now use <img> instead of background-image: url();, but isn't alt text a fairly basic requisite for accessibility?

It'd be fabulous if we could have alt text for covers blocks.

@kjellr
Copy link
Contributor Author

kjellr commented Jul 6, 2021

A quick fix for this issue would be to just ensure the block doesn't error when alt text is manually entered in the markup. A more complete fix would be to add a field to the "Media Settings" section to mirror the one present for image blocks:

Before After
Screen Shot 2021-07-06 at 10 41 55 AM Screen Shot 2021-07-06 at 10 41 55 AM

@Mamaduka Mamaduka self-assigned this Jul 6, 2021
@michaelbourne
Copy link

Seems like this only solved 1 side of the coin, alt text for non-decorative images. With the alt left empty however, the block still fails a11y audits.

I think that when the alt text field is left empty, the block should output role="presentation" and alt=""

Thoughts?

@cngodles
Copy link

cngodles commented Jun 2, 2022

I'm hesitant to suggest going straight to role="presentation" for an empty block. This doesn't solve accessibility but rather creates a blocker for automated accessibility testing. If users don't think it through, then it's failing to convey the text for a describable image, in which case you would want testing to catch it.

I think the best solution would be to add a checkbox for Presentation Only to enable the role attribute. This would disable the Alt text input box and include explainer text for the choice. If the user chooses to leave the field blank and the checkbox unchecked, then hopefully automated testing in the future would catch and report it to them.

@michaelbourne
Copy link

Fair point about that, some users may not understand the impact of leaving the alt empty without context. I'd be fine with a toggle.

@shadowshades
Copy link

I see that this issue is closed. But I find that if we use Fixed background, we cannot adjust alt text to an empty value or other:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.