Add alt tags to images for a11y #529

amyrlam · 2018-04-06T06:22:17Z

Motivation

Resolve part of Pa11y fixes to Docusuarus #462
Improve a11y of Docusaurus

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

Ran pa11y http://localhost:3000/ and confirmed no results for Error: Img element missing an alt attribute. Use the alt attribute to specify a short text alternative from the original gist.

Related PRs

N/A

Details

Add imageAlt to GridBlock
Set alt equal to user.caption for mapped users
Set alt equal to post.author for blog posts
Update documentation

amyrlam · 2018-04-06T06:22:48Z

👋@yangshun let me know if I should make any edits!

JoelMarcey

Thanks for doing this!

Also, do any CSS updates in main.css need to be done for this? In the gridBlock.blockImage section, for example. Maybe not, but just making sure.

JoelMarcey · 2018-04-06T23:30:18Z

docs/api-pages.md

@@ -64,6 +64,7 @@ The `contents` attribute is an array containing the contents of each section of
 - `imageAlign` field for image alignment relative to the text, which defaults to `top` and can be set to `bottom`, `left`, or `right`
 - `title` for the title to display for this section, which is parsed from markdown
 - `imageLink` for a link destination from clicking the image
+- `imageAlt` provides an alt tag for an image for a11y


maybe

imageAlt for the description of what text will be shown in case the image is not available

instead?

JoelMarcey · 2018-04-06T23:32:28Z

