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: Adjust default behaviour when using with experimental-addon-test #30162

Merged
merged 32 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
54653ef
Enhance a11y addon integration
valentinpalkovic Dec 31, 2024
a1506f3
Refactor TestProviderRender to remove a11ytest tag and pre-select a11…
valentinpalkovic Jan 2, 2025
4283a92
Small text adjustments
valentinpalkovic Jan 2, 2025
5a422cf
Add support for additional file extensions in addon-a11y-addon-test
valentinpalkovic Jan 2, 2025
c43aa09
Update a11y addon test snapshots and enhance transformation logic for…
valentinpalkovic Jan 2, 2025
b0d96bc
Update execa dependency to version 9.5.2 and enhance post-install log…
valentinpalkovic Jan 2, 2025
8de05db
Enhance post-install script to inherit stdio for better logging durin…
valentinpalkovic Jan 2, 2025
335c760
Test
valentinpalkovic Jan 2, 2025
15b941d
Refactor TestProviderRender to improve state management and update co…
valentinpalkovic Jan 2, 2025
3ff5a62
Set a11ytest as preview annotation tags in portable stories run
valentinpalkovic Jan 2, 2025
e7b81bc
Refactor a11y test configuration handling in vitest-plugin
valentinpalkovic Jan 2, 2025
711a48f
Merge remote-tracking branch 'origin/next' into valentin/a11y-refacto…
valentinpalkovic Jan 3, 2025
d31350d
Merge remote-tracking branch 'origin/next' into valentin/a11y-refacto…
valentinpalkovic Jan 7, 2025
21b452a
Rename a11ytest tag to a11y-test
valentinpalkovic Jan 7, 2025
4554321
Refactor a11y-addon-test to return formatted config instead of raw lines
valentinpalkovic Jan 7, 2025
51cfe14
Fix tests for Windows
valentinpalkovic Jan 7, 2025
980a51e
Fix tests for Windows
valentinpalkovic Jan 8, 2025
b06a35f
Fix tests for Windows
valentinpalkovic Jan 8, 2025
7e75f73
Merge remote-tracking branch 'origin/next' into valentin/a11y-refacto…
valentinpalkovic Jan 9, 2025
03093e1
Refactor addon-a11y automigration to place a11y-test tag as a comment…
valentinpalkovic Jan 9, 2025
090276b
Fix for windows
valentinpalkovic Jan 9, 2025
fc2610f
Fix for windows
valentinpalkovic Jan 9, 2025
a4a3b3d
Return to execa 8.0.1 to reduce bundle size
valentinpalkovic Jan 10, 2025
f31a131
Use inline snapshots instead of normal ones
valentinpalkovic Jan 10, 2025
733be67
Simplify code
valentinpalkovic Jan 10, 2025
9d614f5
Remove invalid file extensions
valentinpalkovic Jan 10, 2025
3441720
Streamline state initialization and add useEffect for state updates
valentinpalkovic Jan 10, 2025
d8a18a5
Merge branch 'next' into valentin/a11y-refactorings
valentinpalkovic Jan 10, 2025
af159c9
Docs updates for `a11y-test` tag behavior
kylegach Jan 13, 2025
51dfb39
Update messages
kylegach Jan 13, 2025
42d7d30
Merge branch 'next' into valentin/a11y-refactorings
valentinpalkovic Jan 13, 2025
5a33146
Update tests
valentinpalkovic Jan 13, 2025
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
2 changes: 1 addition & 1 deletion code/.storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -360,4 +360,4 @@ export const parameters = {
},
};

export const tags = ['test', 'vitest', '!a11ytest'];
export const tags = ['test', 'vitest'];
2 changes: 1 addition & 1 deletion code/addons/a11y/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@ export const TEST_PROVIDER_ID = 'storybook/addon-a11y/test-provider';

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

export const A11Y_TEST_TAG = 'a11ytest';
export const A11Y_TEST_TAG = 'a11y-test';
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 3 additions & 1 deletion code/addons/a11y/src/postinstall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,7 @@ const $ = execa({
});

export default async function postinstall(options: PostinstallOptions) {
await $`storybook automigrate addonA11yAddonTest ${options.yes ? '--yes' : ''}`;
await $({
stdio: 'inherit',
})`storybook automigrate addonA11yAddonTest ${options.yes ? '--yes' : ''}`;
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
}
2 changes: 1 addition & 1 deletion code/addons/a11y/src/preview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ describe('afterEach', () => {
});
});

