From 0d1ce628e0f5d844e2288513b154432f133d5808 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 28 Oct 2022 16:52:37 +1100 Subject: [PATCH 1/6] Don't emit key strokes if *any* play function is in progress --- code/lib/preview-web/src/PreviewWeb.test.ts | 46 +++++++++++++++++++++ code/lib/preview-web/src/PreviewWeb.tsx | 2 +- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/code/lib/preview-web/src/PreviewWeb.test.ts b/code/lib/preview-web/src/PreviewWeb.test.ts index 356d22018c37..2f085ccf36d5 100644 --- a/code/lib/preview-web/src/PreviewWeb.test.ts +++ b/code/lib/preview-web/src/PreviewWeb.test.ts @@ -3302,6 +3302,52 @@ describe('PreviewWeb', () => { expect.objectContaining({}) ); }); + + it('does not emit PREVIEW_KEYDOWN during story play functions', async () => { + document.location.search = '?id=component-one--a'; + + const [gate, openGate] = createGate(); + componentOneExports.a.play.mockImplementationOnce(async () => gate); + const preview = new PreviewWeb(); + await preview.initialize({ importFn, getProjectAnnotations }); + await waitForRenderPhase('playing'); + + await preview.onKeydown({ + target: { tagName: 'div', getAttribute: jest.fn().mockReturnValue(null) }, + } as any); + + expect(mockChannel.emit).not.toHaveBeenCalledWith( + PREVIEW_KEYDOWN, + expect.objectContaining({}) + ); + openGate(); + }); + + it('does not emit PREVIEW_KEYDOWN during docs play functions', async () => { + document.location.search = '?id=component-one--a'; + + const preview = await createAndRenderPreview(); + + mockChannel.emit.mockClear(); + const [gate, openGate] = createGate(); + componentOneExports.b.play.mockImplementationOnce(async () => gate); + preview.renderStoryToElement( + await preview.storyStore.loadStory({ storyId: 'component-one--b' }), + {} as any, + { runPlayFunction: true } + ); + await waitForRenderPhase('playing'); + + await preview.onKeydown({ + target: { tagName: 'div', getAttribute: jest.fn().mockReturnValue(null) }, + } as any); + + expect(mockChannel.emit).not.toHaveBeenCalledWith( + PREVIEW_KEYDOWN, + expect.objectContaining({}) + ); + openGate(); + }); }); describe('extract', () => { diff --git a/code/lib/preview-web/src/PreviewWeb.tsx b/code/lib/preview-web/src/PreviewWeb.tsx index 71185cfaa8ab..c0365657bcdb 100644 --- a/code/lib/preview-web/src/PreviewWeb.tsx +++ b/code/lib/preview-web/src/PreviewWeb.tsx @@ -200,7 +200,7 @@ export class PreviewWeb extends Preview r.disableKeyListeners) && !focusInInput(event)) { // We have to pick off the keys of the event that we need on the other side const { altKey, ctrlKey, metaKey, shiftKey, key, code, keyCode } = event; this.channel.emit(PREVIEW_KEYDOWN, { From 6e2b837a8851bc7dfaa6f0f2eb16b834d364d446 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 28 Oct 2022 22:34:25 +1100 Subject: [PATCH 2/6] Add stories to check shortcuts aren't called from play --- code/lib/preview-web/src/PreviewWeb.test.ts | 3 +- .../template/stories/shortcuts.stories.ts | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 code/lib/store/template/stories/shortcuts.stories.ts diff --git a/code/lib/preview-web/src/PreviewWeb.test.ts b/code/lib/preview-web/src/PreviewWeb.test.ts index 2f085ccf36d5..36cf0b17ce72 100644 --- a/code/lib/preview-web/src/PreviewWeb.test.ts +++ b/code/lib/preview-web/src/PreviewWeb.test.ts @@ -3333,8 +3333,7 @@ describe('PreviewWeb', () => { componentOneExports.b.play.mockImplementationOnce(async () => gate); preview.renderStoryToElement( await preview.storyStore.loadStory({ storyId: 'component-one--b' }), - {} as any, - { runPlayFunction: true } + {} as any ); await waitForRenderPhase('playing'); diff --git a/code/lib/store/template/stories/shortcuts.stories.ts b/code/lib/store/template/stories/shortcuts.stories.ts new file mode 100644 index 000000000000..36fd93ee6ac2 --- /dev/null +++ b/code/lib/store/template/stories/shortcuts.stories.ts @@ -0,0 +1,29 @@ +import globalThis from 'global'; +import { userEvent, within } from '@storybook/testing-library'; +import { PREVIEW_KEYDOWN } from '@storybook/core-events'; +import { jest, expect } from '@storybook/jest'; + +export default { + component: globalThis.Components.Form, + tags: ['docsPage'], +}; + +export const KeydownDuringPlay = { + play: async ({ canvasElement }) => { + const channel = globalThis.__STORYBOOK_ADDONS_CHANNEL__; + + const previewKeydown = jest.fn(); + channel.on(PREVIEW_KEYDOWN, previewKeydown); + // eslint-disable-next-line storybook/await-interactions + const promise = userEvent.type(await within(canvasElement).findByTestId('value'), '000000', { + delay: 100, + }); + + const button = await within(canvasElement).findByRole('button'); + button.focus(); + + await promise; + + expect(previewKeydown).not.toBeCalled(); + }, +}; From ebc28fc8986c9e090d45afa538c291280e729a5a Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 28 Oct 2022 23:30:52 +1100 Subject: [PATCH 3/6] Add E2E test for keyboard shortcut behaviour --- code/e2e-tests/preview-web.spec.ts | 34 +++++++++++++++++++ code/lib/preview-web/src/PreviewWeb.tsx | 1 + .../template/stories/shortcuts.stories.ts | 14 +++----- 3 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 code/e2e-tests/preview-web.spec.ts diff --git a/code/e2e-tests/preview-web.spec.ts b/code/e2e-tests/preview-web.spec.ts new file mode 100644 index 000000000000..62b6a85289ae --- /dev/null +++ b/code/e2e-tests/preview-web.spec.ts @@ -0,0 +1,34 @@ +import { test, expect } from '@playwright/test'; +import process from 'process'; +import { SbPage } from './util'; + +const storybookUrl = process.env.STORYBOOK_URL || 'http://localhost:8001'; + +test.describe('preview-web', () => { + test.beforeEach(async ({ page }) => { + await page.goto(storybookUrl); + await new SbPage(page).waitUntilLoaded(); + }); + + test('should pass over shortcuts, but not from play functions, story', async ({ page }) => { + const sbPage = new SbPage(page); + await sbPage.navigateToStory('lib/store/shortcuts', 'keydown-during-play'); + await new Promise((r) => setTimeout(r, 1000)); + + await expect(sbPage.page.locator('.sidebar-container')).toBeVisible(); + + await sbPage.previewRoot().locator('button').press('s'); + await expect(sbPage.page.locator('.sidebar-container')).not.toBeVisible(); + }); + + test('should pass over shortcuts, but not from play functions, docs', async ({ page }) => { + const sbPage = new SbPage(page); + await sbPage.navigateToStory('lib/store/shortcuts', 'docs'); + + await expect(sbPage.page.locator('.sidebar-container')).toBeVisible(); + await new Promise((r) => setTimeout(r, 1000)); + + await sbPage.previewRoot().getByText('Submit').press('s'); + await expect(sbPage.page.locator('.sidebar-container')).not.toBeVisible(); + }); +}); diff --git a/code/lib/preview-web/src/PreviewWeb.tsx b/code/lib/preview-web/src/PreviewWeb.tsx index c0365657bcdb..4c36214468f0 100644 --- a/code/lib/preview-web/src/PreviewWeb.tsx +++ b/code/lib/preview-web/src/PreviewWeb.tsx @@ -200,6 +200,7 @@ export class PreviewWeb extends Preview r.disableKeyListeners) && !focusInInput(event)) { // We have to pick off the keys of the event that we need on the other side const { altKey, ctrlKey, metaKey, shiftKey, key, code, keyCode } = event; diff --git a/code/lib/store/template/stories/shortcuts.stories.ts b/code/lib/store/template/stories/shortcuts.stories.ts index 36fd93ee6ac2..6a0ffec340bc 100644 --- a/code/lib/store/template/stories/shortcuts.stories.ts +++ b/code/lib/store/template/stories/shortcuts.stories.ts @@ -2,6 +2,7 @@ import globalThis from 'global'; import { userEvent, within } from '@storybook/testing-library'; import { PREVIEW_KEYDOWN } from '@storybook/core-events'; import { jest, expect } from '@storybook/jest'; +import type { PlayFunctionContext } from '@storybook/csf'; export default { component: globalThis.Components.Form, @@ -9,20 +10,13 @@ export default { }; export const KeydownDuringPlay = { - play: async ({ canvasElement }) => { + play: async ({ canvasElement }: PlayFunctionContext) => { const channel = globalThis.__STORYBOOK_ADDONS_CHANNEL__; const previewKeydown = jest.fn(); channel.on(PREVIEW_KEYDOWN, previewKeydown); - // eslint-disable-next-line storybook/await-interactions - const promise = userEvent.type(await within(canvasElement).findByTestId('value'), '000000', { - delay: 100, - }); - - const button = await within(canvasElement).findByRole('button'); - button.focus(); - - await promise; + const button = await within(canvasElement).findByText('Submit'); + await userEvent.type(button, 's'); expect(previewKeydown).not.toBeCalled(); }, From e50db9fc261fd12951a21768c5405298993bc280 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 28 Oct 2022 23:50:22 +1100 Subject: [PATCH 4/6] Update code/lib/preview-web/src/PreviewWeb.tsx --- code/lib/preview-web/src/PreviewWeb.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/code/lib/preview-web/src/PreviewWeb.tsx b/code/lib/preview-web/src/PreviewWeb.tsx index 4c36214468f0..c0365657bcdb 100644 --- a/code/lib/preview-web/src/PreviewWeb.tsx +++ b/code/lib/preview-web/src/PreviewWeb.tsx @@ -200,7 +200,6 @@ export class PreviewWeb extends Preview r.disableKeyListeners) && !focusInInput(event)) { // We have to pick off the keys of the event that we need on the other side const { altKey, ctrlKey, metaKey, shiftKey, key, code, keyCode } = event; From ce99cfe099becc1cf306d3cc52573056f034c819 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 28 Oct 2022 23:44:10 +1100 Subject: [PATCH 5/6] Remove unnecessary delay --- code/e2e-tests/preview-web.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/code/e2e-tests/preview-web.spec.ts b/code/e2e-tests/preview-web.spec.ts index 62b6a85289ae..f71bbe3658c2 100644 --- a/code/e2e-tests/preview-web.spec.ts +++ b/code/e2e-tests/preview-web.spec.ts @@ -13,7 +13,6 @@ test.describe('preview-web', () => { test('should pass over shortcuts, but not from play functions, story', async ({ page }) => { const sbPage = new SbPage(page); await sbPage.navigateToStory('lib/store/shortcuts', 'keydown-during-play'); - await new Promise((r) => setTimeout(r, 1000)); await expect(sbPage.page.locator('.sidebar-container')).toBeVisible(); @@ -26,7 +25,6 @@ test.describe('preview-web', () => { await sbPage.navigateToStory('lib/store/shortcuts', 'docs'); await expect(sbPage.page.locator('.sidebar-container')).toBeVisible(); - await new Promise((r) => setTimeout(r, 1000)); await sbPage.previewRoot().getByText('Submit').press('s'); await expect(sbPage.page.locator('.sidebar-container')).not.toBeVisible(); From 59d2b28577980d3c24b672eebb98df4a05b377c5 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 31 Oct 2022 12:09:58 +1100 Subject: [PATCH 6/6] Make button selector more specific --- code/e2e-tests/preview-web.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/e2e-tests/preview-web.spec.ts b/code/e2e-tests/preview-web.spec.ts index f71bbe3658c2..3dec28a68910 100644 --- a/code/e2e-tests/preview-web.spec.ts +++ b/code/e2e-tests/preview-web.spec.ts @@ -26,7 +26,7 @@ test.describe('preview-web', () => { await expect(sbPage.page.locator('.sidebar-container')).toBeVisible(); - await sbPage.previewRoot().getByText('Submit').press('s'); + await sbPage.previewRoot().getByRole('button').getByText('Submit').press('s'); await expect(sbPage.page.locator('.sidebar-container')).not.toBeVisible(); }); });