-
Notifications
You must be signed in to change notification settings - Fork 52
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
Provide conditional usage for 'setupMiddlewareReporter' #267
Conversation
Is this still a draft or do you think it's ready, @drewlee? |
If you don't see any glaring issues, then I can pull this out of draft @scalvert. These changes are pretty stable, so I will go ahead in the interest of collab. |
@@ -0,0 +1,10 @@ | |||
import { ENABLE_A11Y_MIDDLEWARE_REPORTER } from './cli-options'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we rename this to useMiddlewareReporter
, should we also rename this const to USE_A11Y_MIDDLEWARE_REPORTER
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather keep it as is, so it's consistent with ENABLE_A11Y_AUDIT
and enableA11yAudit=true
.
Overall this looks good. Thanks, @drewlee! |
Co-authored-by: Steve Calvert <steve.calvert@gmail.com>
Co-authored-by: Steve Calvert <steve.calvert@gmail.com>
Co-authored-by: Steve Calvert <steve.calvert@gmail.com>
Co-authored-by: Steve Calvert <steve.calvert@gmail.com>
Base implementation to address #266.
The basic idea is to output the environment variables in
addon-test-support/cli-options.ts
. If either one is specified, their corresponding value is overridden at build-time totrue
, and thus accessible from the browser/headless-browser environment.This also provides a new
useMiddlewareReporter
API, to conditionally invokesetupMiddlewareReporter
.Either one of these invocations will make
useMiddlewareReporter
returntrue
, otherwise it'sfalse
:Either one of these invocations will force a11y audit:
Ran this implementation with several apps to verify builds and tests work as expected.