-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
♿ Adds Alt & aria-label to Placeholder Images #14391
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nit on a duplicate line. Nice job!
placeholder.setAttribute('layout', 'fill'); | ||
placeholder.setAttribute('placeholder', ''); | ||
placeholder.setAttribute('referrerpolicy', 'origin'); | ||
placeholder.setAttribute('alt', 'loading video'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is duplicated below under the else
block. You can remove it.
well spotted - have removed that line |
@gulliverhan could you merge master onto this branch to incorporate the most recent test fixes? This will make CI more reliable and likely to pass. Sorry about that. |
Thank you! |
* adds alt & aria label to brid-player & tests * adds alt & aria-label to amp-jwplayer & tests to cover * adds alt & aria-label to amp-kaltura-player & tests * adds alt & aria-label to placeholder img for amp-springboard-player * adds alt & aria-label attributes to img placeholder for amp-youtube * fixes lint errors * removes duplicated line
addresses Issue #5508
Adds alt tag & propagates aria label to the placeholder images for
amp-brid-player
amp-jwplayer
amp-kaltura-player
amp-springboard-player
amp-youtube