Skip to content

Commit

Permalink
♿️(course glimpse) put the course icon after the heading
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
manuhabitela committed Mar 24, 2022
1 parent 32006c4 commit b02600a
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 24 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -57,6 +57,8 @@ Versioning](https://semver.org/spec/v2.0.0.html).
except if its placeholder is empty.
- Fix missing styles for Organization plugin 'row' variant link wrapper
- Fix course run deletion when translation title is empty
- Reordered course glimpse text order in the DOM for better screen reader
support.

## [2.13.0] - 2022-02-18

Expand Down
11 changes: 10 additions & 1 deletion src/frontend/js/components/CourseGlimpse/index.spec.tsx
Expand Up @@ -17,7 +17,13 @@ describe('components/CourseGlimpse', () => {
},
duration: '3 months',
effort: '3 hours',
icon: null,
icon: {
sizes: '60px',
src: '/thumbs/icon_small.png',
srcset: 'some srcset',
title: 'Some icon',
color: 'red',
},
id: '742',
organization_highlighted: 'Some Organization',
organization_highlighted_cover_image: {
Expand Down Expand Up @@ -51,6 +57,9 @@ describe('components/CourseGlimpse', () => {
</IntlProvider>,
);

// first text we encounter should be the title, so that screen reader users get it first
expect(container.textContent?.indexOf('Course 42')).toBe(0);

// The link that wraps the course glimpse should have no title as its content is explicit enough
expect(screen.getByRole('link')).not.toHaveAttribute('title');
// The course glimpse shows the relevant information
Expand Down
30 changes: 15 additions & 15 deletions src/frontend/js/components/CourseGlimpse/index.tsx
Expand Up @@ -48,21 +48,6 @@ const CourseGlimpseBase = ({ context, course }: CourseGlimpseProps & CommonDataP
)}
</div>
<div className="course-glimpse__content">
{course.icon ? (
<div className="course-glimpse__icon">
<span className="category-badge">
{/* alt forced to empty string because it's a decorative image */}
<img
alt=""
className="category-badge__icon"
sizes={course.icon.sizes}
src={course.icon.src}
srcSet={course.icon.srcset}
/>
<span className="category-badge__title">{course.icon.title}</span>
</span>
</div>
) : null}
<div className="course-glimpse__wrapper">
<h3 className="course-glimpse__title">{course.title}</h3>
{course.organization_highlighted_cover_image ? (
Expand All @@ -85,6 +70,21 @@ const CourseGlimpseBase = ({ context, course }: CourseGlimpseProps & CommonDataP
<span>{course.code || '-'}</span>
</div>
</div>
{course.icon ? (
<div className="course-glimpse__icon">
<span className="category-badge">
{/* alt forced to empty string because it's a decorative image */}
<img
alt=""
className="category-badge__icon"
sizes={course.icon.sizes}
src={course.icon.src}
srcSet={course.icon.srcset}
/>
<span className="category-badge__title">{course.icon.title}</span>
</span>
</div>
) : null}
<CourseGlimpseFooter context={context} course={course} />
</div>
</a>
Expand Down
Expand Up @@ -21,14 +21,6 @@
{% endblockplugin %}
</div>
<div class="course-{{ course_variant }}__content">
{% get_placeholder_plugins "course_icons" course_page as icon_plugins %}
{% if icon_plugins %}
<div class="course-{{ course_variant }}__icon">
{% with category_variant="badge" has_link=False %}
{% render_plugin icon_plugins.0 %}
{% endwith %}
</div>
{% endif %}
<div class="course-{{ course_variant }}__wrapper">
<h{{ header_level|default:3 }} class="course-{{ course_variant }}__title">{{ course_page.get_title }}</h{{ header_level|default:3 }}>
{% if main_organization %}
Expand Down Expand Up @@ -69,6 +61,15 @@
</div>
</div>

{% get_placeholder_plugins "course_icons" course_page as icon_plugins %}
{% if icon_plugins %}
<div class="course-{{ course_variant }}__icon">
{% with category_variant="badge" has_link=False %}
{% render_plugin icon_plugins.0 %}
{% endwith %}
</div>
{% endif %}

{% block course_glimpse_footer %}
<div class="course-{{ course_variant }}-footer">
<div class="course-{{ course_variant }}-footer__date">
Expand Down

0 comments on commit b02600a

Please sign in to comment.