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

Addon A11y: Remove warnings API #30049

Merged
merged 7 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions code/.storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -358,9 +358,5 @@ export const parameters = {
opacity: 0.4,
},
},
a11y: {
warnings: ['minor', 'moderate', 'serious', 'critical'],
},
tags: ['test', 'vitest', '!a11ytest'],
};

export const tags = ['test', 'vitest'];
3 changes: 0 additions & 3 deletions code/addons/a11y/src/params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,11 @@ export interface Setup {
options: RunOptions;
}

type Impact = NonNullable<ImpactValue>;

export interface A11yParameters {
element?: ElementContext;
config?: Spec;
options?: RunOptions;
/** @deprecated Use globals.a11y.manual instead */
manual?: boolean;
disable?: boolean;
warnings?: Impact[];
}
54 changes: 0 additions & 54 deletions code/addons/a11y/src/preview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -156,60 +156,6 @@ describe('afterEach', () => {
});
});

it('should report warning status when there are only warnings', async () => {
const context = createContext({
parameters: {
a11y: {
warnings: ['minor'],
},
},
});
const result = {
violations: [
{ impact: 'minor', nodes: [] },
{ impact: 'critical', nodes: [] },
],
};
mockedRun.mockResolvedValue(result as any);

await expect(async () => experimental_afterEach(context)).rejects.toThrow();

expect(mockedRun).toHaveBeenCalledWith(context.parameters.a11y);
expect(context.reporting.addReport).toHaveBeenCalledWith({
type: 'a11y',
version: 1,
result,
status: 'failed',
});
});

it('should report error status when there are warnings and errors', async () => {
const context = createContext({
parameters: {
a11y: {
warnings: ['minor'],
},
},
});
const result = {
violations: [
{ impact: 'minor', nodes: [] },
{ impact: 'critical', nodes: [] },
],
};
mockedRun.mockResolvedValue(result as any);

await expect(async () => experimental_afterEach(context)).rejects.toThrow();

expect(mockedRun).toHaveBeenCalledWith(context.parameters.a11y);
expect(context.reporting.addReport).toHaveBeenCalledWith({
type: 'a11y',
version: 1,
result,
status: 'failed',
});
});

it('should run accessibility checks if "a11ytest" flag is not available and is not running in Vitest', async () => {
const context = createContext({
tags: [],
Expand Down
9 changes: 2 additions & 7 deletions code/addons/a11y/src/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ export const experimental_afterEach: AfterEach<any> = async ({
}) => {
const a11yParameter: A11yParameters | undefined = parameters.a11y;
const a11yGlobals = globals.a11y;
const warnings = a11yParameter?.warnings ?? [];

const shouldRunEnvironmentIndependent =
a11yParameter?.manual !== true &&
Expand All @@ -38,15 +37,11 @@ export const experimental_afterEach: AfterEach<any> = async ({
if (result) {
const hasViolations = (result?.violations.length ?? 0) > 0;

const hasErrors = result?.violations.some(
(violation) => !warnings.includes(violation.impact!)
);

reporting.addReport({
type: 'a11y',
version: 1,
result: result,
status: hasErrors ? 'failed' : hasViolations ? 'warning' : 'passed',
status: hasViolations ? 'failed' : 'passed',
});

/**
Expand All @@ -58,7 +53,7 @@ export const experimental_afterEach: AfterEach<any> = async ({
* implement proper try catch handling.
*/
if (getIsVitestStandaloneRun()) {
if (hasErrors) {
if (hasViolations) {
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: throwing on any violations is more strict than before when some violations could be configured as warnings - this may break existing tests

// @ts-expect-error - todo - fix type extension of expect from @storybook/test
expect(result).toHaveNoViolations();
}
Expand Down
21 changes: 21 additions & 0 deletions code/sandbox/experimental-nextjs-vite-default-ts/project.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"name": "experimental-nextjs-vite/default-ts",
"$schema": "../../node_modules/nx/schemas/project-schema.json",
"projectType": "application",
"implicitDependencies": [
"storybook",
"core",
"addon-essentials",
"addon-interactions",
"addon-links",
"addon-onboarding",
"blocks",
"experimental-nextjs-vite"
],
"targets": {
"sandbox": {},
"sb:dev": {},
"sb:build": {}
},
"tags": ["ci:normal", "ci:merged", "ci:daily"]
}

This file was deleted.

18 changes: 3 additions & 15 deletions docs/writing-tests/accessibility-testing.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ Customize the a11y ruleset at the story level by updating your story to include
<If renderer="svelte">

If you are using Svelte CSF, you can turn off automated accessibility testing for stories or components by adding globals to your story or adjusting the `defineMeta` function with the required configuration. With a regular CSF story, you can add the following to your story's export or component's default export:

</If>

<If notRenderer={['svelte']}>
Expand Down Expand Up @@ -143,13 +143,13 @@ You can use tags to progressively work toward a more accessible UI by enabling a
```ts title=".storybook/preview.ts"
// Replace your-renderer with the renderer you are using (e.g., react, vue3)
import { Preview } from '@storybook/your-renderer';

const preview: Preview = {
// ...
// 👇 Temporarily remove the a11ytest tag from all stories
tags: ['!a11ytest'],
};

export default preview;
```

Expand All @@ -163,18 +163,6 @@ You can use tags to progressively work toward a more accessible UI by enabling a

1. Pick another component and repeat the process until you've covered all your components and you're an accessibility hero!

### Override accessibility violation levels

By default, when the accessibility addon runs with the test addon enabled, it interprets all violations as errors. This means that if a story has a minor accessibility violation, the test will fail. However, you can override this behavior by setting the `warnings` parameter in the `a11y` configuration object to define an array of impact levels that should be considered warnings.

{/* prettier-ignore-start */}

<CodeSnippets path="storybook-addon-a11y-test-override-warning-levels.md" />

{/* prettier-ignore-end */}

In the example above, we configured all the `minor` or `moderate` accessibility violations to be considered warnings. All other levels (i.e., `serious` or `critical`) will continue to be considered errors, fail the test, and report the results accordingly in the Storybook UI or CLI output.

</If>

## Automate accessibility tests with test runner
Expand Down
7 changes: 1 addition & 6 deletions scripts/tasks/sandbox-parts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -809,12 +809,7 @@ export const extendPreview: Task['run'] = async ({ template, sandboxDir }) => {
const previewConfig = await readConfig({ cwd: sandboxDir, fileName: 'preview' });

if (template.expected.builder.includes('vite')) {
previewConfig.setFieldValue(['tags'], ['vitest']);
// TODO: Remove this once the starter components + test stories have proper accessibility
previewConfig.setFieldValue(
['parameters', 'a11y', 'warnings'],
['minor', 'moderate', 'serious', 'critical']
);
previewConfig.setFieldValue(['tags'], ['vitest', '!a11ytest']);
}

await writeConfig(previewConfig);
Expand Down
Loading