Address accessibility feedback from audit

thibaudcolas commented 1 year ago

Audit by Scott: #76.

thibaudcolas commented 1 year ago

Copying the findings from Scott,

WAVE

Errors

[x] Search input field is missing a label
[x] Feedback form textarea (only on content pages, only visible after clicking frowny face) is missing a label
[x] en-latest is not a valid value for the lang attribute on the html element
- Related to #16 and #143. I am not very familiar with Wagtail localization or know if this combo of language and version is a common pattern, but it seems like using an invalid value for html lang could be problematic.
[x] aria-labelledby="searchModalLabel" on <div class="modal fade" id="search-modal" tabindex="-1" aria-labelledby="searchModalLabel" aria-hidden="true"> is invalid (and, I believe, unnecessary)

Warnings

There are four screenshots flagged as having "long" alt text, but I feel that this is appropriate for this sort of documentation.

Firefox dev tools accessibility panel

[x] SVG elements (e.g., homepage illustrations, note header ℹ️ icons) are presumed to have ARIA role of diagram and thus it's problematic that they have no text label. Should likely be marked as aria-hidden.
[ ] Broken links in rich text render an <a> element with no href

Axe

[x] There is no main landmark on the homepage. There is one on content pages, but it's just an extra wrapper. Should replace <div class="app__content" id="main-content"> in both page templates with <main class="app__content" id="main-content"> and remove the extra main from the content page template.
[x] The note header ℹ️ icons have id="Layer_1" attributes, which creates duplicate ids when more than one are on a page.

Pa11y

Tweaks we could make to hit Level AAA

[x] Update homepage section description text color (e.g., "Our tutorials will help you learn and master the different ways of creating and managing content in Wagtail. It’s the best place to get started.") to --w-color-grey-600.

Manual observations

Keyboard navigation

[x] User bar should be moved after the "Skip to main content" link, per the guidance added added when Storm made it more accessible in 2.14.
[ ] When focusing the button that reveals the search modal and activating it, focus does not move to the search text input. One more tab focuses it, but I think maybe it should automatically focus it?
[ ] Moving focus to the search input via keyboard tab key (but not via mouse click) initiates a search before you even type anything in the field, resulting in 0 results found.
[x] The left edge of the focus ring is cut off on the main navigation, all the way down, and the top edge is cut off on the top item:
[x] The language/version menu is included in #main-content and is the first thing you tab onto after activating the skip link, which feels wrong. I think placing it before or after the search button probably makes the most sense?

macOS VoiceOver

[x] The skip link is the sole item in an unordered list, so VO unnecessarily reads "list, 1 item"
[ ] When activating the search modal, it reads "Search... and one more item, dialog". It seems to be getting the "Search..." from the input placeholder, which is interesting. I think it would be ideal if we could get this to just say "Search, dialog".
[ ] If you do nothing after that, it says "You are currently on a text field, inside of web content. To enter text in this field, type." The problem is, the input is not actually focused, and typing does nothing. I'm not sure why it thinks you can type without hitting tab again to focus the input.
[ ] After typing some text in the search box, there is no audible indication that suggested results have populated below.
[ ] When stepping through the main navigation, there is no audible indication that you have focused the page you are currently on.
[ ] On the homepage, when it reads the four big links in the main content area, it does not pause at all between the heading and the description that follows, resulting in odd-sounding stuff like "Tutorial our tutorials will help you…". We may need to insert some visually hidden characters to get it to pause in between.
[x] The previous/next buttons at the bottom of a page should be wrapped in a <nav> element so that a screen reader will announce that you have entered a navigation area.
[x] The previous/next buttons should be given aria-labels that prefix the name of the destination page with "Previous page:" or "Next page:", to indicate audibly what the arrow icons indicate visually.
[ ] Pressing the happy button with the Enter key results in no audible feedback; pressing the happy button with VO+Space says "press happy, button", but nothing else. After pressing (with either method), pressing Tab moves on to the footer, and pressing VO+right loops back around to the top of the page for some reason.
[ ] Pressing the unhappy button with the Enter key results in no audible feedback; pressing the happy button with VO+Space says "press unhappy, button", but nothing else. After pressing (with either method), pressing Tab moves to the feedback textarea, but it just says "edit text blank", with no indication of what you should put there; pressing VO+right loops back around to the top of the page for some reason.
[ ] When entering the footer area, different browsers announce it differently. Firefox announces it as "content", Chrome announced is as "footer", and Safari announces it as "content information". Maybe we can set an ARIA label to make this consistent? Or maybe users know what to expect from their browser and we shouldn't mess with it.

