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

A11y: Set alt="" on course badges and some other illustrative images #3993

Closed
2 tasks
Asartea opened this issue Jul 17, 2023 · 11 comments · Fixed by #4031
Closed
2 tasks

A11y: Set alt="" on course badges and some other illustrative images #3993

Asartea opened this issue Jul 17, 2023 · 11 comments · Fixed by #4031
Assignees
Labels
Status: In Progress This issue/PR has ongoing work being done

Comments

@Asartea
Copy link
Contributor

Asartea commented Jul 17, 2023

Complete the following REQUIRED checkboxes:

  • I have thoroughly read and understand The Odin Project Contributing Guide
  • The title of this issue follows the brief description of request format, e.g. Add dark mode to website

The following checkbox is OPTIONAL:

  • I would like to be assigned this issue to work on it

1. Description of the Feature Request:
Currently all of the course badges have an alt text of course name badge, but this does sounds weird when read by a screenreader and also serves no real purpose, as this info is always also conveyed by an accompanying header. These images instead should have their alt text set to none (alt="") to prevent screenreaders from reading them.

There are also several other images for which this is an issue:

  • Succes story avatars
  • How it works images
  • What you can expect at The Odin Project images on /about
  • Why you should get involved images on /contributing
  • probably a few more that I have missed

2. Acceptance Criteria:

Tasks

@Asartea Asartea added the Status: Needs Review This issue/PR needs an initial or additional review label Jul 17, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog / Ideas in Main Site Jul 17, 2023
@luuu-xu
Copy link
Contributor

luuu-xu commented Jul 19, 2023

Hi @Asartea I would like to work on this a11y image alt issue! I will focus on image alt only and scan through our pages and make changes.

@thatblindgeye
Copy link
Contributor

thatblindgeye commented Jul 21, 2023

@Asartea For the course badges, you're talking about how, for example, in the following screenshot:

Intermediate HTML and CSS course badge and header

The badge has an img element with alt text of "Intermediate HTML and CSS badge"?

If so, then we don't want to make the alt text an empty string. Since the badge is inside a link, we need the image to have an alt text both to provide an accessible name for the link, and to display visible text should the image not load. What we should do is remove the "badge" text from the alt text, though. So "Intermediate HTML and CSS" instead of "Intermediate HTML and CSS badge".

Whether we really need these badges to also be links is another question. I would also think that we could convey the progress displayed in a badge as well (which again is another matter).

For the other spots you mentioned, yeah we should update the alt text for those to be an empty string.

@thatblindgeye
Copy link
Contributor

@luuu-xu it looks like @Asartea has asked to be assigned the issue as part of creating the issue. Let's see if they would need to split the work up at all, otherwise they'll be assigned to work on it.

@Asartea
Copy link
Contributor Author

Asartea commented Jul 21, 2023

@thatblindgeye feel free to assign it to them; always happy to see someone else working on a11y stuff

@thatblindgeye
Copy link
Contributor

@luuu-xu you're assigned!

@thatblindgeye thatblindgeye moved this from 📋 Backlog / Ideas to 🏗 In progress in Main Site Jul 21, 2023
@KevinMulhern KevinMulhern added Status: In Progress This issue/PR has ongoing work being done and removed Status: Needs Review This issue/PR needs an initial or additional review labels Jul 22, 2023
@luuu-xu
Copy link
Contributor

luuu-xu commented Jul 22, 2023

@thatblindgeye Hi, I wanted to get an opinion from you.

Since the badge is inside a link, we need the image to have an alt text both to provide an accessible name for the link, and to display visible text should the image not load. What we should do is remove the "badge" text from the alt text, though. So "Intermediate HTML and CSS" instead of "Intermediate HTML and CSS badge".

I understand and agree on this change and have made these changes sitewide. But when I am checking the course badges on our index page, where there are 9 courses in a flexbox under "Learn everything you need to know".

The screenreader will read these links to be "link, heading level 3, ruby, ruby" because now the badge img has alt="ruby" and the h3 has a textcontent of "ruby" too. Should I change these badge images' alt to be empty alt="" ? Because I feel like if the images are not loaded, the information is presented by the h3 so now the alt is kind of redundant.

@thatblindgeye
Copy link
Contributor

@luuu-xu excellent catch! I'd agree that for those particular badges alt="" makes sense. Awesome flex of those a11y muscles 💪🏼

@luuu-xu
Copy link
Contributor

luuu-xu commented Jul 24, 2023

@thatblindgeye Thank you! I think I've got everything and I am creating the PR now. I am still new to improving a11y so I would love to learn and work with you on more of these issues too!

@luuu-xu
Copy link
Contributor

luuu-xu commented Jul 26, 2023

@thatblindgeye @Asartea Hi, I am not sure who I should ask for a review for the PR I submitted. Let me know! Thank you!

@Asartea
Copy link
Contributor Author

Asartea commented Jul 26, 2023

I'm not a maintainer, and so I can't give a review as GitHub means it, but I will note that you seem to have some linter errors you'll need to fix before it gets merged

@ManonLef
Copy link
Member

I have requested a review on the PR from a maintainer

@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Main Site Jul 29, 2023
KevinMulhern added a commit that referenced this issue Jul 29, 2023
<!-- Thank you for taking the time to contribute to The Odin Project. In
order to get this pull request (PR) merged in a reasonable amount of
time, you must complete this entire template. -->

