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

Add discrepancy handling to A11yPanel #29661

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
45 changes: 25 additions & 20 deletions code/addons/a11y/src/components/A11YPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { CheckIcon, SyncIcon } from '@storybook/icons';
import { useA11yContext } from './A11yContext';
import { Report } from './Report';
import { Tabs } from './Tabs';
import { TestDiscrepancyMessage } from './TestDiscrepancyMessage';

export enum RuleType {
VIOLATION,
Expand Down Expand Up @@ -43,7 +44,7 @@ const Centered = styled.span({
});

export const A11YPanel: React.FC = () => {
const { results, status, handleManual, error } = useA11yContext();
const { results, status, handleManual, error, discrepancy } = useA11yContext();

const manualActionItems = useMemo(
() => [{ title: 'Run test', onClick: handleManual }],
Expand Down Expand Up @@ -106,31 +107,35 @@ export const A11YPanel: React.FC = () => {

return (
<>
{status === 'initial' && <Centered>Initializing...</Centered>}
{status === 'manual' && (
<>
<Centered>Manually run the accessibility scan.</Centered>
<ActionBar key="actionbar" actionItems={manualActionItems} />
</>
)}
{status === 'running' && (
<Centered>
<RotatingIcon size={12} /> Please wait while the accessibility scan is running ...
</Centered>
)}
{(status === 'ready' || status === 'ran') && (
{discrepancy && <TestDiscrepancyMessage discrepancy={discrepancy} />}
{status === 'ready' || status === 'ran' ? (
<>
<ScrollArea vertical horizontal>
<Tabs key="tabs" tabs={tabs} />
Copy link
Contributor

Choose a reason for hiding this comment

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

style: key prop on Tabs is unnecessary since it's not in an array/iteration

</ScrollArea>
<ActionBar key="actionbar" actionItems={readyActionItems} />
Copy link
Contributor

Choose a reason for hiding this comment

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

style: key prop on ActionBar is unnecessary since it's not in an array/iteration

</>
)}
{status === 'error' && (
<Centered>
The accessibility scan encountered an error.
<br />
{typeof error === 'string' ? error : JSON.stringify(error)}
) : (
<Centered style={{ marginTop: discrepancy ? '1em' : 0 }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider extracting inline style to a styled component for consistency with the rest of the codebase

{status === 'initial' && 'Initializing...'}
{status === 'manual' && (
<>
<>Manually run the accessibility scan.</>
<ActionBar key="actionbar" actionItems={manualActionItems} />
Comment on lines +122 to +124
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax: Unnecessary nested fragment (<>) inside another fragment. Remove the inner fragment.

Copy link
Contributor

Choose a reason for hiding this comment

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

style: key prop on ActionBar is unnecessary since it's not in an array/iteration

</>
)}
{status === 'running' && (
<>
<RotatingIcon size={12} /> Please wait while the accessibility scan is running ...
</>
)}
{status === 'error' && (
<>
The accessibility scan encountered an error.
<br />
{typeof error === 'string' ? error : JSON.stringify(error)}
</>
)}
</Centered>
)}
</>
Expand Down
77 changes: 68 additions & 9 deletions code/addons/a11y/src/components/A11yContext.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import * as api from 'storybook/internal/manager-api';

import type { AxeResults } from 'axe-core';

import { EVENTS } from '../constants';
import { EVENTS, TEST_PROVIDER_ID } from '../constants';
import { A11yContextProvider, useA11yContext } from './A11yContext';

vi.mock('storybook/internal/manager-api');
Expand Down Expand Up @@ -65,16 +65,22 @@ describe('A11yContext', () => {

const getCurrentStoryData = vi.fn();
const getParameters = vi.fn();

mockedApi.useAddonState.mockImplementation((_, defaultState) => React.useState(defaultState));
mockedApi.useChannel.mockReturnValue(vi.fn());
getCurrentStoryData.mockReturnValue({ id: storyId, type: 'story' });
getParameters.mockReturnValue({});
mockedApi.useStorybookApi.mockReturnValue({ getCurrentStoryData, getParameters } as any);
mockedApi.useParameter.mockReturnValue({ manual: false });
mockedApi.useStorybookState.mockReturnValue({ storyId } as any);
const getCurrentStoryStatus = vi.fn();

beforeEach(() => {
mockedApi.useAddonState.mockImplementation((_, defaultState) => React.useState(defaultState));
mockedApi.useChannel.mockReturnValue(vi.fn());
getCurrentStoryData.mockReturnValue({ id: storyId, type: 'story' });
getParameters.mockReturnValue({});
getCurrentStoryStatus.mockReturnValue({ [TEST_PROVIDER_ID]: { status: 'success' } });
mockedApi.useStorybookApi.mockReturnValue({
getCurrentStoryData,
getParameters,
getCurrentStoryStatus,
} as any);
mockedApi.useParameter.mockReturnValue({ manual: false });
mockedApi.useStorybookState.mockReturnValue({ storyId } as any);

mockedApi.useChannel.mockClear();
mockedApi.useStorybookApi.mockClear();
mockedApi.useAddonState.mockClear();
Expand Down Expand Up @@ -146,6 +152,59 @@ describe('A11yContext', () => {
);
});

it('should set discrepancy to cliFailedButModeManual when in manual mode', () => {
mockedApi.useParameter.mockReturnValue({ manual: true });
getCurrentStoryStatus.mockReturnValue({ [TEST_PROVIDER_ID]: { status: 'error' } });

const Component = () => {
const { discrepancy } = useA11yContext();
return <div data-testid="discrepancy">{discrepancy}</div>;
};

const { getByTestId } = render(
<A11yContextProvider>
<Component />
</A11yContextProvider>
);

expect(getByTestId('discrepancy').textContent).toBe('cliFailedButModeManual');
});

it('should set discrepancy to cliPassedBrowserFailed', () => {
mockedApi.useParameter.mockReturnValue({ manual: true });
getCurrentStoryStatus.mockReturnValue({ [TEST_PROVIDER_ID]: { status: 'success' } });

const Component = () => {
const { discrepancy } = useA11yContext();
return <div data-testid="discrepancy">{discrepancy}</div>;
};

const { getByTestId } = render(
<A11yContextProvider>
<Component />
</A11yContextProvider>
);

const storyFinishedPayload: StoryFinishedPayload = {
storyId,
status: 'error',
reporters: [
{
id: 'a11y',
result: axeResult as any,
status: 'failed',
version: 1,
},
],
};

const useChannelArgs = mockedApi.useChannel.mock.calls[0][0];

act(() => useChannelArgs[STORY_FINISHED](storyFinishedPayload));

expect(getByTestId('discrepancy').textContent).toBe('cliPassedBrowserFailed');
});

it('should handle STORY_RENDER_PHASE_CHANGED event correctly', () => {
const emit = vi.fn();
mockedApi.useChannel.mockReturnValue(emit);
Expand Down
34 changes: 32 additions & 2 deletions code/addons/a11y/src/components/A11yContext.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { FC, PropsWithChildren } from 'react';
import React, { createContext, useCallback, useContext, useEffect, useState } from 'react';
import React, { createContext, useCallback, useContext, useEffect, useMemo, useState } from 'react';

import {
STORY_FINISHED,
Expand All @@ -10,6 +10,7 @@ import {
useAddonState,
useChannel,
useParameter,
useStorybookApi,
useStorybookState,
} from 'storybook/internal/manager-api';
import type { Report } from 'storybook/internal/preview-api';
Expand All @@ -19,9 +20,10 @@ import { HIGHLIGHT } from '@storybook/addon-highlight';

import type { AxeResults, Result } from 'axe-core';

import { ADDON_ID, EVENTS } from '../constants';
import { ADDON_ID, EVENTS, TEST_PROVIDER_ID } from '../constants';
import type { A11yParameters } from '../params';
import type { A11YReport } from '../types';
import type { TestDiscrepancy } from './TestDiscrepancyMessage';

export interface Results {
passes: Result[];
Expand All @@ -40,6 +42,7 @@ export interface A11yContextStore {
setStatus: (status: Status) => void;
error: unknown;
handleManual: () => void;
discrepancy: TestDiscrepancy;
}

const colorsByType = [
Expand All @@ -63,6 +66,7 @@ export const A11yContext = createContext<A11yContextStore>({
status: 'initial',
error: undefined,
handleManual: () => {},
discrepancy: null,
});

const defaultResult = {
Expand All @@ -80,12 +84,15 @@ export const A11yContextProvider: FC<PropsWithChildren> = (props) => {

const getInitialStatus = useCallback((manual = false) => (manual ? 'manual' : 'initial'), []);

const api = useStorybookApi();
const [results, setResults] = useAddonState<Results>(ADDON_ID, defaultResult);
const [tab, setTab] = useState(0);
const [error, setError] = React.useState<unknown>(undefined);
const [status, setStatus] = useState<Status>(getInitialStatus(parameters.manual!));
const [highlighted, setHighlighted] = useState<string[]>([]);

const { storyId } = useStorybookState();
const storyStatus = api.getCurrentStoryStatus();

const handleToggleHighlight = useCallback((target: string[], highlight: boolean) => {
setHighlighted((prevHighlighted) =>
Expand Down Expand Up @@ -178,6 +185,28 @@ export const A11yContextProvider: FC<PropsWithChildren> = (props) => {
emit(HIGHLIGHT, { elements: highlighted, color: colorsByType[tab] });
}, [emit, highlighted, tab]);

const discrepancy: TestDiscrepancy = useMemo(() => {
const storyStatusA11y = storyStatus[TEST_PROVIDER_ID]?.status;

if (storyStatusA11y) {
if (storyStatusA11y === 'success' && results.violations.length > 0) {
return 'cliPassedBrowserFailed';
}

if (storyStatusA11y === 'error' && results.violations.length === 0) {
if (status === 'ready' || status === 'ran') {
return 'browserPassedCliFailed';
}

if (status === 'manual') {
return 'cliFailedButModeManual';
}
}
}

return null;
}, [results.violations.length, status, storyStatus]);

return (
<A11yContext.Provider
value={{
Expand All @@ -191,6 +220,7 @@ export const A11yContextProvider: FC<PropsWithChildren> = (props) => {
setStatus,
error,
handleManual,
discrepancy,
}}
{...props}
/>
Expand Down
73 changes: 73 additions & 0 deletions code/addons/a11y/src/components/TestDiscrepancyMessage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import React, { useMemo } from 'react';

import { Link } from 'storybook/internal/components';
import { useStorybookApi } from 'storybook/internal/manager-api';
import { styled } from 'storybook/internal/theming';

import { DOCUMENTATION_DISCREPANCY_LINK } from '../constants';

const Wrapper = styled.div(({ theme: { color, typography, background } }) => ({
textAlign: 'start',
padding: '11px 15px',
fontSize: `${typography.size.s2}px`,
fontWeight: typography.weight.regular,
lineHeight: '1rem',
background: background.app,
borderBottom: `1px solid ${color.border}`,
color: color.defaultText,
backgroundClip: 'padding-box',
position: 'relative',
code: {
fontSize: `${typography.size.s1 - 1}px`,
color: 'inherit',
margin: '0 0.2em',
padding: '0 0.2em',
background: 'rgba(255, 255, 255, 0.8)',
Copy link
Contributor

Choose a reason for hiding this comment

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

style: background color with alpha channel may not provide sufficient contrast with dark themes

borderRadius: '2px',
boxShadow: '0 0 0 1px rgba(0, 0, 0, 0.1)',
},
}));

export type TestDiscrepancy =
| 'browserPassedCliFailed'
| 'cliPassedBrowserFailed'
| 'cliFailedButModeManual'
| null;

interface TestDiscrepancyMessageProps {
discrepancy: TestDiscrepancy;
}
export const TestDiscrepancyMessage = ({ discrepancy }: TestDiscrepancyMessageProps) => {
const api = useStorybookApi();
const docsUrl = api.getDocsUrl({
subpath: DOCUMENTATION_DISCREPANCY_LINK,
versioned: true,
renderer: true,
});

const message = useMemo(() => {
switch (discrepancy) {
case 'browserPassedCliFailed':
return 'Accessibility checks passed in this browser but failed in the CLI.';
case 'cliPassedBrowserFailed':
return 'Accessibility checks passed in the CLI but failed in this browser.';
case 'cliFailedButModeManual':
return 'Accessibility checks failed in the CLI. Run the tests manually to see the results.';
default:
return null;
}
}, [discrepancy]);

if (!message) {
return null;
}

return (
<Wrapper>
{message}{' '}
<Link href={docsUrl} target="_blank" withArrow>
Learn what could cause this
</Link>
</Wrapper>
);
};
5 changes: 5 additions & 0 deletions code/addons/a11y/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,9 @@ const RUNNING = `${ADDON_ID}/running`;
const ERROR = `${ADDON_ID}/error`;
const MANUAL = `${ADDON_ID}/manual`;

export const DOCUMENTATION_LINK = 'writing-tests/accessibility-testing';
export const DOCUMENTATION_DISCREPANCY_LINK = `${DOCUMENTATION_LINK}#what-happens-when-there-are-different-results-in-multiple-environments`;
Comment on lines +10 to +11
Copy link
Contributor

Choose a reason for hiding this comment

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

style: consider making this a full URL path rather than a relative path to ensure it works across different contexts


export const TEST_PROVIDER_ID = 'storybook/addon-a11y/test-provider';

export const EVENTS = { RESULT, REQUEST, RUNNING, ERROR, MANUAL };
Loading