-
Notifications
You must be signed in to change notification settings - Fork 21
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 all 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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import type {AuthContextInput} from './types.js' | ||
| import type {AuthContextInput, ColorSchemePreference, ReducedMotionPreference} from './types.js' | ||
| import * as core from '@actions/core' | ||
| import {AuthContext} from './AuthContext.js' | ||
| import {findForUrl} from './findForUrl.js' | ||
|
|
@@ -11,11 +11,31 @@ 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}) | ||
|
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: ReducedMotionPreference | undefined | ||
| if (reducedMotionInput) { | ||
| if (!['reduce', 'no-preference', null].includes(reducedMotionInput)) { | ||
| throw new Error( | ||
| "Input 'reduced_motion' must be one of: 'reduce', 'no-preference', or null per Playwright documentation.", | ||
| ) | ||
| } | ||
| reducedMotion = reducedMotionInput as ReducedMotionPreference | ||
| } | ||
| const colorSchemeInput = core.getInput('color_scheme', {required: false}) | ||
| let colorScheme: ColorSchemePreference | undefined | ||
| if (colorSchemeInput) { | ||
| if (!['light', 'dark', 'no-preference', null].includes(colorSchemeInput)) { | ||
| throw new Error( | ||
| "Input 'color_scheme' must be one of: 'light', 'dark', 'no-preference', or null per Playwright documentation.", | ||
| ) | ||
| } | ||
| colorScheme = colorSchemeInput as ColorSchemePreference | ||
| } | ||
|
|
||
| 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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,8 @@ jobs: | |
| # auth_context: # Optional: Stringified JSON object for complex authentication | ||
| # skip_copilot_assignment: false # Optional: Set to true to skip assigning issues to GitHub Copilot (or if you don't have GitHub Copilot) | ||
| # include_screenshots: false # Optional: Set to true to capture screenshots and include links to them in filed issues | ||
| # reduced_motion: no-preference # Optional: Playwright reduced motion configuration option | ||
| # color_scheme: light # Optional: Playwright color scheme configuration option | ||
| ``` | ||
|
|
||
| > 👉 Update all `REPLACE_THIS` placeholders with your actual values. See [Action Inputs](#action-inputs) for details. | ||
|
|
@@ -115,6 +117,8 @@ Trigger the workflow manually or automatically based on your configuration. The | |
| | `auth_context` | No | If scanned pages require authentication, a stringified JSON object containing username, password, cookies, and/or localStorage from an authenticated session | `{"username":"some-user","password":"***","cookies":[...]}` | | ||
| | `skip_copilot_assignment` | No | Whether to skip assigning filed issues to GitHub Copilot. Set to `true` if you don't have GitHub Copilot or prefer to handle issues manually | `true` | | ||
| | `include_screenshots` | No | Whether to capture screenshots of scanned pages and include links to them in filed issues. Screenshots are stored on the `gh-cache` branch of the repository running the workflow. Default: `false` | `true` | | ||
| | `reduced_motion` | No | Playwright `reducedMotion` setting for scan contexts. Allowed values: `reduce`, `no-preference` | `reduce` | | ||
| | `color_scheme` | No | Playwright `colorScheme` setting for scan contexts. Allowed values: `light`, `dark`, `no-preference` | `dark` | | ||
|
|
||
| --- | ||
|
|
||
|
|
@@ -148,11 +152,11 @@ The a11y scanner leverages GitHub Copilot coding agent, which can be configured | |
|
|
||
| 💬 We welcome your feedback! To submit feedback or report issues, please create an issue in this repository. For more information on contributing, please refer to the [CONTRIBUTING](./CONTRIBUTING.md) file. | ||
|
|
||
| ## License | ||
| ## License | ||
|
|
||
| 📄 This project is licensed under the terms of the MIT open source license. Please refer to the [LICENSE](./LICENSE) file for the full terms. | ||
|
|
||
| ## Maintainers | ||
|
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. autoformatting stripped spaces in VSCode. |
||
| ## Maintainers | ||
|
|
||
| 🔧 Please refer to the [CODEOWNERS](./.github/CODEOWNERS) file for more information. | ||
|
|
||
|
|
||
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.