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

Accessibility support targets & tooling setup (#4871) #5245

Merged
merged 5 commits into from Apr 29, 2019

Conversation

@thibaudcolas
Copy link
Member

commented Apr 23, 2019

This adds a suite of tools and related configuration / documentation, to help us improve Wagtail’s accessibility. Related to #4871, but does not cover the automated integration tests bit. The idea is that once this PR is merged it should be possible for us to make accessibility fixes and meaningfully review UI changes for accessibility issues, even if it's more of a manual process to start with.

Here is what's included:

Documentation of accessibility support target

I put them alongside the browser support targets for now. These targets are both:

  • Ambitious, since we don't formally offer any kind of support at the moment
  • Very basic. I chose the assistive technologies that are the easiest to set up and use, and are available at no cost.

I think it's important to have explicit targets so we know where we want to be, nonetheless we can't expect people to actually use all of those tools when making or reviewing changes, so I tried to use the right language to make that clear.

jinjalint

Static analysis for Django templates which should help enforcing HTML is well-formed. Its checks aren't comprehensive, but it makes up for that by being much faster than anything aiming for comprehensiveness. (60s to crawl the whole codebase, 200ms per individual template file).

I have decided to discard its output for now (|true), because I don't want this tooling PR to be held up by those fixes. I'll do that next though (#5258), so we can start to properly rely on the linter.

For reference, here are the issues it found (malformed HTML, missing end tags, mismatched start/end tags):

$ jinjalint --parse-only wagtail/**/*.html
wagtail/admin/templates/wagtailadmin/collection_privacy/ancestor_privacy.html:8:2: Parse error: expected 'p' at 7:2
wagtail/admin/templates/wagtailadmin/edit_handlers/chooser_panel.html:26:150: Parse error: expected '>' at 25:150
wagtail/admin/templates/wagtailadmin/page_privacy/ancestor_privacy.html:8:2: Parse error: expected 'p' at 7:2
wagtail/admin/templates/wagtailadmin/pages/listing/_button_with_dropdown.html:15:6: Parse error: expected 'ul' at 14:6
wagtail/admin/templates/wagtailadmin/pages/listing/_list_explore.html:69:271: Parse error: expected 'p' at 68:271
wagtail/admin/templates/wagtailadmin/permissions/includes/collection_member_permissions_formset.html:43:2: Parse error: expected 'tbody' at 42:2
wagtail/contrib/forms/templates/wagtailforms/list_submissions.html:34:14: Parse error: expected 'td' at 33:14
wagtail/contrib/redirects/templates/wagtailredirects/results.html:19:8: Parse error: expected one of 'autoescape', 'block', 'blocktrans', 'comment', 'filter', 'for', 'if', 'ifchanged', 'ifequal', 'ifnotequal', 'not an intermediate Jinja tag name', 'spaceless', 'verbatim', 'with' at 18:8
wagtail/contrib/styleguide/templates/wagtailstyleguide/base.html:434:205: Parse error: expected one of 'autoescape', 'block', 'blocktrans', 'comment', 'filter', 'for', 'if', 'ifchanged', 'ifequal', 'ifnotequal', 'not an intermediate Jinja tag name', 'spaceless', 'verbatim', 'with' at 433:205
wagtail/documents/templates/wagtaildocs/documents/edit.html:66:5: Parse error: expected one of '[:a-z]', 'area', 'base', 'br', 'col', 'command', 'embed', 'hr', 'img', 'input', 'keygen', 'link', 'meta', 'param', 'script', 'source', 'style', 'track', 'wbr', '{#', '{%', '{{' at 65:5
wagtail/documents/templates/wagtaildocs/multiple/add.html:40:10: Parse error: expected 'p' at 39:10
wagtail/images/templates/wagtailimages/multiple/add.html:41:10: Parse error: expected 'p' at 40:10
wagtail/project_template/home/templates/home/welcome_page.html:6:70: Parse error: expected one of '=', '>', '[0-9-_.]', '[:a-z]', '\\s+', '{#', '{%', '{{' at 5:70
wagtail/snippets/templates/wagtailsnippets/snippets/edit.html:13:11: Parse error: expected one of 'autoescape', 'block', 'blocktrans', 'comment', 'endif', 'filter', 'for', 'if', 'ifchanged', 'ifequal', 'ifnotequal', 'not an intermediate Jinja tag name', 'spaceless', 'verbatim', 'with' at 12:11
wagtail/users/templates/wagtailusers/groups/includes/page_permissions_formset.html:43:2: Parse error: expected 'tbody' at 42:2
wagtail/users/templates/wagtailusers/groups/list.html:18:14: Parse error: expected 'tr' at 17:14

react-axe

An integration of Axe similar to how it would be used for browser extensions, except that:

  • It's part of the build tools, so doesn't require extra setup by individual contributors
  • It automatically runs whenever React components render on the page, without having to manually trigger it.
  • It de-duplicates issues found between runs, or the same run between components
  • Contrary to what the name suggests, it also audits parts of the page that aren't built with React (it just won't do so whenever they dynamically change).

Accessibility Insights for Web

Also uses Axe under the hood, and complements its automated checks by very useful semi-manual ones, and a wizard to do manual auditing.

Axe extension for Chrome

Same as what react-axe does, except can be used in production. Also the same as Accessibility Insights for Web, but it might be more widely used and is equally as relevant.


  • Do the tests still pass? (http://docs.wagtail.io/en/latest/contributing/developing.html#testing)
  • Does the code comply with the style guide? (Run make lint from the Wagtail root)
  • [ ] For Python changes: Have you added tests to cover the new/fixed behaviour?
  • [ ] For front-end changes: Did you test on all of Wagtail’s supported browsers? Please list the exact versions you tested.
  • [ ] For new features: Has the documentation been updated accordingly?
@squash-labs

This comment has been minimized.

Copy link

commented Apr 23, 2019

Manage this branch in Squash

Test this branch here: https://thibaudcolaschoreaccessibility-lnvsc.squash.io

@thibaudcolas thibaudcolas added this to In progress in WCAG2.1 AA compliance Apr 24, 2019

@thibaudcolas thibaudcolas force-pushed the thibaudcolas:chore/accessibility-tooling branch from 4e6443b to af9bf95 Apr 25, 2019

@thibaudcolas thibaudcolas changed the title [WIP] Accessibility tooling setup (#4871) Accessibility tooling setup (#4871) Apr 25, 2019

@thibaudcolas thibaudcolas force-pushed the thibaudcolas:chore/accessibility-tooling branch from af9bf95 to 74e095b Apr 25, 2019

@thibaudcolas thibaudcolas changed the title Accessibility tooling setup (#4871) Accessibility support targets & tooling setup (#4871) Apr 25, 2019

@thibaudcolas thibaudcolas moved this from In progress to In review in WCAG2.1 AA compliance Apr 25, 2019

Magnification `Windows Magnifier <https://support.microsoft.com/en-gb/help/11542/windows-use-magnifier>`_
Magnification macOS Zoom
Voice control Windows Speech Recognition
Voice control macOS Dictation

This comment has been minimized.

Copy link
@jonnyscholes

jonnyscholes Apr 25, 2019

Member

Should we mention a mobile screenreader here? My experience is that VoiceOver works near identically on iOS and MacOS so maybe we could include an Android offering? Or is that biting off too much for now...

This comment has been minimized.

Copy link
@thibaudcolas

thibaudcolas Apr 26, 2019

Author Member

(for reference, these choices are based on the UK’s GDS 2016 assistive technology survey, with further guidance on what to test with if you want to do testing at no cost)

I'm in two minds about it. It feels really relevant for mobile screenreader(s) / accessibility tools to be on there, and I regularly use my phone's VoiceOver when doing testing. At the same time, Wagtail’s mobile experience is quite poor for sighted users, so aiming for screen reader support there feels like a step too far. I don't know how to decide :/


* Write `valid HTML <https://validator.w3.org/nu/>`_. We target the HTML5 doctype.
* Attach JavaScript behavior with ``data-`` attributes, rather than classes or IDs.
* For comments, use Django templates syntax instead of HTML.

This comment has been minimized.

Copy link
@jonnyscholes

jonnyscholes Apr 25, 2019

Member

Could this be a good place to mention the use of semantic html? Something as simple as "Use semantically appropriate HTML elements where possible." Perhaps with a link to http://html5doctor.com/element-index/

This comment has been minimized.

Copy link
@thibaudcolas

thibaudcolas Apr 26, 2019

Author Member

Yes!


We aim for Wagtail to work in those environments. Our development standards ensure that the site is usable with other assistive technologies. In practice, testing with assistive technology can be a daunting task that requires specialised training – here are tools we rely on to help identify accessibility issues, to use during development and code reviews:

* `react-axe <https://github.com/dequelabs/react-axe>`_ integrated directly in our build tools, to identify actionable issues.

This comment has been minimized.

Copy link
@jonnyscholes

jonnyscholes Apr 25, 2019

Member

Might be worth adding that messages appear in the developer console - I expected to see some kind of UI at first before switching to the console tab.

@jonnyscholes
Copy link
Member

left a comment

Great stuff! All the docs make sense and I was easily able to get all the added tools running as per the docs. axe is pretty awesome gear! I've added a couple of minor comments to the documentation additions but nothing that should block this being merged :)

@thibaudcolas

This comment has been minimized.

Copy link
Member Author

commented Apr 26, 2019

Thanks for the super fast review @jonnyscholes 😊 I'll leave this open a bit longer, make the changes you suggested, and most likely merge this early next week unless there is more feedback / discussion.

@thibaudcolas thibaudcolas force-pushed the thibaudcolas:chore/accessibility-tooling branch from 74e095b to 315224c Apr 29, 2019

@thibaudcolas

This comment has been minimized.

Copy link
Member Author

commented Apr 29, 2019

I've added feedback from @jonnyscholes, will merge this in a few hours unless there is more feedback.

@thibaudcolas thibaudcolas merged commit effb086 into wagtail:master Apr 29, 2019

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
ci/circleci: backend Your tests passed on CircleCI!
Details
ci/circleci: frontend Your tests passed on CircleCI!
Details

WCAG2.1 AA compliance automation moved this from In review to Done Apr 29, 2019

@thibaudcolas thibaudcolas deleted the thibaudcolas:chore/accessibility-tooling branch Apr 29, 2019

@thibaudcolas thibaudcolas added this to the 2.6 milestone Apr 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.