## Because
<!-- Summarize the purpose or reasons for this PR, e.g. what problem it
solves or what benefit it provides. -->

Some images like course badges, illustrative images, and user avatars
have `alt` not optimized for a11z.

## This PR
<!-- A bullet point list of one or more items describing the specific
changes. -->

1. For **Course Badge** images, their `alt="Ruby"` is changed to its
course's name since these images are inside of links.
2. Except for the **flexbox of course badges** on landing page, their
`alt=""` is set to empty since their siblings are headers with course
names and their parents are links. Setting `alt` to course names would
result in screen readers reading duplicate names.
3. For some **illustrative images**, their `alt=""` is set to empty
since they are purely decorative and images are not conveying any
meaningful messages because they have texts beside them.
4. **Users' avatars** under success stories now have `alt=""` because
again they are accompanied by users' names as links and the images are
just photographs of these users.

## Issue
<!--
If this PR closes an open issue in this repo, replace the XXXXX below
with the issue number, e.g. Closes #2013.

If this PR closes an open issue in another TOP repo, replace the #XXXXX
with the URL of the issue, e.g. Closes
https://github.com/TheOdinProject/curriculum/issues/XXXXX

If this PR does not close, but is related to another issue or PR, you
can link it as above without the 'Closes' keyword, e.g. 'Related to
#2013'.
-->
Closes #3993 

## Additional Information
<!-- Any other information about this PR, such as a link to a Discord
discussion. -->


## Pull Request Requirements
<!-- Replace the whitespace between the square brackets with an 'x',
e.g. [x]. After you create the PR, they will become checkboxes that you
can click on. -->
- [x] I have thoroughly read and understand [The Odin Project
Contributing
Guide](https://github.com/TheOdinProject/theodinproject/blob/main/CONTRIBUTING.md)
- [x] The title of this PR follows the `keyword: brief description of
change` format, using one of the following keywords:
  - `Feature` - adds new or amends existing user-facing behavior
- `Chore` - changes that have no user-facing value, refactors,
dependency bumps, etc
  - `Fix` - bug fixes
-   [x] The `Because` section summarizes the reason for this PR
- [x] The `This PR` section has a bullet point list describing the
changes in this PR
- [x] I have verified all tests and linters pass after making these
changes.
- [x] If this PR addresses an open issue, it is linked in the `Issue`
section
-   [ ] If applicable, this PR includes new or updated automated tests

---------

Co-authored-by: Kevin <[email protected]>
Mclilzee pushed a commit to Mclilzee/theodinproject that referenced this issue Aug 2, 2023
<!-- Thank you for taking the time to contribute to The Odin Project. In
order to get this pull request (PR) merged in a reasonable amount of
time, you must complete this entire template. -->

## Because
<!-- Summarize the purpose or reasons for this PR, e.g. what problem it
solves or what benefit it provides. -->

Some images like course badges, illustrative images, and user avatars
have `alt` not optimized for a11z.

## This PR
<!-- A bullet point list of one or more items describing the specific
changes. -->

1. For **Course Badge** images, their `alt="Ruby"` is changed to its
course's name since these images are inside of links.
2. Except for the **flexbox of course badges** on landing page, their
`alt=""` is set to empty since their siblings are headers with course
names and their parents are links. Setting `alt` to course names would
result in screen readers reading duplicate names.
3. For some **illustrative images**, their `alt=""` is set to empty
since they are purely decorative and images are not conveying any
meaningful messages because they have texts beside them.
4. **Users' avatars** under success stories now have `alt=""` because
again they are accompanied by users' names as links and the images are
just photographs of these users.

## Issue
<!--
If this PR closes an open issue in this repo, replace the XXXXX below
with the issue number, e.g. Closes TheOdinProject#2013.

If this PR closes an open issue in another TOP repo, replace the #XXXXX
with the URL of the issue, e.g. Closes
https://github.com/TheOdinProject/curriculum/issues/XXXXX

If this PR does not close, but is related to another issue or PR, you
can link it as above without the 'Closes' keyword, e.g. 'Related to
TheOdinProject#2013'.
-->
Closes TheOdinProject#3993 

## Additional Information
<!-- Any other information about this PR, such as a link to a Discord
discussion. -->


## Pull Request Requirements
<!-- Replace the whitespace between the square brackets with an 'x',
e.g. [x]. After you create the PR, they will become checkboxes that you
can click on. -->
- [x] I have thoroughly read and understand [The Odin Project
Contributing
Guide](https://github.com/TheOdinProject/theodinproject/blob/main/CONTRIBUTING.md)
- [x] The title of this PR follows the `keyword: brief description of
change` format, using one of the following keywords:
  - `Feature` - adds new or amends existing user-facing behavior
- `Chore` - changes that have no user-facing value, refactors,
dependency bumps, etc
  - `Fix` - bug fixes
-   [x] The `Because` section summarizes the reason for this PR
- [x] The `This PR` section has a bullet point list describing the
changes in this PR
- [x] I have verified all tests and linters pass after making these
changes.
- [x] If this PR addresses an open issue, it is linked in the `Issue`
section
-   [ ] If applicable, this PR includes new or updated automated tests

---------

Co-authored-by: Kevin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: In Progress This issue/PR has ongoing work being done
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants