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

Add new setup command for a11y #1793

Closed
wants to merge 13 commits into from
Closed

Add new setup command for a11y #1793

wants to merge 13 commits into from

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Feb 14, 2021

Since a lot of this is moving into core, what this'll actually setup is @axe-core/react and maybe jest-axe. It could still do a lot more, it just wont' do most of what's below anymore; that's what #1849 is for.

  • docs?

This PR adds a new setup command, a11y, that equips a Redwood project with accessibility tooling like eslint-plugin-jsx-a11y. To start, the only thing it'll setup is eslint-plugin-jsx-a11y, a "static AST checker for a11y rules on JSX elements", but there's plenty room for more.

We want to make it easy to make accessible websites with Redwood. While the majority of that work will be done in the router, just because we manage focus/announcements/scroll doesn't mean your website's accessible—the modals, listboxes, switches, and pretty much anything interactive need to be as well, especially if they're built from scratch with React.

Since we don't want to constrain anyone to a particular UI library we know to be accessible (though we'll certainly recommend one), we need to make sure we provide the tooling you need to make making accessible websites more tractable. This setups up that tooling.

The prior art here is Gatsby https://www.gatsbyjs.com/docs/conceptual/making-your-site-accessible/#linting-with-eslint-plugin-jsx-a11y.

Questions:

  • set rules to warn, or to error?
    • I think the answer to this one isn't too controversial: default to warn, but make it possible to opt into error, failing CI and such if they want to be rigorous
    • gatsby sets them to warn
    • there could be a flag, --set-to-error or something, to setup the rules as errors from the get-go
  • should this just be in core? i.e., in the @redwoodjs/eslint-config package?
    • if we set all the rules to warn (which will probably will), and we're sure it's configured correctly, I feel like the most we risk is it "being annoying"—i.e. a retort I can imagine is "I'm not ready to focus on making my website accessible yet and these warnings are just getting in the way" / "I wish I could turn them off".
      • so is there a way to include them in core, but make something like a toml var to toggle them?
    • if they're in core, they're harder to configure, but I can't imagine most people wanting to configure them
    • try it with a few of the most popular CSS libs, and see if anything unexpected happens. it'd be a big risk if it make a popular CSS lib unusable. the issues I'm thinking of here are:
  • do all the eslint config in web/.eslintrc.js, web/package.json, or somewhere less visible?
    • the @redwoodjs/eslint-config preset's configured in the root package.json. but since this might not be a one-liner the way that one is, it might be better off in it's own file
  • include @axe-core/react, jest-axe, @storybook/addon-a11y?

@github-actions
Copy link

github-actions bot commented Feb 14, 2021

📦 PR Packages

Click to Show Package Download Links

https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/create-redwood-app-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-api-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-api-server-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-auth-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-cli-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-core-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-dev-server-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-eslint-config-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-eslint-plugin-redwood-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-forms-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-internal-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-prerender-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-router-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-structure-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-testing-0.30.0-a7110ae.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1793/redwoodjs-web-0.30.0-a7110ae.tgz

Install this PR by running yarn rw upgrade --pr 1793:0.30.0-a7110ae

@jtoar
Copy link
Contributor Author

jtoar commented Apr 18, 2021

Getting an unexpected linting error for process.env?

image

@cypress
Copy link

cypress bot commented Apr 18, 2021



Test summary

11 0 1 0Flakiness 0


Run details

Project RedwoodJS Framework
Status Passed
Commit ad814c7 ℹ️
Started Apr 19, 2021 12:20 AM
Ended Apr 19, 2021 12:22 AM
Duration 02:43 💡
OS Linux Ubuntu - 20.04
Browser Chrome 89

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@jtoar jtoar marked this pull request as ready for review April 18, 2021 20:17
@jtoar jtoar marked this pull request as draft April 22, 2021 04:20
@jtoar
Copy link
Contributor Author

jtoar commented May 20, 2021

Closing this out for the time being; needs more design time.

@jtoar jtoar closed this May 20, 2021
@jtoar jtoar added the hopper label May 20, 2021
@jtoar jtoar removed the topic/web label Jun 5, 2021
@thedavidprice thedavidprice deleted the ds-setup-a11y-eslint branch July 27, 2021 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants