-
Notifications
You must be signed in to change notification settings - Fork 31
Add workflow input options for reducedMotion and colorScheme
#145
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
Changes from 3 commits
7911d38
4f418b1
ac972a3
88c9363
8d104a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,11 +11,27 @@ export default async function () { | |
| const authContext = new AuthContext(authContextInput) | ||
|
|
||
| const includeScreenshots = core.getInput('include_screenshots', {required: false}) !== 'false' | ||
| const reducedMotionInput = core.getInput('reduced_motion', {required: false}) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again, non-blocking. so building on what I said in the comment before - most of this could be moved into a context builder function somewhere and called here. I also personally find it valuable to wrap input reads in some kind of 'getter' or 'reader' function to isolate how each input is read/parsed (or if an error needs to be thrown, or if we need to transform the input somehow) before being exposed to the rest of the code (here's one similar example - although, this does a few more things than just read the input).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, once we've trimmed down the list of feature requests I think it would be a good time to do some code maintenance / refactoring (since the scope of the codebase is expanding). Perhaps once we've got your plugin functionality merged? That seems worthy of a major version update. |
||
| let reducedMotion: 'reduce' | 'no-preference' | undefined | ||
| if (reducedMotionInput) { | ||
| if (!['reduce', 'no-preference'].includes(reducedMotionInput)) { | ||
| throw new Error("Input 'reduced_motion' must be one of: 'reduce', 'no-preference'") | ||
| } | ||
| reducedMotion = reducedMotionInput as 'reduce' | 'no-preference' | ||
| } | ||
| const colorSchemeInput = core.getInput('color_scheme', {required: false}) | ||
| let colorScheme: 'light' | 'dark' | 'no-preference' | undefined | ||
| if (colorSchemeInput) { | ||
| if (!['light', 'dark', 'no-preference'].includes(colorSchemeInput)) { | ||
| throw new Error("Input 'color_scheme' must be one of: 'light', 'dark', 'no-preference'") | ||
| } | ||
| colorScheme = colorSchemeInput as 'light' | 'dark' | 'no-preference' | ||
| } | ||
|
|
||
| const findings = [] | ||
| for (const url of urls) { | ||
| core.info(`Preparing to scan ${url}`) | ||
| const findingsForUrl = await findForUrl(url, authContext, includeScreenshots) | ||
| const findingsForUrl = await findForUrl(url, authContext, includeScreenshots, reducedMotion, colorScheme) | ||
| if (findingsForUrl.length === 0) { | ||
| core.info(`No accessibility gaps were found on ${url}`) | ||
| continue | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
this is not blocking.
Since we're now generating a higher-level context object (that includes auth context and other settings), I think it would be valuable to have a 'contextBuilder' (or whatever you want to call it) that generates the full context outside of this function and passes in 1 context object (maybe even create a specific type of what we expect this context to contain). I would image somewhere in the main function in the index file, something like:
side-note. this is more of a preference than a suggestion; we don't even have to pass the context down as a param if we don't want to. we can use a provider (contextProvider) that generates the context and caches it, and any function that needs it can import it and read it directly.