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

♿️(course glimpse) improve global accessibility of a course glimpse #1620

Merged
merged 4 commits into from Mar 24, 2022

Conversation

manuhabitela
Copy link
Collaborator

@manuhabitela manuhabitela commented Feb 25, 2022

Purpose

Make a course glimpse more understandable when using a screen reader.

Proposal

A few stuff can be done:

  • add alt text to meaningful icons
  • reorder the DOM
  • change the structure with the one <a> wrapping everything

Note: some improvements are already added via #1611 (most notably, the removal of redundant title attributes)

@manuhabitela manuhabitela self-assigned this Feb 25, 2022
@manuhabitela manuhabitela added the a11y Accessibility related tasks label Feb 25, 2022
@manuhabitela
Copy link
Collaborator Author

manuhabitela commented Feb 25, 2022

Creating this PR early to talk about one thing that would require some notable changes @jbpenrath @sampaccoud

The fact that each course glimpse is a big <a> wrapping everything is a bit of a problem.

With a screen reader, when navigating in the course glimpse, each thing is announced as a "link". It's not obvious at all that each thing is a link to the course itself, and not a link to the thing it describes.

For example, when hearing "link, organization, [organization name]", we expect to be redirected to the organization, not the course.

And with a mouse, one big anchor wrapping everything also breaks the selection of text, preventing copy/pasting stuff.

Proposals to fix this:

  • wrap everything with a classic <div>, then put the <a> only around the course name. But this would mean now only the title is clickable.
  • wrap everything with a classic <div>, then re-organize the internal wrappers inside, to have one <a> wrapping both the cover and the title. This would mean we can click most of the view, as usually intended for such blocks.

I wouldn't personally recommend trying to keep the exact same current behavior ("being able to click the whole block wherever to activate the link"), as it would require some dirty JS tricks to make it kind of accessible.

@jbpenrath
Copy link
Member

jbpenrath commented Feb 28, 2022

Creating this PR early to talk about one thing that would require some notable changes @jbpenrath @sampaccoud

The fact that each course glimpse is a big <a> wrapping everything is a bit of a problem.

With a screen reader, when navigating in the course glimpse, each thing is announced as a "link". It's not obvious at all that each thing is a link to the course itself, and not a link to the thing it describes.

Hmm.. I believed that screen reader cannot navigate within a course glimpse. From my side I used Voice Over to test it and I cannot navigate within the glimpse by using tab key. So I'm not sure to take your point.
Capture d’écran 2022-02-28 à 09 40 56

For example, when hearing "link, organization, [organization name]", we expect to be redirected to the organization, not the course.

And with a mouse, one big anchor wrapping everything also breaks the selection of text, preventing copy/pasting stuff.

Proposals to fix this:

  • wrap everything with a classic <div>, then put the <a> only around the course name. But this would mean now only the title is clickable.

The click area will be too much reduced, not a fan of this suggestion.

  • wrap everything with a classic <div>, then re-organize the internal wrappers inside, to have one <a> wrapping both the cover and the title. This would mean we can click most of the view, as usually intended for such blocks.

Pros: We could add link to Organization or Categories on glimpses
Cons: This change will modify the behavior of the course glimpse that will be something very confusing for our users.

I wouldn't personally recommend trying to keep the exact same current behavior ("being able to click the whole block wherever to activate the link"), as it would require some dirty JS tricks to make it kind of accessible.

Yes use custom JS here is not the right way.
However I wonder if we could not use some css to expand the click area of a link to the whole glimpse.
e.g : Wrap only the course title within a link but use the pseudo-element :before positioned absolutly.

@manuhabitela
Copy link
Collaborator Author

Hmm.. I believed that screen reader cannot navigate within a course glimpse. From my side I used Voice Over to test it and I cannot navigate within the glimpse by using tab key. So I'm not sure to take your point.

With a screen reader, the usual way of navigating in the content is more to use arrow keys than tab keys. Because the tab key, like without a screen reader, only makes focusing "focusable" things possible (links, buttons, inputs, etc.), contrary to the arrow keys that lets you focus any content. So you could check by using the VO key+arrows instead of tab.

However I wonder if we could not use some css to expand the click area of a link to the whole glimpse.
e.g : Wrap only the course title within a link but use the pseudo-element :before positioned absolutly.

