From 5f21921ba3a76e9dcd0f948ef402011d692cb226 Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Wed, 20 Nov 2024 11:46:50 +0100 Subject: [PATCH 01/21] refactor: Use better sanitisation for the intersect util --- code/core/src/manager-api/lib/intersect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/core/src/manager-api/lib/intersect.ts b/code/core/src/manager-api/lib/intersect.ts index 84eae122322d..764af9ac1723 100644 --- a/code/core/src/manager-api/lib/intersect.ts +++ b/code/core/src/manager-api/lib/intersect.ts @@ -1,6 +1,6 @@ export default (a: T[], b: T[]): T[] => { // no point in intersecting if one of the input is ill-defined - if (!a || !b) { + if (!Array.isArray(a) || !Array.isArray(b)) { return []; } From 103c6b24b5bb62f19fdc42af4c7fa1add22e1ff4 Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Wed, 20 Nov 2024 12:24:39 +0100 Subject: [PATCH 02/21] Manager: Add tags property to GroupEntry objects --- code/core/src/manager-api/lib/stories.ts | 2 + .../src/manager-api/tests/stories.test.ts | 69 +++++++++++++++++++ code/core/src/types/modules/api-stories.ts | 1 + 3 files changed, 72 insertions(+) diff --git a/code/core/src/manager-api/lib/stories.ts b/code/core/src/manager-api/lib/stories.ts index eed677371ef0..aef21759907e 100644 --- a/code/core/src/manager-api/lib/stories.ts +++ b/code/core/src/manager-api/lib/stories.ts @@ -289,6 +289,8 @@ export const transformStoryIndexToStoriesHash = ( children: [childId], }), }); + // same as the merge for the component conditional branch above. + acc[id].tags = intersect(acc[id]?.tags ?? item.tags, item.tags); } }); diff --git a/code/core/src/manager-api/tests/stories.test.ts b/code/core/src/manager-api/tests/stories.test.ts index 9d3c4433fdec..ed84ec857409 100644 --- a/code/core/src/manager-api/tests/stories.test.ts +++ b/code/core/src/manager-api/tests/stories.test.ts @@ -332,6 +332,75 @@ describe('stories API', () => { tags: ['shared', 'two-specific'], }); }); + + it('intersects story/docs tags to compute tags for group entries', () => { + const moduleArgs = createMockModuleArgs({}); + const { api } = initStories(moduleArgs as unknown as ModuleArgs); + const { store } = moduleArgs; + api.setIndex({ + v: 5, + entries: { + 'a-sampleone': { + type: 'story', + id: 'a-sampleone', + title: 'A/SampleOne', + name: '1', + tags: ['shared', 'one-specific'], + importPath: './a.ts', + }, + 'a-sampletwo': { + type: 'story', + id: 'a-sampletwo', + title: 'A/SampleTwo', + name: '2', + tags: ['shared', 'two-specific'], + importPath: './a.ts', + }, + 'a-embedded-othertopic': { + type: 'docs', + id: 'a-embedded-othertopic', + title: 'A/Embedded/OtherTopic', + name: '3', + tags: ['shared', 'embedded-docs-specific', 'other'], + storiesImports: [], + importPath: './embedded/other.mdx', + }, + 'a-embedded-extras': { + type: 'docs', + id: 'a-embedded-extras', + title: 'A/Embedded/Extras', + name: '3', + tags: ['shared', 'embedded-docs-specific', 'extras'], + storiesImports: [], + importPath: './embedded/extras.mdx', + }, + }, + }); + const { index } = store.getState(); + // We need exact key ordering, even if in theory JS doesn't guarantee it + expect(Object.keys(index!)).toEqual([ + 'a', + 'a-sampleone', + 'a-sampletwo', + 'a-embedded', + 'a-embedded-othertopic', + 'a-embedded-extras', + ]); + // Acts as the root, so that the next level is a group we're testing. + expect(index!.a).toMatchObject({ + type: 'root', + id: 'a', + children: ['a-sampleone', 'a-sampletwo', 'a-embedded'], + }); + // The object of this test. + expect(index!['a-embedded']).toMatchObject({ + type: 'group', + id: 'a-embedded', + parent: 'a', + name: 'Embedded', + tags: ['shared', 'embedded-docs-specific'], + }); + }); // Stories can get out of order for a few reasons -- see reproductions on // https://github.com/storybookjs/storybook/issues/5518 it('does the right thing for out of order stories', async () => { diff --git a/code/core/src/types/modules/api-stories.ts b/code/core/src/types/modules/api-stories.ts index c53d379b7192..79ffb64d4d29 100644 --- a/code/core/src/types/modules/api-stories.ts +++ b/code/core/src/types/modules/api-stories.ts @@ -21,6 +21,7 @@ export interface API_GroupEntry extends API_BaseEntry { type: 'group'; parent?: StoryId; children: StoryId[]; + tags: Tag[]; } export interface API_ComponentEntry extends API_BaseEntry { From 247e65cde81b6a4f5707ed610f4b42cfe1727c43 Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Wed, 20 Nov 2024 14:33:28 +0100 Subject: [PATCH 03/21] refactor: Further improve sanitisation in intersect --- code/core/src/manager-api/lib/intersect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/core/src/manager-api/lib/intersect.ts b/code/core/src/manager-api/lib/intersect.ts index 764af9ac1723..5918f7c8babf 100644 --- a/code/core/src/manager-api/lib/intersect.ts +++ b/code/core/src/manager-api/lib/intersect.ts @@ -1,6 +1,6 @@ export default (a: T[], b: T[]): T[] => { // no point in intersecting if one of the input is ill-defined - if (!Array.isArray(a) || !Array.isArray(b)) { + if (!Array.isArray(a) || !Array.isArray(b) || !a.length || !b.length) { return []; } From f69b7f35ab2c52f2f057fd0cd8b552e2c6adbb43 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Fri, 22 Nov 2024 11:12:41 +0100 Subject: [PATCH 04/21] Extract test provider render method to a component --- .../test/src/components/RelativeTime.tsx | 19 +- .../test/src/components/TestProvider.tsx | 145 ++++++++++++++++ code/addons/test/src/manager.tsx | 164 +----------------- 3 files changed, 172 insertions(+), 156 deletions(-) create mode 100644 code/addons/test/src/components/TestProvider.tsx diff --git a/code/addons/test/src/components/RelativeTime.tsx b/code/addons/test/src/components/RelativeTime.tsx index d643960b06ed..f819eb2211ca 100644 --- a/code/addons/test/src/components/RelativeTime.tsx +++ b/code/addons/test/src/components/RelativeTime.tsx @@ -1,6 +1,23 @@ import { useEffect, useState } from 'react'; -import { getRelativeTimeString } from '../manager'; +function getRelativeTimeString(date: Date): string { + const delta = Math.round((date.getTime() - Date.now()) / 1000); + const cutoffs = [60, 3600, 86400, 86400 * 7, 86400 * 30, 86400 * 365, Infinity]; + const units: Intl.RelativeTimeFormatUnit[] = [ + 'second', + 'minute', + 'hour', + 'day', + 'week', + 'month', + 'year', + ]; + + const unitIndex = cutoffs.findIndex((cutoff) => cutoff > Math.abs(delta)); + const divisor = unitIndex ? cutoffs[unitIndex - 1] : 1; + const rtf = new Intl.RelativeTimeFormat('en', { numeric: 'auto' }); + return rtf.format(Math.floor(delta / divisor), units[unitIndex]); +} export const RelativeTime = ({ timestamp, testCount }: { timestamp: Date; testCount: number }) => { const [relativeTimeString, setRelativeTimeString] = useState(null); diff --git a/code/addons/test/src/components/TestProvider.tsx b/code/addons/test/src/components/TestProvider.tsx new file mode 100644 index 000000000000..b0095ae454b9 --- /dev/null +++ b/code/addons/test/src/components/TestProvider.tsx @@ -0,0 +1,145 @@ +import React, { useState } from 'react'; + +import { Button, Link as LinkComponent } from 'storybook/internal/components'; +import { + TESTING_MODULE_RUN_ALL_REQUEST, + type TestProviderConfig, +} from 'storybook/internal/core-events'; +import type { API } from 'storybook/internal/manager-api'; +import { styled } from 'storybook/internal/theming'; + +import { EyeIcon, PlayHollowIcon, StopAltHollowIcon } from '@storybook/icons'; +import type { Addon_TestProviderState } from '@storybook/types'; + +import { TEST_PROVIDER_ID } from '../constants'; +import type { Details } from '../manager'; +import { GlobalErrorModal } from './GlobalErrorModal'; +import { RelativeTime } from './RelativeTime'; + +const Info = styled.div({ + display: 'flex', + flexDirection: 'column', + marginLeft: 6, +}); + +const SidebarContextMenuTitle = styled.div<{ crashed?: boolean }>(({ crashed, theme }) => ({ + fontSize: theme.typography.size.s1, + fontWeight: crashed ? 'bold' : 'normal', + color: crashed ? theme.color.negativeText : theme.color.defaultText, +})); + +const Description = styled.div(({ theme }) => ({ + fontSize: theme.typography.size.s1, + color: theme.barTextColor, +})); + +const Actions = styled.div({ + display: 'flex', + gap: 6, +}); + +interface TestProviderProps { + api: API; + state: TestProviderConfig & Addon_TestProviderState
; +} + +export const TestProvider = ({ api, state }: TestProviderProps) => { + const [isModalOpen, setIsModalOpen] = useState(false); + + const title = state.crashed || state.failed ? 'Component tests failed' : 'Component tests'; + const errorMessage = state.error?.message; + let description: string | React.ReactNode = 'Not run'; + + if (state.running) { + description = state.progress + ? `Testing... ${state.progress.numPassedTests}/${state.progress.numTotalTests}` + : 'Starting...'; + } else if (state.failed && !errorMessage) { + description = ''; + } else if (state.crashed || (state.failed && errorMessage)) { + description = ( + <> + { + setIsModalOpen(true); + }} + > + {state.error?.name || 'View full error'} + + + ); + } else if (state.progress?.finishedAt) { + description = ( + + ); + } else if (state.watching) { + description = 'Watching for file changes'; + } + + return ( + <> + + + {title} + + {description} + + + + {state.watchable && ( + + )} + {state.runnable && ( + <> + {state.running && state.cancellable ? ( + + ) : ( + + )} + + )} + + + { + setIsModalOpen(false); + }} + onRerun={() => { + setIsModalOpen(false); + api.getChannel().emit(TESTING_MODULE_RUN_ALL_REQUEST, { providerId: TEST_PROVIDER_ID }); + }} + /> + + ); +}; diff --git a/code/addons/test/src/manager.tsx b/code/addons/test/src/manager.tsx index 9b560cdd8cee..92b67dbd962a 100644 --- a/code/addons/test/src/manager.tsx +++ b/code/addons/test/src/manager.tsx @@ -1,10 +1,8 @@ -import React, { useState } from 'react'; +import React from 'react'; -import { AddonPanel, Button, Link as LinkComponent } from 'storybook/internal/components'; -import { TESTING_MODULE_RUN_ALL_REQUEST } from 'storybook/internal/core-events'; +import { AddonPanel } from 'storybook/internal/components'; import type { Combo } from 'storybook/internal/manager-api'; import { Consumer, addons, types } from 'storybook/internal/manager-api'; -import { styled } from 'storybook/internal/theming'; import { type API_StatusObject, type API_StatusValue, @@ -12,63 +10,23 @@ import { Addon_TypesEnum, } from 'storybook/internal/types'; -import { EyeIcon, PlayHollowIcon, StopAltHollowIcon } from '@storybook/icons'; - import { ContextMenuItem } from './components/ContextMenuItem'; -import { GlobalErrorModal } from './components/GlobalErrorModal'; import { Panel } from './components/Panel'; import { PanelTitle } from './components/PanelTitle'; -import { RelativeTime } from './components/RelativeTime'; +import { TestProvider } from './components/TestProvider'; import { ADDON_ID, PANEL_ID, TEST_PROVIDER_ID } from './constants'; import type { TestResult } from './node/reporter'; +export type Details = { + testResults: TestResult[]; +}; + const statusMap: Record = { failed: 'error', passed: 'success', pending: 'pending', }; -export function getRelativeTimeString(date: Date): string { - const delta = Math.round((date.getTime() - Date.now()) / 1000); - const cutoffs = [60, 3600, 86400, 86400 * 7, 86400 * 30, 86400 * 365, Infinity]; - const units: Intl.RelativeTimeFormatUnit[] = [ - 'second', - 'minute', - 'hour', - 'day', - 'week', - 'month', - 'year', - ]; - - const unitIndex = cutoffs.findIndex((cutoff) => cutoff > Math.abs(delta)); - const divisor = unitIndex ? cutoffs[unitIndex - 1] : 1; - const rtf = new Intl.RelativeTimeFormat('en', { numeric: 'auto' }); - return rtf.format(Math.floor(delta / divisor), units[unitIndex]); -} - -const Info = styled.div({ - display: 'flex', - flexDirection: 'column', - marginLeft: 6, -}); - -const SidebarContextMenuTitle = styled.div<{ crashed?: boolean }>(({ crashed, theme }) => ({ - fontSize: theme.typography.size.s1, - fontWeight: crashed ? 'bold' : 'normal', - color: crashed ? theme.color.negativeText : theme.color.defaultText, -})); - -const Description = styled.div(({ theme }) => ({ - fontSize: theme.typography.size.s1, - color: theme.barTextColor, -})); - -const Actions = styled.div({ - display: 'flex', - gap: 6, -}); - addons.register(ADDON_ID, (api) => { const storybookBuilder = (globalThis as any).STORYBOOK_BUILDER || ''; if (storybookBuilder.includes('vite')) { @@ -82,6 +40,7 @@ addons.register(ADDON_ID, (api) => { runnable: true, watchable: true, name: 'Component tests', + render: (state) => , sidebarContextMenu: ({ context, state }, { ListItem }) => { if (context.type === 'docs') { @@ -94,109 +53,6 @@ addons.register(ADDON_ID, (api) => { return ; }, - render: (state) => { - const [isModalOpen, setIsModalOpen] = useState(false); - - const title = state.crashed || state.failed ? 'Component tests failed' : 'Component tests'; - const errorMessage = state.error?.message; - let description: string | React.ReactNode = 'Not run'; - - if (state.running) { - description = state.progress - ? `Testing... ${state.progress.numPassedTests}/${state.progress.numTotalTests}` - : 'Starting...'; - } else if (state.failed && !errorMessage) { - description = ''; - } else if (state.crashed || (state.failed && errorMessage)) { - description = ( - <> - { - setIsModalOpen(true); - }} - > - {state.error?.name || 'View full error'} - - - ); - } else if (state.progress?.finishedAt) { - description = ( - - ); - } else if (state.watching) { - description = 'Watching for file changes'; - } - - return ( - <> - - - {title} - - {description} - - - - {state.watchable && ( - - )} - {state.runnable && ( - <> - {state.running && state.cancellable ? ( - - ) : ( - - )} - - )} - - - { - setIsModalOpen(false); - }} - onRerun={() => { - setIsModalOpen(false); - api - .getChannel() - .emit(TESTING_MODULE_RUN_ALL_REQUEST, { providerId: TEST_PROVIDER_ID }); - }} - /> - - ); - }, - mapStatusUpdate: (state) => Object.fromEntries( (state.details.testResults || []).flatMap((testResult) => @@ -221,9 +77,7 @@ addons.register(ADDON_ID, (api) => { .filter(Boolean) ) ), - } as Addon_TestProviderType<{ - testResults: TestResult[]; - }>); + } as Addon_TestProviderType
); } const filter = ({ state }: Combo) => { From 049cec156914bd9c1983339d9bad7b461fe77189 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Fri, 22 Nov 2024 15:03:00 +0100 Subject: [PATCH 05/21] Draw horizontal line between each test provider --- code/addons/test/src/components/TestProvider.tsx | 2 +- .../src/manager/components/sidebar/TestingModule.tsx | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/code/addons/test/src/components/TestProvider.tsx b/code/addons/test/src/components/TestProvider.tsx index b0095ae454b9..3588a6a58ef8 100644 --- a/code/addons/test/src/components/TestProvider.tsx +++ b/code/addons/test/src/components/TestProvider.tsx @@ -35,7 +35,7 @@ const Description = styled.div(({ theme }) => ({ const Actions = styled.div({ display: 'flex', - gap: 6, + gap: 2, }); interface TestProviderProps { diff --git a/code/core/src/manager/components/sidebar/TestingModule.tsx b/code/core/src/manager/components/sidebar/TestingModule.tsx index a28a338857b8..0385cdcd70ed 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.tsx @@ -78,10 +78,8 @@ const Collapsible = styled.div(({ theme }) => ({ })); const Content = styled.div({ - padding: '12px 6px', display: 'flex', flexDirection: 'column', - gap: '12px', }); const Bar = styled.div<{ onClick?: (e: SyntheticEvent) => void }>(({ onClick }) => ({ @@ -138,11 +136,16 @@ const StatusButton = styled(Button)<{ status: 'negative' | 'warning' }>( }) ); -const TestProvider = styled.div({ +const TestProvider = styled.div(({ theme }) => ({ display: 'flex', justifyContent: 'space-between', + padding: '12px 6px', gap: 6, -}); + + '&:not(:last-child)': { + boxShadow: `inset 0 -1px 0 ${theme.appBorderColor}`, + }, +})); interface TestingModuleProps { testProviders: TestProviders[keyof TestProviders][]; From 9efed90d22a6a404ff3fc78750e1fd72fb9ebdfd Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Mon, 25 Nov 2024 17:07:42 +0100 Subject: [PATCH 06/21] Glow testing module when changing settings --- .../sidebar/TestingModule.stories.tsx | 44 ++++++--- .../components/sidebar/TestingModule.tsx | 89 +++++++++++-------- 2 files changed, 84 insertions(+), 49 deletions(-) diff --git a/code/core/src/manager/components/sidebar/TestingModule.stories.tsx b/code/core/src/manager/components/sidebar/TestingModule.stories.tsx index a13e4ccbad5d..fdbfa3db37d8 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.stories.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.stories.tsx @@ -1,5 +1,6 @@ import React from 'react'; +import { styled } from '@storybook/core/theming'; import { Addon_TypesEnum } from '@storybook/core/types'; import type { Meta, StoryObj } from '@storybook/react'; import { fn, userEvent } from '@storybook/test'; @@ -9,6 +10,11 @@ import { ManagerContext } from '@storybook/core/manager-api'; import { TestingModule } from './TestingModule'; +const TestProvider = styled.div({ + padding: 8, + fontSize: 12, +}); + const baseState = { details: {}, cancellable: false, @@ -25,11 +31,11 @@ const testProviders: TestProviders[keyof TestProviders][] = [ id: 'component-tests', name: 'Component tests', render: () => ( - <> + Component tests
Ran 2 seconds ago - +
), runnable: true, watchable: true, @@ -40,11 +46,11 @@ const testProviders: TestProviders[keyof TestProviders][] = [ id: 'visual-tests', name: 'Visual tests', render: () => ( - <> + Visual tests
Not run - +
), runnable: true, ...baseState, @@ -54,11 +60,11 @@ const testProviders: TestProviders[keyof TestProviders][] = [ id: 'linting', name: 'Linting', render: () => ( - <> + Linting
Watching for changes - +
), ...baseState, watching: true, @@ -67,6 +73,8 @@ const testProviders: TestProviders[keyof TestProviders][] = [ const managerContext: any = { api: { + on: fn().mockName('api::on'), + off: fn().mockName('api::off'), runTestProvider: fn().mockName('api::runTestProvider'), cancelTestProvider: fn().mockName('api::cancelTestProvider'), updateTestProviderState: fn().mockName('api::updateTestProviderState'), @@ -104,9 +112,9 @@ type Story = StoryObj; export const Default: Story = {}; -export const Collapsed: Story = { +export const Expanded: Story = { play: async ({ canvas }) => { - const button = await canvas.findByRole('button', { name: /Collapse/ }); + const button = await canvas.findByRole('button', { name: /Expand/ }); await userEvent.click(button); }, }; @@ -116,6 +124,7 @@ export const Statuses: Story = { errorCount: 14, warningCount: 42, }, + play: Expanded.play, }; export const ErrorsActive: Story = { @@ -123,6 +132,7 @@ export const ErrorsActive: Story = { ...Statuses.args, errorsActive: true, }, + play: Expanded.play, }; export const WarningsActive: Story = { @@ -130,6 +140,7 @@ export const WarningsActive: Story = { ...Statuses.args, warningsActive: true, }, + play: Expanded.play, }; export const BothActive: Story = { @@ -138,28 +149,31 @@ export const BothActive: Story = { errorsActive: true, warningsActive: true, }, + play: Expanded.play, }; export const CollapsedStatuses: Story = { args: Statuses.args, - play: Collapsed.play, + play: Expanded.play, }; export const Running: Story = { args: { testProviders: [{ ...testProviders[0], running: true }, ...testProviders.slice(1)], }, + play: Expanded.play, }; export const RunningAll: Story = { args: { testProviders: testProviders.map((tp) => ({ ...tp, running: !!tp.runnable })), }, + play: Expanded.play, }; export const CollapsedRunning: Story = { args: RunningAll.args, - play: Collapsed.play, + play: Expanded.play, }; export const Cancellable: Story = { @@ -169,6 +183,7 @@ export const Cancellable: Story = { ...testProviders.slice(1), ], }, + play: Expanded.play, }; export const Cancelling: Story = { @@ -178,12 +193,14 @@ export const Cancelling: Story = { ...testProviders.slice(1), ], }, + play: Expanded.play, }; export const Watching: Story = { args: { testProviders: [{ ...testProviders[0], watching: true }, ...testProviders.slice(1)], }, + play: Expanded.play, }; export const Failing: Story = { @@ -193,12 +210,14 @@ export const Failing: Story = { ...testProviders.slice(1), ], }, + play: Expanded.play, }; export const Failed: Story = { args: { testProviders: [{ ...testProviders[0], failed: true }, ...testProviders.slice(1)], }, + play: Expanded.play, }; export const Crashed: Story = { @@ -207,15 +226,16 @@ export const Crashed: Story = { { ...testProviders[0], render: () => ( - <> + Component tests didn't complete
Problems! - +
), crashed: true, }, ...testProviders.slice(1), ], }, + play: Expanded.play, }; diff --git a/code/core/src/manager/components/sidebar/TestingModule.tsx b/code/core/src/manager/components/sidebar/TestingModule.tsx index c07f37e8b027..6724f52d640f 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.tsx @@ -1,11 +1,11 @@ -import React, { Fragment, type SyntheticEvent, useEffect, useRef, useState } from 'react'; +import React, { type SyntheticEvent, useEffect, useRef, useState } from 'react'; import { Button, TooltipNote } from '@storybook/core/components'; import { WithTooltip } from '@storybook/core/components'; import { keyframes, styled } from '@storybook/core/theming'; import { ChevronSmallUpIcon, PlayAllHollowIcon } from '@storybook/icons'; -import type { TestProviders } from '@storybook/core/core-events'; +import { TESTING_MODULE_CONFIG_CHANGE, type TestProviders } from '@storybook/core/core-events'; import { useStorybookApi } from '@storybook/core/manager-api'; import { LegacyRender } from './LegacyRender'; @@ -22,42 +22,42 @@ const spin = keyframes({ '100%': { transform: 'rotate(360deg)' }, }); -const Outline = styled.div<{ crashed: boolean; failed: boolean; running: boolean }>( - ({ crashed, running, theme, failed }) => ({ - position: 'relative', - lineHeight: '20px', - width: '100%', - padding: 1, - overflow: 'hidden', - background: `var(--sb-sidebar-bottom-card-background, ${theme.background.content})`, - borderRadius: - `var(--sb-sidebar-bottom-card-border-radius, ${theme.appBorderRadius + 1}px)` as any, - boxShadow: `inset 0 0 0 1px ${crashed && !running ? theme.color.negative : theme.appBorderColor}, var(--sb-sidebar-bottom-card-box-shadow, 0 1px 2px 0 rgba(0, 0, 0, 0.05), 0px -5px 20px 10px ${theme.background.app})`, - transitionProperty: - 'color, background-color, border-color, text-decoration-color, fill, stroke', - transitionTimingFunction: 'cubic-bezier(0.4, 0, 0.2, 1)', - transitionDuration: '0.15s', +const Outline = styled.div<{ + crashed: boolean; + failed: boolean; + running: boolean; + updated: boolean; +}>(({ crashed, failed, running, theme, updated }) => ({ + position: 'relative', + lineHeight: '20px', + width: '100%', + padding: 1, + overflow: 'hidden', + backgroundColor: `var(--sb-sidebar-bottom-card-background, ${theme.background.content})`, + borderRadius: + `var(--sb-sidebar-bottom-card-border-radius, ${theme.appBorderRadius + 1}px)` as any, + boxShadow: `inset 0 0 0 1px ${crashed && !running ? theme.color.negative : updated ? theme.color.positive : theme.appBorderColor}, var(--sb-sidebar-bottom-card-box-shadow, 0 1px 2px 0 rgba(0, 0, 0, 0.05), 0px -5px 20px 10px ${theme.background.app})`, + transition: 'box-shadow 1s', - '&:after': { - content: '""', - display: running ? 'block' : 'none', - position: 'absolute', - left: '50%', - top: '50%', - marginLeft: 'calc(max(100vw, 100vh) * -0.5)', - marginTop: 'calc(max(100vw, 100vh) * -0.5)', - height: 'max(100vw, 100vh)', - width: 'max(100vw, 100vh)', - animation: `${spin} 3s linear infinite`, - background: failed - ? // Hardcoded colors to prevent themes from messing with them (orange+gold, secondary+seafoam) - `conic-gradient(transparent 90deg, #FC521F 150deg, #FFAE00 210deg, transparent 270deg)` - : `conic-gradient(transparent 90deg, #029CFD 150deg, #37D5D3 210deg, transparent 270deg)`, - opacity: 1, - willChange: 'auto', - }, - }) -); + '&:after': { + content: '""', + display: running ? 'block' : 'none', + position: 'absolute', + left: '50%', + top: '50%', + marginLeft: 'calc(max(100vw, 100vh) * -0.5)', + marginTop: 'calc(max(100vw, 100vh) * -0.5)', + height: 'max(100vw, 100vh)', + width: 'max(100vw, 100vh)', + animation: `${spin} 3s linear infinite`, + background: failed + ? // Hardcoded colors to prevent themes from messing with them (orange+gold, secondary+seafoam) + `conic-gradient(transparent 90deg, #FC521F 150deg, #FFAE00 210deg, transparent 270deg)` + : `conic-gradient(transparent 90deg, #029CFD 150deg, #37D5D3 210deg, transparent 270deg)`, + opacity: 1, + willChange: 'auto', + }, +})); const Card = styled.div(({ theme }) => ({ position: 'relative', @@ -165,6 +165,7 @@ export const TestingModule = ({ }: TestingModuleProps) => { const api = useStorybookApi(); const contentRef = useRef(null); + const [updated, setUpdated] = useState(false); const [animating, setAnimating] = useState(false); const [collapsed, setCollapsed] = useState(true); const [maxHeight, setMaxHeight] = useState(DEFAULT_HEIGHT); @@ -173,6 +174,19 @@ export const TestingModule = ({ setMaxHeight(contentRef.current?.offsetHeight || DEFAULT_HEIGHT); }, []); + useEffect(() => { + let timeout: NodeJS.Timeout; + const handler = () => { + setUpdated(true); + timeout = setTimeout(setUpdated, 1000, false); + }; + api.on(TESTING_MODULE_CONFIG_CHANGE, handler); + return () => { + api.off(TESTING_MODULE_CONFIG_CHANGE, handler); + clearTimeout(timeout); + }; + }, [api]); + const toggleCollapsed = () => { setAnimating(true); setMaxHeight(contentRef.current?.offsetHeight || DEFAULT_HEIGHT); @@ -191,6 +205,7 @@ export const TestingModule = ({ running={running} crashed={crashed} failed={failed || errorCount > 0} + updated={updated} > Date: Tue, 26 Nov 2024 06:36:45 +0100 Subject: [PATCH 07/21] Manager: Generalise tag intersection to root entries and ensure all entries have tags --- code/core/src/manager-api/lib/stories.ts | 22 ++++++++++++++----- .../src/manager-api/tests/stories.test.ts | 13 ++++++++++- code/core/src/types/modules/api-stories.ts | 5 +---- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/code/core/src/manager-api/lib/stories.ts b/code/core/src/manager-api/lib/stories.ts index aef21759907e..59b59f070a10 100644 --- a/code/core/src/manager-api/lib/stories.ts +++ b/code/core/src/manager-api/lib/stories.ts @@ -1,4 +1,5 @@ import type { + API_BaseEntry, API_ComponentEntry, API_DocsEntry, API_GroupEntry, @@ -16,6 +17,7 @@ import type { StoryId, StoryIndexV2, StoryIndexV3, + Tag, } from '@storybook/core/types'; import { sanitize } from '@storybook/csf'; @@ -248,6 +250,7 @@ export const transformStoryIndexToStoriesHash = ( type: 'root', id, name: names[idx], + tags: [], depth: idx, renderLabel, startCollapsed: collapsedRoots.includes(id), @@ -267,6 +270,7 @@ export const transformStoryIndexToStoriesHash = ( type: 'component', id, name: names[idx], + tags: [], parent: paths[idx - 1], depth: idx, renderLabel, @@ -274,14 +278,12 @@ export const transformStoryIndexToStoriesHash = ( children: [childId], }), }); - // merge computes a union of arrays but we want an intersection on this - // specific array property, so it's easier to add it after the merge. - acc[id].tags = intersect(acc[id]?.tags ?? item.tags, item.tags); } else { acc[id] = merge((acc[id] || {}) as API_GroupEntry, { type: 'group', id, name: names[idx], + tags: [], parent: paths[idx - 1], depth: idx, renderLabel, @@ -289,14 +291,13 @@ export const transformStoryIndexToStoriesHash = ( children: [childId], }), }); - // same as the merge for the component conditional branch above. - acc[id].tags = intersect(acc[id]?.tags ?? item.tags, item.tags); } }); // Finally add an entry for the docs/story itself acc[item.id] = { type: 'story', + tags: [], ...item, depth: paths.length, parent: paths[paths.length - 1], @@ -315,9 +316,18 @@ export const transformStoryIndexToStoriesHash = ( } acc[item.id] = item; - // Ensure we add the children depth-first *before* inserting any other entries + // Ensure we add the children depth-first *before* inserting any other entries, + // and compute tags from the children put in the accumulator afterwards, once + // they're all known and we can compute a sound intersection. if (item.type === 'root' || item.type === 'group' || item.type === 'component') { item.children.forEach((childId: any) => addItem(acc, storiesHashOutOfOrder[childId])); + + item.tags = item.children.reduce((currentTags: Tag[] | null, childId: any): Tag[] => { + const child = acc[childId]; + + // On the first child, we have nothing to intersect against so we use it as a source of data. + return currentTags === null ? child.tags : intersect(currentTags, child.tags); + }, null); } return acc; } diff --git a/code/core/src/manager-api/tests/stories.test.ts b/code/core/src/manager-api/tests/stories.test.ts index ed84ec857409..b652c87cd7a1 100644 --- a/code/core/src/manager-api/tests/stories.test.ts +++ b/code/core/src/manager-api/tests/stories.test.ts @@ -162,6 +162,7 @@ describe('stories API', () => { expect(index!['design-system']).toMatchObject({ type: 'root', name: 'Design System', // root name originates from `kind`, so it gets trimmed + tags: [], }); expect(index!['design-system-some-component']).toMatchObject({ type: 'component', @@ -186,6 +187,7 @@ describe('stories API', () => { title: 'Root/First', name: 'Story 1', importPath: './path/to/root/first.ts', + tags: [], }, ...mockEntries, }, @@ -207,6 +209,7 @@ describe('stories API', () => { type: 'root', id: 'root', children: ['root-first'], + tags: [], }); }); it('sets roots when showRoots = true', () => { @@ -222,6 +225,7 @@ describe('stories API', () => { id: 'a-b--1', title: 'a/b', name: '1', + tags: [], importPath: './a/b.ts', }, }, @@ -233,6 +237,7 @@ describe('stories API', () => { type: 'root', id: 'a', children: ['a-b'], + tags: [], }); expect(index!['a-b']).toMatchObject({ type: 'component', @@ -333,7 +338,7 @@ describe('stories API', () => { }); }); - it('intersects story/docs tags to compute tags for group entries', () => { + it('intersects story/docs tags to compute tags for root and group entries', () => { const moduleArgs = createMockModuleArgs({}); const { api } = initStories(moduleArgs as unknown as ModuleArgs); const { store } = moduleArgs; @@ -391,6 +396,7 @@ describe('stories API', () => { type: 'root', id: 'a', children: ['a-sampleone', 'a-sampletwo', 'a-embedded'], + tags: ['shared'], }); // The object of this test. expect(index!['a-embedded']).toMatchObject({ @@ -1584,6 +1590,7 @@ describe('stories API', () => { "parent": "a", "prepared": false, "renderLabel": undefined, + "tags": [], "title": "a", "type": "story", }, @@ -1595,6 +1602,7 @@ describe('stories API', () => { "parent": "a", "prepared": false, "renderLabel": undefined, + "tags": [], "title": "a", "type": "story", }, @@ -1650,6 +1658,7 @@ describe('stories API', () => { "parent": "a", "prepared": false, "renderLabel": undefined, + "tags": [], "title": "a", "type": "story", }, @@ -1692,6 +1701,7 @@ describe('stories API', () => { "parent": "a", "prepared": false, "renderLabel": undefined, + "tags": [], "title": "a", "type": "story", }, @@ -1703,6 +1713,7 @@ describe('stories API', () => { "parent": "a", "prepared": false, "renderLabel": undefined, + "tags": [], "title": "a", "type": "story", }, diff --git a/code/core/src/types/modules/api-stories.ts b/code/core/src/types/modules/api-stories.ts index 79ffb64d4d29..5f7b8dfe864b 100644 --- a/code/core/src/types/modules/api-stories.ts +++ b/code/core/src/types/modules/api-stories.ts @@ -7,6 +7,7 @@ export interface API_BaseEntry { id: StoryId; depth: number; name: string; + tags: Tag[]; refId?: string; renderLabel?: (item: API_BaseEntry, api: any) => any; } @@ -21,14 +22,12 @@ export interface API_GroupEntry extends API_BaseEntry { type: 'group'; parent?: StoryId; children: StoryId[]; - tags: Tag[]; } export interface API_ComponentEntry extends API_BaseEntry { type: 'component'; parent?: StoryId; children: StoryId[]; - tags: Tag[]; } export interface API_DocsEntry extends API_BaseEntry { @@ -36,7 +35,6 @@ export interface API_DocsEntry extends API_BaseEntry { parent: StoryId; title: ComponentTitle; importPath: Path; - tags: Tag[]; prepared: boolean; parameters?: { [parameterName: string]: any; @@ -48,7 +46,6 @@ export interface API_StoryEntry extends API_BaseEntry { parent: StoryId; title: ComponentTitle; importPath: Path; - tags: Tag[]; prepared: boolean; parameters?: { [parameterName: string]: any; From a71d9012c4ee016231ea44382d8bc5121e5afd68 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 09:34:55 +0100 Subject: [PATCH 08/21] Add story for updated state --- .../sidebar/TestingModule.stories.tsx | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/code/core/src/manager/components/sidebar/TestingModule.stories.tsx b/code/core/src/manager/components/sidebar/TestingModule.stories.tsx index fdbfa3db37d8..625396064f09 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.stories.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.stories.tsx @@ -1,12 +1,13 @@ import React from 'react'; +import type { Listener } from '@storybook/core/channels'; import { styled } from '@storybook/core/theming'; import { Addon_TypesEnum } from '@storybook/core/types'; import type { Meta, StoryObj } from '@storybook/react'; -import { fn, userEvent } from '@storybook/test'; +import { fireEvent, fn } from '@storybook/test'; -import type { TestProviders } from '@storybook/core/core-events'; -import { ManagerContext } from '@storybook/core/manager-api'; +import { TESTING_MODULE_CONFIG_CHANGE, type TestProviders } from '@storybook/core/core-events'; +import { ManagerContext, mockChannel } from '@storybook/core/manager-api'; import { TestingModule } from './TestingModule'; @@ -71,10 +72,17 @@ const testProviders: TestProviders[keyof TestProviders][] = [ }, ]; +let triggerUpdate: () => void; +const channel = mockChannel(); const managerContext: any = { api: { - on: fn().mockName('api::on'), - off: fn().mockName('api::off'), + on: (eventName: string, listener: Listener) => { + if (eventName === TESTING_MODULE_CONFIG_CHANGE) { + triggerUpdate = listener; + } + return channel.on(eventName, listener); + }, + off: (eventName: string, listener: Listener) => channel.off(eventName, listener), runTestProvider: fn().mockName('api::runTestProvider'), cancelTestProvider: fn().mockName('api::cancelTestProvider'), updateTestProviderState: fn().mockName('api::updateTestProviderState'), @@ -115,7 +123,8 @@ export const Default: Story = {}; export const Expanded: Story = { play: async ({ canvas }) => { const button = await canvas.findByRole('button', { name: /Expand/ }); - await userEvent.click(button); + await fireEvent.click(button); + await new Promise((resolve) => setTimeout(resolve, 500)); }, }; @@ -239,3 +248,11 @@ export const Crashed: Story = { }, play: Expanded.play, }; + +export const Updated: Story = { + args: {}, + play: async (context) => { + await Expanded.play!(context); + triggerUpdate?.(); + }, +}; From dba14dcdc90d5864748881d7057a0b181ab08aca Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 09:45:24 +0100 Subject: [PATCH 09/21] Fix cursor on checkbox --- code/addons/test/src/components/TestProviderRender.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index cf6f4c81e4c8..b6e079dc58b0 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -60,6 +60,9 @@ const Extras = styled.div({ const Checkbox = styled.input({ margin: 0, + '&:enabled': { + cursor: 'pointer', + }, }); export const TestProviderRender: FC<{ From 40cce2bf8de84fc6626ebf2f2507b46d22239a67 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 10:10:36 +0100 Subject: [PATCH 10/21] Refactor useConfig to debounce update synchronization --- .../src/components/TestProviderRender.tsx | 51 +++++++++---------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index b6e079dc58b0..e2ff8c8cbf20 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -1,11 +1,10 @@ -import React, { type FC, Fragment, useCallback, useRef, useState } from 'react'; +import React, { type FC, useCallback, useRef, useState } from 'react'; import { Button, ListItem } from 'storybook/internal/components'; import { TESTING_MODULE_CONFIG_CHANGE, type TestProviderConfig, type TestProviderState, - type TestingModuleConfigChangePayload, } from 'storybook/internal/core-events'; import type { API } from 'storybook/internal/manager-api'; import { styled, useTheme } from 'storybook/internal/theming'; @@ -20,6 +19,8 @@ import { StopAltHollowIcon, } from '@storybook/icons'; +import { debounce } from 'es-toolkit/compat'; + import { type Config, type Details, TEST_PROVIDER_ID } from '../constants'; import { Description } from './Description'; import { GlobalErrorModal } from './GlobalErrorModal'; @@ -76,10 +77,10 @@ export const TestProviderRender: FC<{ const title = state.crashed || state.failed ? 'Local tests failed' : 'Run local tests'; const errorMessage = state.error?.message; - const [config, changeConfig] = useConfig( + const [config, updateConfig] = useConfig( + api, state.id, - state.config || { a11y: false, coverage: false }, - api + state.config || { a11y: false, coverage: false } ); return ( @@ -157,7 +158,7 @@ export const TestProviderRender: FC<{ changeConfig({ coverage: !config.coverage })} + onChange={() => updateConfig({ coverage: !config.coverage })} /> } /> @@ -169,7 +170,7 @@ export const TestProviderRender: FC<{ changeConfig({ a11y: !config.a11y })} + onChange={() => updateConfig({ a11y: !config.a11y })} /> } /> @@ -201,29 +202,27 @@ export const TestProviderRender: FC<{ ); }; -function useConfig(id: string, config: Config, api: API) { - const data = useRef(config); - data.current = config || { - a11y: false, - coverage: false, - }; +function useConfig(api: API, providerId: string, initialConfig: Config) { + const [currentConfig, setConfig] = useState(initialConfig); + + const saveConfig = useCallback( + debounce((config: Config) => { + api.updateTestProviderState(providerId, { config }); + api.emit(TESTING_MODULE_CONFIG_CHANGE, { providerId, config }); + }, 500), + [api, providerId] + ); - const changeConfig = useCallback( + const updateConfig = useCallback( (update: Partial) => { - const newConfig = { - ...data.current, - ...update, - }; - api.updateTestProviderState(id, { - config: newConfig, + setConfig((value) => { + const updated = { ...value, ...update }; + saveConfig(updated); + return updated; }); - api.emit(TESTING_MODULE_CONFIG_CHANGE, { - providerId: id, - config: newConfig, - } as TestingModuleConfigChangePayload); }, - [api, id] + [saveConfig] ); - return [data.current, changeConfig] as const; + return [currentConfig, updateConfig] as const; } From 8789ac40081cb8909283e238fd74a98b631c374c Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 10:26:51 +0100 Subject: [PATCH 11/21] Fix story --- .../src/manager/components/sidebar/SidebarBottom.stories.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/code/core/src/manager/components/sidebar/SidebarBottom.stories.tsx b/code/core/src/manager/components/sidebar/SidebarBottom.stories.tsx index 898b02f28945..22cf6e68ef96 100644 --- a/code/core/src/manager/components/sidebar/SidebarBottom.stories.tsx +++ b/code/core/src/manager/components/sidebar/SidebarBottom.stories.tsx @@ -155,7 +155,9 @@ export const DynamicHeight: StoryObj = { play: async ({ canvasElement }) => { const screen = await within(canvasElement); - const toggleButton = await screen.getByLabelText('Collapse testing module'); + const toggleButton = await screen.getByLabelText(/Expand/); + await userEvent.click(toggleButton); + const content = await screen.findByText('CUSTOM CONTENT WITH DYNAMIC HEIGHT'); const collapse = await screen.getByTestId('collapse'); From cd203fa05a3c66c55bcff857c7886eb32425f81b Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 10:30:39 +0100 Subject: [PATCH 12/21] Use proper type for timeouts --- code/addons/onboarding/src/features/GuidedTour/GuidedTour.tsx | 2 +- code/core/src/manager/components/sidebar/TestingModule.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/code/addons/onboarding/src/features/GuidedTour/GuidedTour.tsx b/code/addons/onboarding/src/features/GuidedTour/GuidedTour.tsx index 56bbb75e7d50..8ba6930efb5e 100644 --- a/code/addons/onboarding/src/features/GuidedTour/GuidedTour.tsx +++ b/code/addons/onboarding/src/features/GuidedTour/GuidedTour.tsx @@ -23,7 +23,7 @@ export function GuidedTour({ const theme = useTheme(); useEffect(() => { - let timeout: NodeJS.Timeout; + let timeout: ReturnType; setStepIndex((current) => { const index = steps.findIndex(({ key }) => key === step); diff --git a/code/core/src/manager/components/sidebar/TestingModule.tsx b/code/core/src/manager/components/sidebar/TestingModule.tsx index 09256e100dd4..4dcb743934e7 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.tsx @@ -191,7 +191,7 @@ export const TestingModule = ({ }, [isCollapsed]); useEffect(() => { - let timeout: NodeJS.Timeout; + let timeout: ReturnType; const handler = () => { setUpdated(true); timeout = setTimeout(setUpdated, 1000, false); From c4071cf10935b7c1c93849c8a8d779699a69a7a7 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 11:06:33 +0100 Subject: [PATCH 13/21] Show 'Settings updated' when changing config, and don't update config if it's equal to the previous value --- .../test/src/components/Description.tsx | 21 ++++++++++++++++--- .../src/components/TestProviderRender.tsx | 9 ++++++-- .../modules/experimental_testmodule.ts | 1 - 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/code/addons/test/src/components/Description.tsx b/code/addons/test/src/components/Description.tsx index aa7365a007e8..3f1257972666 100644 --- a/code/addons/test/src/components/Description.tsx +++ b/code/addons/test/src/components/Description.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useEffect } from 'react'; import { Link as LinkComponent } from 'storybook/internal/components'; import { type TestProviderConfig, type TestProviderState } from 'storybook/internal/core-events'; @@ -11,6 +11,10 @@ export const DescriptionStyle = styled.div(({ theme }) => ({ color: theme.barTextColor, })); +const PositiveText = styled.span(({ theme }) => ({ + color: theme.color.positiveText, +})); + export function Description({ errorMessage, setIsModalOpen, @@ -20,9 +24,20 @@ export function Description({ errorMessage: string; setIsModalOpen: React.Dispatch>; }) { - let description: string | React.ReactNode = 'Not run'; + const [isUpdated, setUpdated] = React.useState(false); - if (state.running) { + useEffect(() => { + setUpdated(true); + const timeout = setTimeout(setUpdated, 2000, false); + return () => { + clearTimeout(timeout); + }; + }, [state.config]); + + let description: string | React.ReactNode = 'Not run'; + if (isUpdated) { + description = Settings updated; + } else if (state.running) { description = state.progress ? `Testing... ${state.progress.numPassedTests}/${state.progress.numTotalTests}` : 'Starting...'; diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index e2ff8c8cbf20..7f085ac2e435 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -19,6 +19,7 @@ import { StopAltHollowIcon, } from '@storybook/icons'; +import { isEqual } from 'es-toolkit'; import { debounce } from 'es-toolkit/compat'; import { type Config, type Details, TEST_PROVIDER_ID } from '../constants'; @@ -204,11 +205,15 @@ export const TestProviderRender: FC<{ function useConfig(api: API, providerId: string, initialConfig: Config) { const [currentConfig, setConfig] = useState(initialConfig); + const lastConfig = useRef(initialConfig); const saveConfig = useCallback( debounce((config: Config) => { - api.updateTestProviderState(providerId, { config }); - api.emit(TESTING_MODULE_CONFIG_CHANGE, { providerId, config }); + if (!isEqual(config, lastConfig.current)) { + api.updateTestProviderState(providerId, { config }); + api.emit(TESTING_MODULE_CONFIG_CHANGE, { providerId, config }); + lastConfig.current = config; + } }, 500), [api, providerId] ); diff --git a/code/core/src/manager-api/modules/experimental_testmodule.ts b/code/core/src/manager-api/modules/experimental_testmodule.ts index a6a0eff1d376..52cf4f5042cb 100644 --- a/code/core/src/manager-api/modules/experimental_testmodule.ts +++ b/code/core/src/manager-api/modules/experimental_testmodule.ts @@ -20,7 +20,6 @@ export type SubState = { }; const initialTestProviderState: TestProviderState = { - config: {} as { [key: string]: any }, details: {} as { [key: string]: any }, cancellable: false, cancelling: false, From 84b6a1d7d868697b9122737f11e44f82ff69fa0f Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 11:34:29 +0100 Subject: [PATCH 14/21] Fix stories --- .../components/TestProviderRender.stories.tsx | 2 +- .../sidebar/TestingModule.stories.tsx | 28 ++++--------------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.stories.tsx b/code/addons/test/src/components/TestProviderRender.stories.tsx index 522060b47070..13a9e9bd3f35 100644 --- a/code/addons/test/src/components/TestProviderRender.stories.tsx +++ b/code/addons/test/src/components/TestProviderRender.stories.tsx @@ -158,6 +158,6 @@ export const EnableEditing: Story = { play: async ({ canvasElement }) => { const screen = within(canvasElement); - screen.getByLabelText('Edit').click(); + screen.getByLabelText(/Open settings/).click(); }, }; diff --git a/code/core/src/manager/components/sidebar/TestingModule.stories.tsx b/code/core/src/manager/components/sidebar/TestingModule.stories.tsx index 3d16a655015a..66fd53c488a0 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.stories.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.stories.tsx @@ -31,13 +31,8 @@ const testProviders: TestProviders[keyof TestProviders][] = [ type: Addon_TypesEnum.experimental_TEST_PROVIDER, id: 'component-tests', name: 'Component tests', - render: () => ( - - Component tests -
- Ran 2 seconds ago -
- ), + title: () => 'Component tests', + description: () => 'Ran 2 seconds ago', runnable: true, watchable: true, ...baseState, @@ -46,13 +41,8 @@ const testProviders: TestProviders[keyof TestProviders][] = [ type: Addon_TypesEnum.experimental_TEST_PROVIDER, id: 'visual-tests', name: 'Visual tests', - render: () => ( - - Visual tests -
- Not run -
- ), + title: () => 'Visual tests', + description: () => 'Not run', runnable: true, ...baseState, }, @@ -60,13 +50,7 @@ const testProviders: TestProviders[keyof TestProviders][] = [ type: Addon_TypesEnum.experimental_TEST_PROVIDER, id: 'linting', name: 'Linting', - render: () => ( - - Linting -
- Watching for changes -
- ), + render: () => Custom render function, ...baseState, watching: true, }, @@ -163,7 +147,6 @@ export const BothActive: Story = { export const CollapsedStatuses: Story = { args: Statuses.args, - play: Expanded.play, }; export const Running: Story = { @@ -182,7 +165,6 @@ export const RunningAll: Story = { export const CollapsedRunning: Story = { args: RunningAll.args, - play: Expanded.play, }; export const Cancellable: Story = { From 155741ec6c7b13e20a28ae18b59342367d6f61b8 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 14:48:52 +0100 Subject: [PATCH 15/21] Add aria labels --- .../test/src/components/TestProviderRender.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index 7f085ac2e435..bf665ca191c6 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -178,13 +178,20 @@ export const TestProviderRender: FC<{ ) : ( - } /> + } + /> } + icon={} right={`60%`} /> - } right={73} /> + } + right={73} + /> )} From 2f3ec95a46941f36479cc1e621e4edbe12e44651 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 26 Nov 2024 15:06:19 +0100 Subject: [PATCH 16/21] Disable coverage and a11y checkboxes for now --- code/addons/test/src/components/TestProviderRender.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index bf665ca191c6..544892e651f0 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -158,6 +158,7 @@ export const TestProviderRender: FC<{ right={ updateConfig({ coverage: !config.coverage })} /> @@ -170,6 +171,7 @@ export const TestProviderRender: FC<{ right={ updateConfig({ a11y: !config.a11y })} /> From f0f2a4c5bd7a05956b37c2b7cd779f4b3d789a98 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 27 Nov 2024 01:32:23 +0800 Subject: [PATCH 17/21] Fix check --- .../src/manager/components/sidebar/__tests__/Sidebar.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/code/core/src/manager/components/sidebar/__tests__/Sidebar.test.tsx b/code/core/src/manager/components/sidebar/__tests__/Sidebar.test.tsx index 2e1583fdcf87..0840be5fcd71 100644 --- a/code/core/src/manager/components/sidebar/__tests__/Sidebar.test.tsx +++ b/code/core/src/manager/components/sidebar/__tests__/Sidebar.test.tsx @@ -50,6 +50,7 @@ const generateStories = ({ title, refId }: { title: string; refId?: string }): A name: root, children: [componentId], startCollapsed: false, + tags: [], }, { type: 'component', From 9954cd1ca1adf8a37a1bfa445b315ba3ab89b063 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 27 Nov 2024 10:35:43 +0100 Subject: [PATCH 18/21] Collapsed by default --- code/core/src/manager/components/sidebar/TestingModule.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/core/src/manager/components/sidebar/TestingModule.tsx b/code/core/src/manager/components/sidebar/TestingModule.tsx index 09604461ec82..b7d5536ce203 100644 --- a/code/core/src/manager/components/sidebar/TestingModule.tsx +++ b/code/core/src/manager/components/sidebar/TestingModule.tsx @@ -169,7 +169,7 @@ export const TestingModule = ({ const contentRef = useRef(null); const [maxHeight, setMaxHeight] = useState(DEFAULT_HEIGHT); const [isUpdated, setUpdated] = useState(false); - const [isCollapsed, setCollapsed] = useState(false); + const [isCollapsed, setCollapsed] = useState(true); const [isChangingCollapse, setChangingCollapse] = useState(false); useEffect(() => { From e2d2e53fe64ca9824a48ecc99efdaab440f55f0b Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 27 Nov 2024 10:54:23 +0100 Subject: [PATCH 19/21] Don't show updated state when mounting --- code/addons/test/src/components/Description.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/code/addons/test/src/components/Description.tsx b/code/addons/test/src/components/Description.tsx index 3f1257972666..9e7441aca3d3 100644 --- a/code/addons/test/src/components/Description.tsx +++ b/code/addons/test/src/components/Description.tsx @@ -24,14 +24,18 @@ export function Description({ errorMessage: string; setIsModalOpen: React.Dispatch>; }) { + const isMounted = React.useRef(false); const [isUpdated, setUpdated] = React.useState(false); useEffect(() => { - setUpdated(true); - const timeout = setTimeout(setUpdated, 2000, false); - return () => { - clearTimeout(timeout); - }; + if (isMounted.current) { + setUpdated(true); + const timeout = setTimeout(setUpdated, 2000, false); + return () => { + clearTimeout(timeout); + }; + } + isMounted.current = true; }, [state.config]); let description: string | React.ReactNode = 'Not run'; From a74f5115845b21e4ca2e82b4430595d285c07052 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 27 Nov 2024 11:03:30 +0100 Subject: [PATCH 20/21] Only apply hover styling to labels that contain an enabled input --- .../test/src/components/TestProviderRender.tsx | 1 + .../src/components/components/tooltip/ListItem.tsx | 13 +++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/code/addons/test/src/components/TestProviderRender.tsx b/code/addons/test/src/components/TestProviderRender.tsx index 544892e651f0..588523f89ad9 100644 --- a/code/addons/test/src/components/TestProviderRender.tsx +++ b/code/addons/test/src/components/TestProviderRender.tsx @@ -147,6 +147,7 @@ export const TestProviderRender: FC<{ {isEditing ? ( } right={} diff --git a/code/core/src/components/components/tooltip/ListItem.tsx b/code/core/src/components/components/tooltip/ListItem.tsx index 023914f6f2d3..2a93287af503 100644 --- a/code/core/src/components/components/tooltip/ListItem.tsx +++ b/code/core/src/components/components/tooltip/ListItem.tsx @@ -140,8 +140,8 @@ const Item = styled.div( paddingLeft: 10, }, }), - ({ theme, href, onClick, as }) => - (href || onClick || as === 'label') && { + ({ theme, href, onClick }) => + (href || onClick) && { cursor: 'pointer', '&:hover': { background: theme.background.hoverable, @@ -150,6 +150,15 @@ const Item = styled.div( opacity: 1, }, }, + ({ theme, as }) => + as === 'label' && { + '&:has(input:not(:disabled))': { + cursor: 'pointer', + '&:hover': { + background: theme.background.hoverable, + }, + }, + }, ({ disabled }) => disabled && { cursor: 'not-allowed' } ); From e68fd8b59b6cc07224cb49bbaae277bc19d0913b Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 27 Nov 2024 11:38:14 +0100 Subject: [PATCH 21/21] Fix E2E tests for Testing Module --- .../react/e2e-tests/component-testing.spec.ts | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts b/test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts index 2b60a9314d51..68276fdb1a22 100644 --- a/test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts +++ b/test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts @@ -49,11 +49,14 @@ test.describe("component testing", () => { await sbPage.navigateToStory("addons/test", "Mismatch Failure"); + const expandButton = await page.getByLabel('Expand testing module') + await expandButton.click(); + // For whatever reason, sometimes it takes longer for the story to load const storyElement = sbPage .getCanvasBodyElement() .getByRole("button", { name: "test" }); - await expect(storyElement).toBeVisible({ timeout: 10000 }); + await expect(storyElement).toBeVisible({ timeout: 30000 }); await sbPage.viewAddonPanel("Component tests"); @@ -65,13 +68,12 @@ test.describe("component testing", () => { if ((await testStoryElement.getAttribute("aria-expanded")) !== "true") { testStoryElement.click(); } - + const testingModuleDescription = await page.locator('#testing-module-description'); await expect(testingModuleDescription).toContainText('Not run'); const runTestsButton = await page.getByLabel('Start component tests') - await runTestsButton.click(); await expect(testingModuleDescription).toContainText('Testing', { timeout: 60000 }); @@ -117,14 +119,17 @@ test.describe("component testing", () => { const sbPage = new SbPage(page, expect); await sbPage.navigateToStory("addons/test", "Expected Failure"); - + + const expandButton = await page.getByLabel('Expand testing module') + await expandButton.click(); + // For whatever reason, sometimes it takes longer for the story to load const storyElement = sbPage .getCanvasBodyElement() .getByRole("button", { name: "test" }); - await expect(storyElement).toBeVisible({ timeout: 10000 }); + await expect(storyElement).toBeVisible({ timeout: 30000 }); - await expect(page.locator('#testing-module-title')).toHaveText('Component tests'); + await expect(page.locator('#testing-module-title')).toHaveText('Run local tests'); const testingModuleDescription = await page.locator('#testing-module-description'); @@ -142,7 +147,7 @@ test.describe("component testing", () => { // Wait for test results to appear await expect(testingModuleDescription).toHaveText(/Ran \d+ tests/, { timeout: 30000 }); - + await expect(runTestsButton).toBeEnabled(); await expect(watchModeButton).toBeEnabled(); @@ -186,11 +191,14 @@ test.describe("component testing", () => { const sbPage = new SbPage(page, expect); await sbPage.navigateToStory("addons/test", "Expected Failure"); + const expandButton = await page.getByLabel('Expand testing module') + await expandButton.click(); + // For whatever reason, sometimes it takes longer for the story to load const storyElement = sbPage .getCanvasBodyElement() .getByRole("button", { name: "test" }); - await expect(storyElement).toBeVisible({ timeout: 10000 }); + await expect(storyElement).toBeVisible({ timeout: 30000 }); await page.getByLabel("Enable watch mode for Component tests").click();