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 1 commit
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 @@ -109,7 +109,7 @@

expect(mockedRun).toHaveBeenCalledWith(context.parameters.a11y);

expect(context.reporting.addReport).toHaveBeenCalledWith({

Check failure on line 112 in code/addons/a11y/src/preview.test.tsx

View workflow job for this annotation

GitHub Actions / Core Unit Tests, windows-latest

src/preview.test.tsx > afterEach > should run accessibility checks and report results

AssertionError: expected "spy" to be called with arguments: [ { type: 'a11y', version: 1, …(2) } ] Received: 1st spy call: Array [ Object { "result": Object { "violations": Array [ Object { "description": "Ensures the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds", "help": "Elements must meet minimum color contrast ratio thresholds", "helpUrl": "https://dequeuniversity.com/rules/axe/4.8/color-contrast?application=axeAPI", "id": "color-contrast", "impact": "serious", "nodes": Array [ Object { "all": Array [], "any": Array [ Object { "_constructor-name_": "CheckResult", "data": Object { "bgColor": "#f6f9fc", "contrastRatio": 2.76, "expectedContrastRatio": "4.5:1", "fgColor": "#029cfd", "fontSize": "10.5pt (14px)", "fontWeight": "normal", "messageKey": null, "shadowColor": "_undefined_", }, "id": "color-contrast", "impact": "serious", "message": "Element has insufficient color contrast of 2.76 (foreground color: #029cfd, background color: #f6f9fc, font size: 10.5pt (14px), font weight: normal). Expected contrast ratio of 4.5:1", "relatedNodes": Array [ Object { "html": "<div class=\"css-1av19vu\">", "target": Array [ ".css-1av19vu", ], }, ], }, ], "failureSummary": "Fix any of the following: Element has insufficient color contrast of 2.76 (foreground color: #029cfd, background color: #f6f9fc, font size: 10.5pt (14px), font weight: normal). Expected contrast ratio of 4.5:1", "html": "<span class=\"css-1mjgzsp\">", "impact": "serious", "none": Array [], "target": Array [ ".css-1mjgzsp", ], }, ], "tags": "_duplicate_[\"args\",\"0\",\"reporters\",\"0\",\"result\",\"passes\",\"12\",\"tags\"]", }, ], }, - "status": "failed", + "status": "warning", "type": "a11y", "version": 1, }, ] 2nd spy call: Array [ Object { "result": Object { - "violations": Array [ - Object { - "description": "Ensures the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds", - "help": "Elements must meet minimum color contrast ratio thresholds", - "helpUrl": "https://dequeuniversity.com/rules/axe/4.8/color-contrast?application=axeAPI", - "id": "color-contrast", - "impact": "serious", - "nodes": Array [ - Object { - "all": Array [], - "any": Array [ - Object { - "_constructor-name_": "CheckResult", - "data": Object { - "bgColor": "#f6f9fc", - "contrastRatio": 2.76, - "expectedContrastRatio": "4.5:1", - "fgColor": "#029cfd", - "fontSize": "10.5pt (14px)", - "fontWeight": "normal", - "messageKey": null, - "shadowColor": "_undefined_", - }, - "id": "color-contrast", - "impact": "serious", - "message": "Element has insufficient color contrast of 2.76 (foreground color: #029cfd, background color: #f6f9fc, font size: 10.5pt (14px), font weight: normal). Expected contrast ratio of 4.5:1
type: 'a11y',
version: 1,
result,
Expand All @@ -130,7 +130,7 @@

expect(mockedRun).toHaveBeenCalledWith(context.parameters.a11y);

expect(context.reporting.addReport).toHaveBeenCalledWith({

Check failure on line 133 in code/addons/a11y/src/preview.test.tsx

View workflow job for this annotation

GitHub Actions / Core Unit Tests, windows-latest

src/preview.test.tsx > afterEach > should run accessibility checks and report results without throwing

AssertionError: expected "spy" to be called with arguments: [ { type: 'a11y', version: 1, …(2) } ] Received: 1st spy call: Array [ Object { "result": Object { "violations": Array [ Object { "description": "Ensures the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds", "help": "Elements must meet minimum color contrast ratio thresholds", "helpUrl": "https://dequeuniversity.com/rules/axe/4.8/color-contrast?application=axeAPI", "id": "color-contrast", "impact": "serious", "nodes": Array [ Object { "all": Array [], "any": Array [ Object { "_constructor-name_": "CheckResult", "data": Object { "bgColor": "#f6f9fc", "contrastRatio": 2.76, "expectedContrastRatio": "4.5:1", "fgColor": "#029cfd", "fontSize": "10.5pt (14px)", "fontWeight": "normal", "messageKey": null, "shadowColor": "_undefined_", }, "id": "color-contrast", "impact": "serious", "message": "Element has insufficient color contrast of 2.76 (foreground color: #029cfd, background color: #f6f9fc, font size: 10.5pt (14px), font weight: normal). Expected contrast ratio of 4.5:1", "relatedNodes": Array [ Object { "html": "<div class=\"css-1av19vu\">", "target": Array [ ".css-1av19vu", ], }, ], }, ], "failureSummary": "Fix any of the following: Element has insufficient color contrast of 2.76 (foreground color: #029cfd, background color: #f6f9fc, font size: 10.5pt (14px), font weight: normal). Expected contrast ratio of 4.5:1", "html": "<span class=\"css-1mjgzsp\">", "impact": "serious", "none": Array [], "target": Array [ ".css-1mjgzsp", ], }, ], "tags": "_duplicate_[\"args\",\"0\",\"reporters\",\"0\",\"result\",\"passes\",\"12\",\"tags\"]", }, ], }, - "status": "failed", + "status": "warning", "type": "a11y", "version": 1, }, ] Number of calls: 1 ❯ src/preview.test.tsx:133:41
type: 'a11y',
version: 1,
result,
Expand All @@ -156,60 +156,6 @@
});
});

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 ? 'warning' : 'passed',
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: this change means all violations will now show as warnings in Storybook UI, rather than being able to configure some as errors - consider if this reduced severity is appropriate

});

/**
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
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: This file is being completely removed, which removes documentation for a feature that users may be actively using. The PR should include updates to MIGRATION.md to inform users about this breaking change and provide alternative approaches if available.

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
<IfRenderer 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:

</IfRenderer>

<IfRenderer renderer={['angular', 'vue', 'web-components', 'ember', 'html', 'react', 'preact', 'qwik', 'solid']}>
Expand Down Expand Up @@ -130,18 +130,6 @@ If you enabled the addon and you're manually upgrading to Storybook 8.5 or later

<CodeSnippets path="storybook-addon-a11y-test-setup.md" />

### Override accessibility violation levels
Copy link
Contributor

Choose a reason for hiding this comment

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

@kasperpeulen — Heads up that you'll need to merge next because I merged #30035, which updates this file.

Also, if we're removing this, should we document reviewOnFail instead?


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 Vitest.

### Configure accessibility tests with the test addon

If you want to run accessibility tests only for a subset of your stories, you can use the [tags](../writing-stories/tags.mdx) mechanism to filter the tests you want to run with the test addon. For example, to turn off accessibility tests for a specific story, add the `!a11ytest` tag to the story's meta or directly to the story's `tags` configuration option. For example:
Expand All @@ -154,8 +142,8 @@ If you want to run accessibility tests only for a subset of your stories, you ca

<Callout variant="info">

Tags can be applied at the project, component (meta), or story levels. Read our [documentation](../writing-stories/tags.mdx) for more information on configuring tags.
Tags can be applied at the project, component (meta), or story levels. Read our [documentation](../writing-stories/tags.mdx) for more information on configuring tags.

</Callout>

</If>
Expand Down
Loading