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

Image: Fix inclusion of alttext in lightbox button aria-label #55000

Closed
wants to merge 1 commit into from

Conversation

artemiomorales
Copy link
Contributor

What?

The logic to include alttext in the lightbox button's aria-label was not working properly, and screen readers were not able process that information as a result. This fixes that.

Why?

We want users who use screen readers to have a good experience when navigating pages that contain the lightbox.

How?

It adds back a line of logic that had been accidentally removed that allows us to create the aria-label with the alttext.

Testing Instructions

  1. Add an image to a post
  2. Add alt text to the image
  3. In the image block's settings, make sure "Expand on Click" is enabled
  4. Publish and view the post using a screen reader
  5. Tab to the image and make sure the alt text is announced by the screen reader

Testing Instructions for Keyboard

N/A (specified above)

@artemiomorales artemiomorales added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Block] Image Affects the Image Block [Feature] Interactivity API API to add frontend interactivity to blocks. labels Oct 3, 2023
@afercia
Copy link
Contributor

afercia commented Oct 3, 2023

Thanks for working on this.
Noting that I previously created #54971 and was working on this fix and more fixes. Pull requeste without related issues are somewhat difficult to find for other contributors in a large open source project. A little more coordination would be apprciated.

That said, I think the same fix should be applied also to the file in the block-supports?

@@ -124,6 +124,7 @@ function block_core_image_render_lightbox( $block_content, $block ) {

$aria_label = __( 'Enlarge image' );

$processor->next_tag( 'img' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At line 138 the $processor instance is used again but now it points to the img. To my understanding, a new instance should be created before using get_updated_html()?

@artemiomorales
Copy link
Contributor Author

artemiomorales commented Oct 3, 2023

@afercia Perfect, thanks for taking a look at this and capturing the other issues in #54971 🙏

Since it looks like you've already begun addressing this issue in depth in #55010, I'll go ahead and close this PR, though I can open it back up and continue working on it if that makes sense.

@artemiomorales
Copy link
Contributor Author

Pull requeste without related issues are somewhat difficult to find for other contributors in a large open source project. A little more coordination would be apprciated.

@afercia Pardon, got it, will be sure to create related issues for PRs going forward to make sure we coordinate more effectively 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block [Feature] Interactivity API API to add frontend interactivity to blocks. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants