chipzoller / hugo-clarity

A theme for Hugo based on VMware Clarity
Other
572 stars 263 forks source link

Improve accessibility issues #310

Open rootwork opened 2 years ago

rootwork commented 2 years ago

Happy Global Accessibility Awareness Day!

I've been wanting to do this for awhile, and I'm finally taking the time to do it. I ran Hugo Clarity through axe devtools to see how it performs. (You could also run it through the default accessibility checker in Firefox devtools, but I find the axe version more helpful.)

Below are the a11y (accessibility) issues it highlighted in decreasing order of severity. I can create PRs for any/all of these but would like to know if there are reasons why a) a given a11y assessment might be wrong, or b) a given style/design is desired even though it doesn't have good a11y (I hope there aren't many of these).

The links go to pages giving more information about each issue. For ease of responding I'm numbering the issues.

Critical issues

C1. Zooming and scaling should not be disabled

Location: <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">

Suggested fix: Remove user-scalable=no

C2. Form elements must have labels

Location: Light/dark mode switcher.

Suggested fix: Rewrite to include <label> tags. Here's how I'm doing it on my local version of the theme.

C3. Focus state is not visible

Looking at https://github.com/chipzoller/hugo-clarity/blob/master/assets/sass/_base.sass#L71-L73 I see we're disabling visible focus for all links. I understand the design motivation, because they're inconsistent and ugly (and often not accessible!) across browsers, but we can't turn them off and then provide nothing to replace it. Currently there's no indication when navigating by keyboard what the current focus is. In particular we shouldn't disable :focus-visible without a replacement, because the whole point of :focus-visible is that it's only visible during keyboard interaction (so it's not going to interfere with, say, hover styles).

Anyway there are a few different ways to handle this based on context, but at the very least the menu items and the inline links in post content need visible focus styles

Background info and resources on :focus and :focus-visible styling

Serious issues

S1. Elements must have sufficient color contrast (tags, both light and dark modes)

Location: All the post_tags.

Suggested fix: Color contrast is a combination of text color, background color, font size and font weight. The calculated color contrast for tags is 3.95:1 but should be 4.5:1 for basic (AA) a11y. So the simple fix would be to darken the text or lighten the background, but other fixes could include making the text bigger or bolder.

S2. Links must have discernible text

Location: The post (sub-)heading links that appear on hover.

Suggested fix: Right now the <a> element has no text inside; simply give it some text and then hide it visually with CSS. On my site I use the text "permalink".

S3. Elements must have sufficient color contrast (inline links in posts, dark mode)

Location: Inline links when in dark mode, e.g. at the top of the "Rich Content" sample post.

Suggested fix: Link color just needs to be lighter.

S4. Elements must have sufficient color contrast (code blocks with default highlighting, dark mode)

Location: <code class="noClass"> blocks.

Suggested fix: The red font color isn't quite light enough. Lighten the font color for just this portion of the syntax highlighting.

S5. Elements must have sufficient color contrast (main menu drop-down links, light mode)

Location: nav_sub items.

Description: For some reason when navigating by keyboard, the background color of the drop-down links in the main menu doesn't seem to get triggered. I'll have to look at the CSS to see why that is.

Suggested fix: Not sure yet.

S6. Frames must have an accessible name

Location: Embedded <iframe>s, such as on the "Rich Content" sample post.

Suggested fix: Add a title attribute describing what the frame is (in this case something like "video" or "YouTube video")

Moderate issues

M1. Scrollable region must have keyboard access

Location: <code> blocks. To explain, if a given code block is scrollable (that is, some of its content is obscured until the user scrolls it into view) it has to be focusable via keyboard navigation, or else a keyboard user would never be able to see the rest of the content.

Suggested fix: Add tabindex="0" to all <code> blocks. If there are particular blocks that we know will never be scrollable, we should add overflow: hidden to their CSS.

M2. Ensure landmarks are unique

Location: <nav> elements.

Suggested fix: Add aria-labels to each <nav> to distinguish them from one another. If an element already labels them somewhere, we could use aria-labelledby instead.

M3. Page should have a level one (<h1>) heading

Location: Anywhere within <body>.

Suggested fix: There are several:

Minor issues

N1. ID attributes must be unique

Location: <a href="#linkedinshare" id = "linkedinshare" class="linkedin" title="Share on LinkedIn" rel="nofollow">

Suggested fix: Like all menu elements, this menu item exists in two places in order to produce the mobile menu. I'm not sure why it has that id on it -- none of the other social media icons do -- but technically it should have two different ids.

N2. Provide a skip-to-content link

This is completely optional, but I think it would be a nice and easy thing to do. Basically you create a link that is hidden by default, and only becomes visible when it is tabbed to by keyboard. On a site with a lot of items in the menu it makes a big difference.

Background and resources


So as I said at the top, I'm happy to create PRs for all of these, but I wanted to get some feedback first to see if there were specific reasons or things to keep in mind while addressing them.

0xTanvir commented 2 years ago

I think it will be best. It will be suitable for the Core Web Vitals report performance

I noticed two other issues for performance when I was using webp image at the featured image, maybe you can also add these two on the list too

0xTanvir commented 2 years ago

C1: Please also remove maximum-scale=1.0 S2: An aria-label will also be a nice touch like link.ariaLabel = 'Copy link to this section' and some case this line generating conflict URL like when you are coming from a URL which already has a section id

For this URL: https://example.com/foo-bar/#redirected-section-id It will generate: https://example.com/foo-bar/#redirected-section-id#another-section-id Instead, it should generate: https://example.com/foo-bar/#another-section-id

rootwork commented 2 years ago

@chipzoller @onweru Any feedback or opinions on the above? If you don't feel strongly one way or another, I'll just start working on some PRs.

onweru commented 2 years ago

@rootwork, I'm not big on accessibility myself so the issue makes sense as described. Please feel free to proceed as you deem fit.