Amphtml: [A11Y] Components with dynamic placeholders need to have a reasonable `alt` for the default placeholder.

Created on 11 Oct 2016  Â·  5Comments  Â·  Source: ampproject/amphtml

TLDR

Components that override createPlaceholderCallback() (and amp-youtube) need to set a reasonable alt for the image placeholder they create. For instance amp-jwplayer may add "Loading Video " + (aria-label || textContent of aria-labelledby) as the alt for the placeholder image.

Instructions

Implementation

  • Pick a component from the list below. The component code can be found in extensions/name-of-component/0.1/name-of-component.js.
  • Inside the the overridden createPlaceholderCallback() function, make sure that the placeholder image has a reasonable alt attribute for accessibility.
  • Manually test this via an example in /test/manual/ to ensure that the poster image does indeed have a reasonable alt attribute that can be read by a screenreader.
  • Write a unit test for your given component to guarantee this behavior.

Manual testing:

  • Run gulp in the repository root directory.
  • You can create manual tests under test/manual/name-of-component.amp.html. These tests already exists for some of the components below (e.g. amp-gfycat).
  • Navigate to localhost:8000/test/manual/name-of-component.amp.html to manually verify that your code behaves as intended.

Unit testing:

Unit tests can be found extensions/name-of-component/0.1/test/test-name-of-component.js. You can run the unit test for that file with the command gulp test --files=extensions/name-of-component/0.1/test/test-name-of-component.js. To run your test only, feel free to use it.only for testing, but remember to revert that back before you submit your PR. Once the PR merges, feel free to check off the component on this list below.

List of components that require this change:

  • [x] amp-apester-media
  • [x] amp-brid-player
  • [x] amp-gfycat
  • [x] amp-jwplayer
  • [x] amp-kaltura-player
  • [x] amp-playbuzz
  • [x] amp-springboard-player

Special case instructions for <amp-youtube>:
For <amp-youtube>, this logic needs to happen within the buildImagePlaceholder function.

Step by step

  • [ ] Claim a component in this issue by adding a comment below. Please only claim it if you plan on starting work in the next day or so. (If you join the AMP Project we'll be able to assign this issue to you after you've claimed it.)
  • [ ] If you aren't too familiar with Git/GitHub, see the Getting Started End-to-End Guide for an intro to Git & GitHub, and how to get a copy of the code. You can also refer to the Quick Start Guide for the necessary setup steps with less explanation than the End-to-End guide.
  • [ ] Follow the instructions for building AMP.
  • [ ] [Create a Git branch](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#create-a-git-branch) for making your changes.
  • [ ] [Sign the Contributor License Agreement](https://github.com/ampproject/amphtml/blob/master/CONTRIBUTING.md#contributor-license-agreement) before creating a Pull Request. (If you are contributing code on behalf of a corporation start this process as early as possible.)
  • [ ] [Commit your changes](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#edit-files-and-commit-them) frequently.
  • [ ] [Push your changes to GitHub](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#push-your-changes-to-your-github-fork).
  • [ ] [Create a Pull Request](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#send-a-pull-request-ie-request-a-code-review). Mention addresses Issue #5508 in the description.
  • [ ] [Respond to your reviewer's comments](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#respond-to-pull-request-comments) (if any).

Once approved, your changes will be merged. ⚡⚡⚡Congrats on making your first contribution to the AMP Project!⚡⚡⚡ You'll be able to see it live across the web soon!

Thanks, and we hope to see more contributions from you soon.

Questions?

If you have questions ask in this issue or on your Pull Request (mentioning @cathyxz) or see the How to get help section of the Getting Started guide.

When Possible Accessibility Bug good first issue

Most helpful comment

hi - have submitted a pr for what i think is a fix for amp-gfycat - if that's ok, happy to have the rest assigned to me

All 5 comments

/to @erwinmombay

hi - have submitted a pr for what i think is a fix for amp-gfycat - if that's ok, happy to have the rest assigned to me

@cathyxz I've taken a look at the remaining components and have just submitted a pr for them.
I haven't included amp-apester or amp-playbuzz as they don't create a placeholder image - they create a placeholder div with animated loading divs inside. I could carry the aria-label attribute over or set a aria-label with a loading message if you think it's appropriate?

I think the same strategy as what you describe above is fine, i.e. setting aria-label since alt does not apply to divs. The purpose is to allow screen readers to have something to read (they will read alt, aria-label, etc.). Basically so long as the screen reader will say "loading video ..." with whatever defaults they provide, it is fine. If they provide no aria-label attribute, "Loading Video" is technically fine too, since that at least tells what they are currently selecting on the screen is a loading video. =)

Closed by https://github.com/ampproject/amphtml/pull/14468.

Thank you very much!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

torch2424 picture torch2424  Â·  3Comments

radiovisual picture radiovisual  Â·  3Comments

Download picture Download  Â·  3Comments

mrjoro picture mrjoro  Â·  3Comments

sryze picture sryze  Â·  3Comments