lib/core/GridBlock.js

    if (image) {
      if (imageLink) {
        return (
          <div className="blockImage">
            <a href={imageLink}>
-              <img src={image} />
+              <img src={image} alt={imageAlt} />


maybe we do a check here in case imageAlt isn't provided?

like

<img src={image} alt={imageAlt ? imageAlt : ''}

or something like that?

I think it's fine as-is because React does not render any props that are undefined or null.

👍 Yeah I didn't add the check, because without an imageAlt, the alt didn't show up on inspection in the browser. Thanks @yangshun for explaining why!

JoelMarcey · 2018-04-06T23:32:35Z

lib/core/GridBlock.js

            </a>
          </div>
        );
      } else {
        return (
          <div className="blockImage">
-            <img src={image} />
+            <img src={image} alt={imageAlt} />


From #529 (comment), leave as-is.

yangshun

Great work @amyrlam ! LGTM once the comments are responded to. Will leave it to @JoelMarcey to give the final approval and merging.

yangshun · 2018-04-07T03:26:01Z

lib/core/GridBlock.js

    if (image) {
      if (imageLink) {
        return (
          <div className="blockImage">
            <a href={imageLink}>
-              <img src={image} />
+              <img src={image} alt={imageAlt} />


I think it's fine as-is because React does not render any props that are undefined or null.

yangshun · 2018-04-07T03:26:42Z

website/blog/2017-12-14-introducing-docusaurus.md

@@ -5,7 +5,7 @@ authorURL: http://twitter.com/JoelMarcey
 authorFBID: 611217057
 ---

-![](/img/slash-introducing.png)
+![introducing Slash](/img/slash-introducing.png)


Nit - Title case - "Introducing Slash"

Fixed. Sorry, had done that intentionally thinking it was the alt standard. I changed some other ones too to Title Case.

yangshun · 2018-04-07T03:26:57Z

website/blog/2017-12-14-introducing-docusaurus.md

@@ -24,7 +24,7 @@ Docusaurus also provides core website and documentation features out-of-the-box

 ## The Birth of docusaurus

-![](/img/slash-birth.png)
+![birth of Slash](/img/slash-birth.png)


Nit - Title case

yangshun · 2018-04-07T03:27:11Z

website/pages/en/about-slash.js

          <p>
            Slash is the official mascot of Docusaurus. You will find different variations of her throughout the <a href="https://docusaurus.io">website</a>, whether she is moving fast on her scooter or writing documentation at her standing desk. At Facebook, we have actual Slash plushies -- and you never know, you may see these plushies at various events and conferences in the future.
          </p>
        </Container>
        <Container className="mainContainer">
          <h2>Birth of Slash</h2>
-          <img src={`${siteConfig.baseUrl}img/slash-birth.png`} />
+          <img src={`${siteConfig.baseUrl}img/slash-birth.png`} alt="birth of Slash"/>


Nit - Title case

- Add imageAlt to GridBlock - Set alt equal to `user.caption` for mapped users - Set alt equal to `post.author` for blog posts - Update documentation

amyrlam · 2018-04-07T21:09:37Z

Made edits and squashed them in, wondering if you prefer squashing or a separate commit?

I don't think any CSS changes are needed for the alt tag, since it's for if the images cannot be displayed.

Let me know if other edits needed and thanks for the feedback @JoelMarcey @yangshun!

yangshun · 2018-04-07T21:32:37Z

Both ways are fine Feel free to leave it in non-squashed mode. The project maintainers have a squash option on GitHub when they merge the PR. So it's not an issue if your PR branch has many commits (:

…

On Sat, Apr 7, 2018, 2:09 PM Amy Lam ***@***.***> wrote: Made edits and squashed them in, wondering if you prefer squashing or a separate commit? I don't think any CSS changes are needed for the alt tag, since it's for if the images cannot be displayed. Let me know if other edits needed and thanks for the feedback @JoelMarcey <https://github.com/JoelMarcey> @yangshun <https://github.com/yangshun>! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#529 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQRHQWZX52KeFuwq6WId6Ep9N7czTI0ks5tmSsUgaJpZM4TJjjv> .

amyrlam · 2018-04-08T00:51:48Z

Thanks - good to know about the squash option for next time!

JoelMarcey

Let's ship it!

facebook-github-bot added the CLA Signed Signed Facebook CLA label Apr 6, 2018

amyrlam force-pushed the amyrlam/a11y-alt-tag branch from 3f03bb5 to be360a1 Compare April 6, 2018 06:25

JoelMarcey reviewed Apr 6, 2018

View reviewed changes

yangshun reviewed Apr 7, 2018

View reviewed changes

amyrlam force-pushed the amyrlam/a11y-alt-tag branch 2 times, most recently from d437fd9 to 038e6fa Compare April 7, 2018 21:00

Add alt tags to images for a11y

e0c2991

- Add imageAlt to GridBlock - Set alt equal to `user.caption` for mapped users - Set alt equal to `post.author` for blog posts - Update documentation

amyrlam force-pushed the amyrlam/a11y-alt-tag branch from 038e6fa to e0c2991 Compare April 7, 2018 21:02

JoelMarcey approved these changes Apr 8, 2018

View reviewed changes

JoelMarcey merged commit c2cd169 into facebook:master Apr 8, 2018

yangshun mentioned this pull request Apr 11, 2018

Pa11y fixes to Docusuarus #462

Closed

amyrlam deleted the amyrlam/a11y-alt-tag branch May 11, 2018 20:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add alt tags to images for a11y #529

Add alt tags to images for a11y #529

amyrlam commented Apr 6, 2018

amyrlam commented Apr 6, 2018

JoelMarcey left a comment

JoelMarcey Apr 6, 2018

JoelMarcey Apr 6, 2018

yangshun Apr 7, 2018

amyrlam Apr 7, 2018

JoelMarcey Apr 6, 2018

amyrlam Apr 7, 2018

yangshun left a comment •

edited

Loading

yangshun Apr 7, 2018

yangshun Apr 7, 2018

amyrlam Apr 7, 2018

yangshun Apr 7, 2018

amyrlam Apr 7, 2018

yangshun Apr 7, 2018

amyrlam Apr 7, 2018

amyrlam commented Apr 7, 2018

yangshun commented Apr 7, 2018 via email

amyrlam commented Apr 8, 2018

JoelMarcey left a comment

Add alt tags to images for a11y #529

Add alt tags to images for a11y #529

Conversation

amyrlam commented Apr 6, 2018

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

Related PRs

Details

amyrlam commented Apr 6, 2018

JoelMarcey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yangshun left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amyrlam commented Apr 7, 2018

yangshun commented Apr 7, 2018 via email

amyrlam commented Apr 8, 2018

JoelMarcey left a comment

Choose a reason for hiding this comment

yangshun left a comment •

edited

Loading