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

Update the alt text description #14668

Merged
merged 3 commits into from
Mar 28, 2019
Merged

Update the alt text description #14668

merged 3 commits into from
Mar 28, 2019

Conversation

afercia
Copy link
Contributor

@afercia afercia commented Mar 27, 2019

Fixes #14326.

In core, the alt text description in the media views has been updated with the new text discussed and recommended by the accessibility team. See https://core.trac.wordpress.org/changeset/44900. See screenshots on https://core.trac.wordpress.org/ticket/41612#comment:39. Example:

Screenshot 2019-03-27 at 19 20 49

Note: I'm going to update the core string to include the word "Describe" within the link.

This PR updates the equivalent descriptions in Gutenberg for the Image and Media+Text blocks.

The new text links to the W3C "alt decision tree" tutorial and clarifies to leave the alt text empty "when the image is purely decorative". Previously, this part of the text was "if the image is not a key part of the content". Agreed the terms "key part" added cognitive load and needed to be clarified.

  • uses the ExternalLink component
  • worth noting the help prop accepts also components: given the current BaseControl implementation there's no reason why it shouldn't accept components and this seems correct to me
  • the help prop documentation in BaseControl and all the derived components needs to be updated
  • suggestions welcome, would - Type: String|WPComponent|null be appropriate?

Screenshot:

Screenshot 2019-03-27 at 19 06 33

@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Mar 27, 2019
help={ __( 'Alternative text describes your image to people who can’t see it. Add a short description with its key details.' ) }
help={
<Fragment>
<ExternalLink href={ __( 'https://www.w3.org/WAI/tutorials/images/decision-tree' ) }>
Copy link
Member

Choose a reason for hiding this comment

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

Is it intended this link value be localized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're planning to copy the W3C alt decision tree tutorial to the Handbook so it can be translated. But yes, can be non-translatable for now, will adjust.

@aduth
Copy link
Member

aduth commented Mar 27, 2019

  • suggestions welcome, would - Type: String|WPComponent|null be appropriate?

I expect it should be WPElement in place of WPComponent, as it's not the component definition itself, rather the result of createElement (related).

It may be reasonable to omit null, by the presence of "Required: No" in the documentation.

@afercia
Copy link
Contributor Author

afercia commented Mar 28, 2019

Last commit also removes a few trailing spaces (because my editor does that automatically).

@gziolo gziolo added this to the 5.4 (Gutenberg) milestone Mar 28, 2019
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Code looks good and it tests well.

@gziolo gziolo merged commit 1e40b28 into master Mar 28, 2019
@gziolo gziolo deleted the update/alt-text-description branch March 28, 2019 12:55
@gziolo gziolo added the [Block] Image Affects the Image Block label Mar 28, 2019
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 [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants