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

starts automated accessibility steps: loads @axe-core/react on next dev, enables eslint-plugin-jsx-a11y on strict #245

Merged
merged 3 commits into from
Aug 20, 2021

Conversation

mattxwang
Copy link
Member

@mattxwang mattxwang commented Aug 18, 2021

This PR does a couple of things:

  1. installs and enables eslint-plugin-jsx-a11y, and turns it on with strict mode
    • this triggers on the navbar improperly using the <label> element, which is a valid concern. reworking it involves changing the DOM structure of the navbar, which in turn breaks the CSS - I'd like to table fixing this until we rework the navbar
    • this also triggers on next's <Link><a></a></Link> pattern, which is realistically a false-positive. Unfortunately, there's no great solution to this, and as such I've temporarily disabled the rule for <Link>; see the comments in the eslint config for more info
  2. installs and enables @axe-core/react, which logs accessibility issues to the console in dev
    • there are a ton of issues to resolve. so, I'll scope that out for another PR instead.

Something that's still left to be desired is to get aXe's level of accessibility specificity, but getting it as a CI action (like eslint-plugin-jsx-a11y) - will figure that out soon!

Part of #218.

@mattxwang mattxwang marked this pull request as draft August 18, 2021 22:59
@mattxwang mattxwang changed the title loads @axe-core/react on next dev starts automated accessibility steps: loads @axe-core/react on next dev, enables eslint-plugin-jsx-a11y on strict Aug 18, 2021
@mattxwang mattxwang marked this pull request as ready for review August 19, 2021 02:58
@mattxwang mattxwang requested a review from a team August 19, 2021 02:58
Copy link
Member

@evanzhong evanzhong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

Small thing:

  • this triggers on the navbar improperly using the element, which is a valid concern. reworking it involves changing the DOM structure of the navbar, which in turn breaks the CSS

    • I think tabling this rather large change is the right call here. If you haven't already, could you create a new issue outlining a navbar refactor and reference this PR.

@mattxwang
Copy link
Member Author

Huh, having some problems w/ the merge - let me roll this back.

@mattxwang
Copy link
Member Author

I think tabling this rather large change is the right call here. If you haven't already, could you create a new issue outlining a navbar refactor and reference this PR.

Yup, will do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants