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

Experiment: Improvements for the Image block lightbox #50959

Closed
afercia opened this issue May 25, 2023 · 0 comments · Fixed by #50962
Closed

Experiment: Improvements for the Image block lightbox #50959

afercia opened this issue May 25, 2023 · 0 comments · Fixed by #50962
Assignees
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

Comments

@afercia
Copy link
Contributor

afercia commented May 25, 2023

Description

Follow-up to #50373

While testing the Image block lightbox, noticed a few things that can be improved. Main issues:

  • Some strings are not translatable. This was already mentioned in the original PR>
  • Translatable strings must avoid concatenation. Use full sentences instead.
  • When the image has an empty alt text, the modal dialog is unlabelled (because aria-labelledby points to the image with the empty alt text).
  • Missing escaping for some HTML attributes.
  • Potentially duplicate IDs when hte same image is added more than once on a page. This would also cause incorrect labelling of the modal dialog.
  • The buttons miss type="button". I'm not sure the buttons default action is prevented. There is a potential risk to submit a form in the edge case where the lightbox button is rendered within a form.

Step-by-step reproduction instructions

  • Check the code at https://github.com/WordPress/gutenberg/blob/2b687d03ecdfb52229c25d5d455038b14fc8fa89/packages/block-library/src/image/index.php and see the button labels are not translatable:
    • $aria_label = 'Open image lightbox';
    • aria-label="Close lightbox"
  • Observe one translatable string uses concatenation.
  • Make sure to enable Core blocks in the Gutenberg Experiments settings page.
  • Add an image with no alt text.
  • Set the image to open in a lightbox (from the Advanced settings in the block settings sidebar).
  • Publisha and go to the fron end.
  • Open the image lightbox by clicking the image.
  • Inspect the source and check the element with role=dialog has an aria-labelledby attribute that points to the ID of the image with the empty alt text. As such, the dialog is unlabelled.
  • Inspect the source and observe the buttosn to open and close the lightbox miss a type="button" attribute.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Bug An existing feature does not function as intended [Block] Image Affects the Image Block [Feature] Interactivity API API to add frontend interactivity to blocks. labels May 25, 2023
@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label May 25, 2023
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label May 25, 2023
@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label Jul 24, 2023
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 a pull request may close this issue.

2 participants