it('should run accessibility checks if "a11ytest" flag is not available and is not running in Vitest', async () => {
it('should run accessibility checks if "a11y-test" flag is not available and is not running in Vitest', async () => {
const context = createContext({
tags: [],
});
Expand Down
3 changes: 0 additions & 3 deletions code/addons/a11y/src/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,3 @@ export const initialGlobals = {
manual: false,
},
};

// A11Y_TEST_TAG constant in ./constants.ts. Has to be statically analyzable.
export const tags = ['a11ytest'];
62 changes: 50 additions & 12 deletions code/addons/test/src/components/TestProviderRender.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ import {
type TestProviderConfig,
type TestProviderState,
} from 'storybook/internal/core-events';
import { addons } from 'storybook/internal/manager-api';
import { addons, useStorybookState } from 'storybook/internal/manager-api';
import type { API } from 'storybook/internal/manager-api';
import { styled, useTheme } from 'storybook/internal/theming';

import type { Tag } from '@storybook/csf';
import {
AccessibilityIcon,
EditIcon,
Expand Down Expand Up @@ -106,23 +107,49 @@ const statusMap: Record<TestStatus, ComponentProps<typeof TestStatusIcon>['statu
pending: 'pending',
};

export const TestProviderRender: FC<
{
api: API;
state: TestProviderConfig & TestProviderState<Details, Config>;
entryId?: string;
} & ComponentProps<typeof Container>
> = ({ state, api, entryId, ...props }) => {
type TestProviderRenderProps = {
api: API;
state: TestProviderConfig & TestProviderState<Details, Config>;
entryId?: string;
} & ComponentProps<typeof Container>;

export const TestProviderRender: FC<TestProviderRenderProps> = ({
state,
api,
entryId,
...props
}) => {
const [isEditing, setIsEditing] = useState(false);
const theme = useTheme();
const coverageSummary = state.details?.coverageSummary;
const storybookState = useStorybookState();

const isA11yAddon = addons.experimental_getRegisteredAddons().includes(A11Y_ADDON_ID);

const isA11yAddonInitiallyChecked = useMemo(() => {
const internalIndex = storybookState.internal_index;
if (!internalIndex || !isA11yAddon) {
return false;
}

const allTags = Object.values(internalIndex.entries).reduce((acc, entry) => {
entry.tags?.forEach((tag: Tag) => {
acc.add(tag);
});
return acc;
}, new Set<Tag>());

if (allTags.has('a11y-test')) {
return true;
}

return false;
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
}, [isA11yAddon, storybookState.internal_index]);

const [config, updateConfig] = useConfig(
api,
state.id,
state.config || { a11y: false, coverage: false }
state.config || { a11y: isA11yAddonInitiallyChecked, coverage: false }
);

const isStoryEntry = entryId?.includes('--') ?? false;
Expand Down Expand Up @@ -425,14 +452,25 @@ export const TestProviderRender: FC<
};

function useConfig(api: API, providerId: string, initialConfig: Config) {
const [currentConfig, setConfig] = useState<Config>(initialConfig);
const updateTestProviderState = useCallback(
(config: Config) => {
api.updateTestProviderState(providerId, { config });
api.emit(TESTING_MODULE_CONFIG_CHANGE, { providerId, config });
},
[api, providerId]
);

const [currentConfig, setConfig] = useState<Config>(() => {
updateTestProviderState(initialConfig);
return initialConfig;
});
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved

const lastConfig = useRef(initialConfig);

const saveConfig = useCallback(
debounce((config: Config) => {
if (!isEqual(config, lastConfig.current)) {
api.updateTestProviderState(providerId, { config });
api.emit(TESTING_MODULE_CONFIG_CHANGE, { providerId, config });
updateTestProviderState(config);
lastConfig.current = config;
}
}, 500),
Expand Down
28 changes: 21 additions & 7 deletions code/addons/test/src/postinstall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,20 +327,13 @@ export default async function postInstall(options: PostinstallOptions) {
existsSync
);

const a11yAddon = info.addons.find((addon) => addon.includes(addonA11yName));

const imports = [
`import { beforeAll } from 'vitest';`,
`import { setProjectAnnotations } from '${annotationsImport}';`,
];

const projectAnnotations = [];

if (a11yAddon) {
imports.push(`import * as a11yAddonAnnotations from '@storybook/addon-a11y/preview';`);
projectAnnotations.push('a11yAddonAnnotations');
}

if (previewExists) {
imports.push(`import * as projectAnnotations from './preview';`);
projectAnnotations.push('projectAnnotations');
Expand All @@ -359,6 +352,27 @@ export default async function postInstall(options: PostinstallOptions) {
`
);

const a11yAddon = info.addons.find((addon) => addon.includes(addonA11yName));

if (a11yAddon) {
try {
logger.plain(`${step} Setting up ${addonA11yName} for @storybook/experimental-addon-test:`);
await $({
stdio: 'inherit',
})`storybook automigrate addonA11yAddonTest ${options.yes ? '--yes' : ''}`;
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
} catch (e) {
printError(
'🚨 Oh no!',
dedent`
We have detected that you have ${addonA11yName} installed but could not automatically set it up for @storybook/experimental-addon-test.

Please refer to the documentation to complete the setup manually:
${picocolors.cyan(`https://storybook.js.org/docs/writing-tests/accessibility-testing#test-addon-integration`)}
`
);
}
}

// Check for existing Vitest workspace. We can't extend it so manual setup is required.
const vitestWorkspaceFile = await findFile('vitest.workspace');
if (vitestWorkspaceFile) {
Expand Down
13 changes: 11 additions & 2 deletions code/addons/test/src/vitest-plugin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,16 +200,25 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin> => {
getInitialGlobals: () => {
const envConfig = JSON.parse(process.env.VITEST_STORYBOOK_CONFIG ?? '{}');

const isA11yEnabled = process.env.VITEST_STORYBOOK
const shouldRunA11yTests = process.env.VITEST_STORYBOOK
? (envConfig.a11y ?? false)
: true;

return {
a11y: {
manual: !isA11yEnabled,
manual: !shouldRunA11yTests,
},
};
},
getTags: () => {
const envConfig = JSON.parse(process.env.VITEST_STORYBOOK_CONFIG ?? '{}');

const shouldSetTag = process.env.VITEST_STORYBOOK
? (envConfig.a11y ?? false)
: false;
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved

return shouldSetTag ? ['a11y-test'] : [];
},
},
// if there is a test.browser config AND test.browser.screenshotFailures is not explicitly set, we set it to false
...(inputConfig_ONLY_MUTATE_WHEN_STRICTLY_NEEDED_OR_YOU_WILL_BE_FIRED.test?.browser &&
Expand Down
5 changes: 3 additions & 2 deletions code/addons/test/src/vitest-plugin/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ import { setViewport } from './viewports';
declare module '@vitest/browser/context' {
interface BrowserCommands {
getInitialGlobals: () => Promise<Record<string, any>>;
getTags: () => Promise<string[] | undefined>;
}
}

const { getInitialGlobals } = server.commands;
const { getInitialGlobals, getTags } = server.commands;

export const testStory = (
exportName: string,
Expand All @@ -28,7 +29,7 @@ export const testStory = (
const composedStory = composeStory(
story,
meta,
{ initialGlobals: (await getInitialGlobals?.()) ?? {} },
{ initialGlobals: (await getInitialGlobals?.()) ?? {}, tags: await getTags?.() },
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
undefined,
exportName
);
Expand Down
4 changes: 2 additions & 2 deletions code/core/template/stories/tags-add.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ export const Inheritance = {
tags: ['story-one', '!vitest'],
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
play: async ({ canvasElement, tags }: PlayFunctionContext<any>) => {
const canvas = within(canvasElement);
if (tags.includes('a11ytest')) {
if (tags.includes('a11y-test')) {
await expect(JSON.parse(canvas.getByTestId('pre').innerText)).toEqual({
tags: ['a11ytest', 'story-one'],
tags: ['a11y-test', 'story-one'],
});
} else {
await expect(JSON.parse(canvas.getByTestId('pre').innerText)).toEqual({
Expand Down
4 changes: 2 additions & 2 deletions code/core/template/stories/tags-config.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ export const Inheritance = {
tags: ['story-one', '!vitest'],
play: async ({ canvasElement, tags }: PlayFunctionContext<any>) => {
const canvas = within(canvasElement);
if (tags.includes('a11ytest')) {
if (tags.includes('a11y-test')) {
await expect(JSON.parse(canvas.getByTestId('pre').innerText)).toEqual({
tags: [
'dev',
'test',
'a11ytest',
'a11y-test',
'component-one',
'component-two',
'autodocs',
Expand Down
4 changes: 2 additions & 2 deletions code/core/template/stories/tags-remove.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ export const Inheritance = {
tags: ['story-one', '!vitest'],
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
play: async ({ canvasElement, tags }: PlayFunctionContext<any>) => {
const canvas = within(canvasElement);
if (tags.includes('a11ytest')) {
if (tags.includes('a11y-test')) {
await expect(JSON.parse(canvas.getByTestId('pre').innerText)).toEqual({
tags: ['dev', 'test', 'a11ytest', 'component-one', 'autodocs', 'story-one'],
tags: ['dev', 'test', 'a11y-test', 'component-one', 'autodocs', 'story-one'],
});
} else {
await expect(JSON.parse(canvas.getByTestId('pre').innerText)).toEqual({
Expand Down
Loading
Loading