Other miscellaneous accessibility concerns

[ ] The skip link is wrapped in <nav class="skip-link" aria-label="Quick links"> – "Quick links" seems a bit of a misnomer, but want to double-check whether wrapping a skip link in a nav element is good practice in the first place.
[x] "Note" headings should not be h2 elements, and probably not headings at all. I suspect a heading with text that is merely "Note" is not particularly useful for navigation or general understanding of document structure. (also mentioned in #150)

Other random non-a11y stuff I don't want to forget

[x] The note header ℹ️ icon SVG markup has a bunch of empty <g> elements.
[x] Full-width images placed via Draftail are requesting 800px-wide renditions, but at max layout width, the images end up rendering at 900px wide, resulting in some upscaling and fuzziness.
[ ] Font sizing for a simple paragraph on a content page is going through quite a journey…
[x] We use "Wagtail user guide", "Wagtail User guide", and "Wagtail User Guide" in various places. We should pick one and stick with it. As it's the title of a publication, I would prefer full Title Case.
[ ] Besides the potential AAA contrast fix mentioned above, it would be good generally to ensure we're only using colors from the current Wagtail style guide.
[x] Search results need some design love:
[ ] Search results in the modal include the category of the result; the dedicated search results page does not include this.
[ ] Search results in the modal (but not on the dedicated results page) are linking to the old non-production domain.

nicklee commented 1 year ago

I've ticked off the items I believe are addressed by #204, this fixes quite a bit of the items but we still have a fair few to address.

Scotchester commented 1 year ago

I've ticked off the items I believe are addressed by #204, this fixes the majority of items but we still have a fair few to address.

Awesome, @nicklee! Will try to review that soon.

lb- commented 1 year ago

Not sure if it's already listed (could not see it) but the skip link is cut off on < md breakpoints and not visible at all on < small breakpoints.

Even though small devices are less likely to have a keyboard, I would expect this link to become visible no matter what the viewport width.

thibaudcolas commented 1 year ago

Ticked off a couple more items based on recent changes. If you spot broken links please let me know.

thibaudcolas commented 1 year ago

@Scotchester from an accessibility standpoint, do you think we should process broken links differently in Wagtail when rendering them on a page?

Here is the logic where we discard URLs and on save page identifiers when storing rich text links in the DB: https://github.com/wagtail/wagtail/blob/1434a93c5418b2dbd4ec22e7ba2761c62f6fcb51/wagtail/admin/rich_text/converters/contentstate.py#L24-L28

And here is where we create <a> tags with no href if we can’t resolve a page link: https://github.com/wagtail/wagtail/blob/919dab00e5e4e66e936dc93cff52793785cf4080/wagtail/rich_text/pages.py#L24

It would be possible for us to give those links a dummy href, or store the page’s URL at the point of creation of the link and use that as a fallback (which would still very likely cause a 404 but one that might be easier to track down).

Scotchester commented 1 year ago

It's a tricky question... I think there are valid reasons to let it render broken links (for example, the text around them could indicate the presence of a link, and getting a broken one might be less confusing than not seeing one at all), but I remember being confused by the way the screen reader handled the <a> with no href. I don't remember the specifics of how it did that, though; I should have recorded them. I will test again to refresh my memory.

Scotchester commented 1 year ago

store the page’s URL at the point of creation of the link and use that as a fallback (which would still very likely cause a 404 but one that might be easier to track down)

I will say, though, speaking for myself as a general user, I would prefer this approach.

wagtail / guide