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

[pni] Product Page Accessibility Testing #4006

Closed
youriwims opened this issue Dec 3, 2019 · 6 comments
Closed

[pni] Product Page Accessibility Testing #4006

youriwims opened this issue Dec 3, 2019 · 6 comments
Assignees
Labels
a11y Accessibility buyer's guide 🛍 Issues related to the buyer's guide engineering epic
Milestone

Comments

@youriwims
Copy link
Contributor

youriwims commented Dec 3, 2019

Morphed to epic to track all the issues identified in comment #2

Related Issue: #3934

@youriwims youriwims added engineering buyer's guide 🛍 Issues related to the buyer's guide a11y Accessibility labels Dec 3, 2019
@alanmoo alanmoo added this to the Dec 20 milestone Dec 10, 2019
@Pomax Pomax modified the milestones: Dec 20, Mar 9 Feb 20, 2020
@kristinashu kristinashu modified the milestones: Mar 9, Icebox Feb 20, 2020
@Pomax Pomax modified the milestones: Icebox, May 18 May 13, 2020
@Pomax Pomax self-assigned this May 13, 2020
@Pomax
Copy link
Contributor

Pomax commented May 13, 2020

Issues identified

→ Cross-page issues

"Serious" issues

Join us content is invisible instead of display:none

This means tabbing to the main content will "get stuck" cycling through everything in the "join us" newsletter slide-out panel, without any indication that that's where focus is.

We can either make that slide-out be display:none with extra work to make sure the transitions work properly, or we can make it so that if there is focus anywhere inside the slide-out, the slide-out itself gets revealed.

PNI nav contrast

Text color is too low contrast (currently #999999, which has contrast 2.85, and needs to be #707070 or lower to pass)

image

"Moderate" issues

Missing <main> element

This element is relied on by accessibility tools to directly navigate to the start of the main content... we don't have one =)

→ Homepage

"Serious" issues

creep-o-meter hint contrast

Text color is too low contrast (currently #999999, which has contrast 2.85, and needs to be #707070 or lower to pass)

image

"Moderate" issues

<figcaption> cannot be nested

The <figcaption> that we use in <figure> elements for each product cannot be nested inside another element and needs to be a direct child of <figure>

image

Headings "skip" from h1 to h3

Headings should not skip levels, so this h3 should be an h2, or not use heading markup.

image

→ Category pages

"Critical" issues

Missing alt attribute

Our product thumbnails do not have an alt attribute O_o

→ Specific product page

"Critical" issues

Missing alt attribute

Our product thumbnail does not have an alt attribute O_o

"Moderate" issues

missing <h1>

Documents should have at least one top-level heading element, whereas the highest heading for products is <h2> (the product name)

→ About page

"Moderate" issues

Headings "skip"

Headings should not skip levels, so this h3 should be an h2, or not use heading markup. Given that it's just more text, this should probably be <p> content with appropriate classes.

image

@Pomax Pomax added the epic label May 14, 2020
@phildexter phildexter modified the milestones: May 18, June 2 May 19, 2020
@Pomax
Copy link
Contributor

Pomax commented Jun 1, 2020

With all this work landed, a new a11y pass needs to be done to determine what work is still left on the PNI a11y testing front.

@Pomax
Copy link
Contributor

Pomax commented Jun 1, 2020

homepage

  • Text for The 😮 below shows how creepy users find these products. should be gray-60
  • Footer text "Protect the internet as a global public resource" should be <p> with h5-heading CSS, no an actual <h5>
  • Footer text "More about us" should be a <p> as above

@Pomax
Copy link
Contributor

Pomax commented Jun 1, 2020

Category page

  • company + name should be <p>, not <h3>

@Pomax
Copy link
Contributor

Pomax commented Jun 1, 2020

About page

  • side nav should use gray-60
  • inner <main> should be <div>

@Pomax
Copy link
Contributor

Pomax commented Jun 3, 2020

And we're done.

@Pomax Pomax closed this as completed Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Accessibility buyer's guide 🛍 Issues related to the buyer's guide engineering epic
Projects
None yet
Development

No branches or pull requests

5 participants