Yes, I think we could have an invisible, absolutely positioned a on top of the whole glimpse, that is only clickable (not focusable). And have a a in the h3 for keyboard users. This would be OK for screen readers and we would keep current behavior for mouse users. Didn't talk about it though because this behavior keeps the problem of "not being able to select any text in the glimpse", which I think is kind of a bummer. It's not an RGAA rule to actually have any text selectable, but it's usually a good thing to do to keep it that way. Some people have way less trouble copy/pasting stuff than retyping it.

And this would prevent the bonus of adding links on organization/categories ;)

@manuhabitela
Copy link
Collaborator Author

My proposal to handle the big wrapping <a>:

  • replace the wrapping <a> containing everything by a <div>
  • have a <a> around the media. This one is unfocusable and hidden from screen readers. It's here only to have a big clickable zone for mouse users
  • have a <a> inside the title <h3>. This one is focusable normally.
  • when you hover or focus the media or title, we toggle a class on the wrapper to keep the behavior with the card shadow.
  • the title anchor has padding/margin trick to make it go outside it's container in order to have a bigger clickable zone.

The result is:

  • while the clickable zone obviously is smaller than before, it's still pretty decent
  • the organization/code/date text is selectable normally, and we'll be able to add a link on the organization if we want later
  • screen reader users have stuff announced in a clearer way
demo-course-glimpse.mp4

What do you think? If you think it's OK I'll reproduce this in the django templates.

@sveetch
Copy link
Collaborator

sveetch commented Mar 17, 2022

Just to notice than since Bootstrap 5.0 there is the stretched link feature which is somewhat to implement the "link wrapper around container" behavior without to use a link wrapper, but a simple link instead. However it may not work in some context and i don't know about its accessibility.

@manuhabitela manuhabitela force-pushed the a11y/course-glimpse branch 3 times, most recently from 885b117 to 1f525c4 Compare March 20, 2022 16:56
@manuhabitela manuhabitela marked this pull request as ready for review March 20, 2022 16:57
@manuhabitela manuhabitela force-pushed the a11y/course-glimpse branch 5 times, most recently from ba43269 to 9f703b9 Compare March 22, 2022 13:27
@manuhabitela
Copy link
Collaborator Author

This should be good for review with the changes made on TS and django template code @jbpenrath 👌

@manuhabitela manuhabitela force-pushed the a11y/course-glimpse branch 2 times, most recently from 10bec9b to 5f73e2e Compare March 24, 2022 09:30
@manuhabitela
Copy link
Collaborator Author

Hey @jbpenrath if you have time to review this today I'd appreciate it :)

Copy link
Member

@jbpenrath jbpenrath left a comment

Choose a reason for hiding this comment

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

Great works 👏

src/frontend/scss/objects/_course_glimpses.scss Outdated Show resolved Hide resolved
@manuhabitela manuhabitela force-pushed the a11y/course-glimpse branch 2 times, most recently from 560a5be to 8f0e855 Compare March 24, 2022 14:10
organization, code and date icons let users understand that what is
written next to them are organization name, course code and course date.

So, they convey meaning but they are hidden from screen readers.

Add the alt text so that screen reader users get the meaning from the
icons.
putting info related to the course glimpse before its heading is not
great for two reasons:

- with a screen reader, when navigate by headings on the page to jump
between each course glimpse, it makes users miss the info
- with a screen reader, when navigating in the content "normally" (with
down arrow), the first thing announced for a glimpse course is the icon
title, which is pretty misleading. We should hear the course name
instead.
It's a bit confusing when navigating via screen reader to only hear
"Sous-titres" or "Malentendants". Specify it's a category just before to
be more explicit.
@manuhabitela manuhabitela force-pushed the a11y/course-glimpse branch 2 times, most recently from 1c73510 to 3ea2a20 Compare March 24, 2022 14:43
this makes for a better UX for screen reader/keyboard users.

The idea is to have a big clickable zone for mouse users with the link
on the media cover + on the title, with the link on the title having a
bigger clickable space than shown.

We manually add/remove the card shadow when the link is hovered/focused

src/richie/apps/courses/templates/courses/cms/fragment_course_glimpse.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Accessibility related tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants