From 35833f58208a74c931aca482b4cecc404ebe7a0f Mon Sep 17 00:00:00 2001 From: Alexandre Philibeaux Date: Tue, 24 Dec 2024 12:27:21 +0100 Subject: [PATCH 1/5] feat(playwright): add playwright e2e on checkbox list --- .github/workflows/playwright.yml | 27 ++++++++++++ .gitignore | 4 ++ e2e/playwright.config.ts | 73 ++++++++++++++++++++++++++++++++ e2e/tests/list-checkbox.spec.ts | 20 +++++++++ e2e/tests/list.spec.ts | 9 ++++ package.json | 4 ++ pnpm-lock.yaml | 47 ++++++++++++++++++-- 7 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/playwright.yml create mode 100644 e2e/playwright.config.ts create mode 100644 e2e/tests/list-checkbox.spec.ts create mode 100644 e2e/tests/list.spec.ts diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml new file mode 100644 index 0000000000..e35b7b91ab --- /dev/null +++ b/.github/workflows/playwright.yml @@ -0,0 +1,27 @@ +name: Playwright Tests +on: + push: + branches: [main, master] + pull_request: + branches: [main, master] +jobs: + test: + timeout-minutes: 60 + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: lts/* + - name: Install dependencies + run: npm install -g pnpm && pnpm install + - name: Install Playwright Browsers + run: pnpm exec playwright install --with-deps + - name: Run Playwright tests + run: pnpm run test:e2e + - uses: actions/upload-artifact@v4 + if: ${{ !cancelled() }} + with: + name: playwright-report + path: playwright-report/ + retention-days: 30 diff --git a/.gitignore b/.gitignore index 31dce8deab..4880e961e0 100644 --- a/.gitignore +++ b/.gitignore @@ -49,3 +49,7 @@ tools/*/.turbo # next next-env.d.ts* +/test-results/ +/playwright-report/ +/blob-report/ +/playwright/.cache/ diff --git a/e2e/playwright.config.ts b/e2e/playwright.config.ts new file mode 100644 index 0000000000..3161952c63 --- /dev/null +++ b/e2e/playwright.config.ts @@ -0,0 +1,73 @@ +/* eslint-disable eslint-comments/disable-enable-pair */ +/* eslint-disable import/no-extraneous-dependencies */ +import { defineConfig, devices } from '@playwright/test' + +const isCI = process.env['CI'] + +const baseURL = process.env['BASE_URL'] ?? 'http://localhost:6006' + +export default defineConfig({ + testDir: './tests', + /* Run tests in files in parallel */ + fullyParallel: true, + /* Fail the build on CI if you accidentally left test.only in the source code. */ + forbidOnly: !!isCI, + /* Retry on CI only */ + retries: isCI ? 2 : 0, + /* Opt out of parallel tests on CI. */ + workers: isCI ? 1 : undefined, + /* Reporter to use. See https://playwright.dev/docs/test-reporters */ + reporter: 'html', + /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ + use: { + baseURL, + + /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */ + trace: 'on-first-retry', + }, + + /* Configure projects for major browsers */ + projects: [ + { + name: 'chromium', + use: { ...devices['Desktop Chrome'] }, + }, + + // { + // name: 'firefox', + // use: { ...devices['Desktop Firefox'] }, + // }, + + // { + // name: 'webkit', + // use: { ...devices['Desktop Safari'] }, + // }, + + /* Test against mobile viewports. */ + // { + // name: 'Mobile Chrome', + // use: { ...devices['Pixel 5'] }, + // }, + // { + // name: 'Mobile Safari', + // use: { ...devices['iPhone 12'] }, + // }, + + /* Test against branded browsers. */ + // { + // name: 'Microsoft Edge', + // use: { ...devices['Desktop Edge'], channel: 'msedge' }, + // }, + // { + // name: 'Google Chrome', + // use: { ...devices['Desktop Chrome'], channel: 'chrome' }, + // }, + ], + + /* Run your local dev server before starting the tests */ + // webServer: { + // command: 'npm run start', + // url: 'http://127.0.0.1:3000', + // reuseExistingServer: !process.env.CI, + // }, +}) diff --git a/e2e/tests/list-checkbox.spec.ts b/e2e/tests/list-checkbox.spec.ts new file mode 100644 index 0000000000..437f685fe0 --- /dev/null +++ b/e2e/tests/list-checkbox.spec.ts @@ -0,0 +1,20 @@ +import { expect, test } from '@playwright/test' + +const defaultLocator = 'iframe[title="storybook-preview-iframe"]' + +test('has title', async ({ page, baseURL }) => { + const url = `${baseURL}/?path=/story/components-data-display-list--selectable` + await page.goto(url) + + const rootLocator = page.locator(defaultLocator).contentFrame() + + await rootLocator + .getByRole('row', { name: 'select Venus 0.718AU 0.728AU' }) + .getByLabel('select') + .check() + + const checkbox = rootLocator.locator( + `input[type='checkbox'][name='list-select-checkbox'][value="venus"]`, + ) + await expect(checkbox).toBeChecked() +}) diff --git a/e2e/tests/list.spec.ts b/e2e/tests/list.spec.ts new file mode 100644 index 0000000000..900d6294e4 --- /dev/null +++ b/e2e/tests/list.spec.ts @@ -0,0 +1,9 @@ +import { expect, test } from '@playwright/test' + +test('has title', async ({ page, baseURL }) => { + const url = baseURL + await page.goto(`${url}`) + + // Expect a title "to contain" a substring. + await expect(page).toHaveTitle('Get started - Docs ⋅ Storybook') +}) diff --git a/package.json b/package.json index 64d031c5fd..0d0607d15b 100644 --- a/package.json +++ b/package.json @@ -20,6 +20,9 @@ "build:storybook:stats": "pnpm turbo build:storybook -- --webpack-stats-json", "test:unit": "turbo run test:unit", "test:unit:coverage": "turbo run test:unit:coverage", + "test:e2e": "playwright test -c e2e", + "test:e2e:headed": "pnpm run test:e2e --headed", + "test:e2e:debug": "PWDEBUG=1 pnpm run test:e2e", "check:deps": "npx depcheck . --skip-missing=true --ignores='bin,eslint,vite,jest,husky,@commitlint/*,@babel/*,babel-*'", "commit": "npx git-cz -a --disable-emoji", "start": "STORYBOOK_ENVIRONMENT=development storybook dev -p 6006", @@ -110,6 +113,7 @@ "@eslint/compat": "1.2.4", "@eslint/eslintrc": "3.2.0", "@manypkg/cli": "0.23.0", + "@playwright/test": "1.49.1", "@scaleway/eslint-config-react": "4.0.9", "@scaleway/tsconfig": "1.1.1", "@size-limit/file": "11.1.6", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 903cc784f2..447309588d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -74,6 +74,9 @@ importers: '@manypkg/cli': specifier: 0.23.0 version: 0.23.0 + '@playwright/test': + specifier: 1.49.1 + version: 1.49.1 '@scaleway/eslint-config-react': specifier: 4.0.9 version: 4.0.9(eslint@9.17.0(jiti@2.4.2))(typescript@5.7.2) @@ -331,7 +334,7 @@ importers: version: link:../../packages/ui next: specifier: 15.1.2 - version: 15.1.2(@babel/core@7.26.0)(react-dom@19.0.0(react@19.0.0))(react@19.0.0) + version: 15.1.2(@babel/core@7.26.0)(@playwright/test@1.49.1)(react-dom@19.0.0(react@19.0.0))(react@19.0.0) react: specifier: 19.0.0 version: 19.0.0 @@ -389,7 +392,7 @@ importers: version: link:../../packages/ui next: specifier: 15.1.2 - version: 15.1.2(@babel/core@7.26.0)(react-dom@19.0.0(react@19.0.0))(react@19.0.0) + version: 15.1.2(@babel/core@7.26.0)(@playwright/test@1.49.1)(react-dom@19.0.0(react@19.0.0))(react@19.0.0) react: specifier: 19.0.0 version: 19.0.0 @@ -438,7 +441,7 @@ importers: version: link:../../packages/ui next: specifier: 15.1.2 - version: 15.1.2(@babel/core@7.26.0)(react-dom@19.0.0(react@19.0.0))(react@19.0.0) + version: 15.1.2(@babel/core@7.26.0)(@playwright/test@1.49.1)(react-dom@19.0.0(react@19.0.0))(react@19.0.0) react: specifier: 19.0.0 version: 19.0.0 @@ -2551,6 +2554,11 @@ packages: resolution: {integrity: sha512-+1VkjdD0QBLPodGrJUeqarH8VAIvQODIbwh9XpP5Syisf7YoQgsJKPNFoqqLQlu+VQ/tVSshMR6loPMn8U+dPg==} engines: {node: '>=14'} + '@playwright/test@1.49.1': + resolution: {integrity: sha512-Ky+BVzPz8pL6PQxHqNRW1k3mIyv933LML7HktS8uik0bUXNCdPhoS/kLihiO1tMf/egaJb4IutXd7UywvXEW+g==} + engines: {node: '>=18'} + hasBin: true + '@pnpm/config.env-replace@1.1.0': resolution: {integrity: sha512-htyl8TWnKL7K/ESFa1oW2UB5lVDxuF5DpM7tBi6Hu2LNL3mWkIzNLG6N4zoCUP1lCKNxWy/3iu8mS8MvToGd6w==} engines: {node: '>=12.22.0'} @@ -4922,6 +4930,11 @@ packages: fs.realpath@1.0.0: resolution: {integrity: sha512-OO0pH2lK6a0hZnAdau5ItzHPI6pUlvI7jMVnxUQRtw4owF2wk8lOSabtGDCTP4Ggrg2MbGnWO9X8K1t4+fGMDw==} + fsevents@2.3.2: + resolution: {integrity: sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==} + engines: {node: ^8.16.0 || ^10.6.0 || >=11.0.0} + os: [darwin] + fsevents@2.3.3: resolution: {integrity: sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw==} engines: {node: ^8.16.0 || ^10.6.0 || >=11.0.0} @@ -6327,6 +6340,16 @@ packages: resolution: {integrity: sha512-Ie9z/WINcxxLp27BKOCHGde4ITq9UklYKDzVo1nhk5sqGEXU3FpkwP5GM2voTGJkGd9B3Otl+Q4uwSOeSUtOBA==} engines: {node: '>=14.16'} + playwright-core@1.49.1: + resolution: {integrity: sha512-BzmpVcs4kE2CH15rWfzpjzVGhWERJfmnXmniSyKeRZUs9Ws65m+RGIi7mjJK/euCegfn3i7jvqWeWyHe9y3Vgg==} + engines: {node: '>=18'} + hasBin: true + + playwright@1.49.1: + resolution: {integrity: sha512-VYL8zLoNTBxVOrJBbDuRgDWa3i+mfQgDTrL8Ah9QXZ7ax4Dsj0MSq5bYgytRnDVVe+njoKnfsYkH3HzqVj5UZA==} + engines: {node: '>=18'} + hasBin: true + polished@4.3.1: resolution: {integrity: sha512-OBatVyC/N7SCW/FaDHrSd+vn0o5cS855TOmYi4OkdWUMSJCET/xip//ch8xGUvtr3i44X9LVyWwQlRMTN3pwSA==} engines: {node: '>=10'} @@ -10190,6 +10213,10 @@ snapshots: '@pkgjs/parseargs@0.11.0': optional: true + '@playwright/test@1.49.1': + dependencies: + playwright: 1.49.1 + '@pnpm/config.env-replace@1.1.0': {} '@pnpm/network.ca-file@1.0.2': @@ -13104,6 +13131,9 @@ snapshots: fs.realpath@1.0.0: {} + fsevents@2.3.2: + optional: true + fsevents@2.3.3: optional: true @@ -14378,7 +14408,7 @@ snapshots: dependencies: enhanced-resolve: 5.16.0 - next@15.1.2(@babel/core@7.26.0)(react-dom@19.0.0(react@19.0.0))(react@19.0.0): + next@15.1.2(@babel/core@7.26.0)(@playwright/test@1.49.1)(react-dom@19.0.0(react@19.0.0))(react@19.0.0): dependencies: '@next/env': 15.1.2 '@swc/counter': 0.1.3 @@ -14398,6 +14428,7 @@ snapshots: '@next/swc-linux-x64-musl': 15.1.2 '@next/swc-win32-arm64-msvc': 15.1.2 '@next/swc-win32-x64-msvc': 15.1.2 + '@playwright/test': 1.49.1 sharp: 0.33.5 transitivePeerDependencies: - '@babel/core' @@ -14695,6 +14726,14 @@ snapshots: dependencies: find-up: 6.3.0 + playwright-core@1.49.1: {} + + playwright@1.49.1: + dependencies: + playwright-core: 1.49.1 + optionalDependencies: + fsevents: 2.3.2 + polished@4.3.1: dependencies: '@babel/runtime': 7.26.0 From 056f82fdb78451add3fc400bd01f8ae080db3c68 Mon Sep 17 00:00:00 2001 From: Alexandre Philibeaux Date: Tue, 24 Dec 2024 13:10:57 +0100 Subject: [PATCH 2/5] fix(checkbox-list): event click --- e2e/tests/list-checkbox.spec.ts | 2 +- packages/ui/src/components/Checkbox/index.tsx | 309 +++++++------- packages/ui/src/components/List/Row.tsx | 386 +++++++++--------- 3 files changed, 343 insertions(+), 354 deletions(-) diff --git a/e2e/tests/list-checkbox.spec.ts b/e2e/tests/list-checkbox.spec.ts index 437f685fe0..98b121bc8a 100644 --- a/e2e/tests/list-checkbox.spec.ts +++ b/e2e/tests/list-checkbox.spec.ts @@ -2,7 +2,7 @@ import { expect, test } from '@playwright/test' const defaultLocator = 'iframe[title="storybook-preview-iframe"]' -test('has title', async ({ page, baseURL }) => { +test('checkbox list', async ({ page, baseURL }) => { const url = `${baseURL}/?path=/story/components-data-display-list--selectable` await page.goto(url) diff --git a/packages/ui/src/components/Checkbox/index.tsx b/packages/ui/src/components/Checkbox/index.tsx index bc42cd6725..5510273be1 100644 --- a/packages/ui/src/components/Checkbox/index.tsx +++ b/packages/ui/src/components/Checkbox/index.tsx @@ -1,13 +1,8 @@ import { useTheme } from '@emotion/react' import styled from '@emotion/styled' import { AsteriskIcon } from '@ultraviolet/icons' -import type { - ChangeEvent, - ForwardedRef, - InputHTMLAttributes, - ReactNode, -} from 'react' -import { forwardRef, useCallback, useEffect, useId, useState } from 'react' +import type { ChangeEvent, InputHTMLAttributes, ReactNode, Ref } from 'react' +import { useCallback, useEffect, useId, useState } from 'react' import { Loader } from '../Loader' import { Stack } from '../Stack' import { Text } from '../Text' @@ -275,6 +270,7 @@ type LabelProp = } type CheckboxProps = { + ref?: Ref error?: string | ReactNode /** * @deprecated Size prop is deprecated and will be removed in next major update. @@ -302,164 +298,163 @@ type CheckboxProps = { | 'id' | 'onChange' | 'tabIndex' + | 'onClick' > & LabelProp /** * Checkbox is an input component used to select or deselect an option. */ -export const Checkbox = forwardRef( - ( - { - id, - checked = false, - onChange, - onFocus, - onBlur, - error, - name, - helper, - value, - size, - children, - progress = false, - disabled = false, - autoFocus = false, - className, - 'data-visibility': dataVisibility, - 'aria-label': ariaLabel, - required, - 'data-testid': dataTestId, - tooltip, - tabIndex, - }: CheckboxProps, - ref: ForwardedRef, - ) => { - const theme = useTheme() - const [state, setState] = useState(checked) - const uniqId = useId() - const localId = id ?? uniqId - - useEffect(() => { - setState(checked) - }, [checked]) - - const onLocalChange = useCallback( - (event: ChangeEvent) => { - if (!progress) onChange?.(event) - setState(current => - current === 'indeterminate' ? false : event.target.checked, - ) - }, - [onChange, progress, setState], - ) - - return ( - - - {progress ? ( - - - - ) : null} - - - {!progress ? ( - - - {state !== 'indeterminate' ? ( - +export const Checkbox = ({ + id, + checked = false, + onChange, + onFocus, + onBlur, + onClick, + error, + name, + helper, + value, + size, + children, + progress = false, + disabled = false, + autoFocus = false, + className, + 'data-visibility': dataVisibility, + 'aria-label': ariaLabel, + required, + 'data-testid': dataTestId, + tooltip, + tabIndex, + ref, +}: CheckboxProps) => { + const theme = useTheme() + const [state, setState] = useState(checked) + const uniqId = useId() + const localId = id ?? uniqId + + useEffect(() => { + setState(checked) + }, [checked]) + + const onLocalChange = useCallback( + (event: ChangeEvent) => { + if (!progress) onChange?.(event) + setState(current => + current === 'indeterminate' ? false : event.target.checked, + ) + }, + [onChange, progress, setState], + ) + + return ( + + + {progress ? ( + + + + ) : null} + + + {!progress ? ( + + + {state !== 'indeterminate' ? ( + + ) : ( + + )} + + + ) : null} + + + + {children ? ( + <> + {typeof children === 'string' ? ( + + {children} + ) : ( - + {children} )} - - - ) : null} - - - - {children ? ( - <> - {typeof children === 'string' ? ( - - {children} - - ) : ( - {children} - )} - - ) : null} - {required ? ( - - - - ) : null} - - - {helper ? ( - - {helper} - + ) : null} - - {error && typeof error !== 'boolean' ? ( - - {error} - + {required ? ( + + + ) : null} - - - ) - }, -) + + {helper ? ( + + {helper} + + ) : null} + + {error && typeof error !== 'boolean' ? ( + + {error} + + ) : null} + + + + ) +} diff --git a/packages/ui/src/components/List/Row.tsx b/packages/ui/src/components/List/Row.tsx index 539753d3f6..9066190c1d 100644 --- a/packages/ui/src/components/List/Row.tsx +++ b/packages/ui/src/components/List/Row.tsx @@ -1,14 +1,7 @@ import { useTheme } from '@emotion/react' import styled from '@emotion/styled' -import type { ForwardedRef, ReactNode } from 'react' -import { - Children, - forwardRef, - useCallback, - useEffect, - useId, - useRef, -} from 'react' +import type { ReactNode, Ref } from 'react' +import { Children, useCallback, useEffect, useId, useRef } from 'react' import type { SENTIMENTS, space } from '../../theme' import { Button } from '../Button' import { Checkbox } from '../Checkbox' @@ -190,198 +183,199 @@ type RowProps = { */ expandablePadding?: keyof typeof space 'data-testid'?: string + ref?: Ref } -export const Row = forwardRef( - ( - { - children, - id, - expandable, - disabled, - selectDisabled, - sentiment = 'neutral', - expanded, - className, - expandablePadding, - 'data-testid': dataTestid, - }: RowProps, - ref: ForwardedRef, - ) => { - const { - selectable, - registerExpandableRow, - expandedRowIds, - expandRow, - collapseRow, - registerSelectableRow, - selectedRowIds, - selectRow, - unselectRow, - expandButton, - refList, - inRange, - columns, - } = useListContext() - - const theme = useTheme() - - const expandedRowId = useId() - - const checkboxRef = useRef(null) - - const isSelectDisabled = - disabled || (selectDisabled !== undefined && selectDisabled !== false) - - const hasExpandable = !!expandable - useEffect(() => { - if (hasExpandable) { - const unregisterCallback = registerExpandableRow(id, expanded) - - return unregisterCallback - } - - return undefined - }, [id, hasExpandable, registerExpandableRow, expanded, expandRow]) - - useEffect(() => { - if (!isSelectDisabled) { - const unregisterCallback = registerSelectableRow(id) - - return unregisterCallback - } - - return undefined - }, [id, registerSelectableRow, isSelectDisabled]) - - const toggleRowExpand = useCallback(() => { - if (expandedRowIds[id]) { - collapseRow(id) - } else { - expandRow(id) - } - }, [collapseRow, expandRow, expandedRowIds, id]) - - const canClickRowToExpand = !disabled && !!expandable && !expandButton - - useEffect(() => { - const refAtEffectStart = refList.current - const { current } = checkboxRef - - if (refAtEffectStart && current && !refAtEffectStart.includes(current)) { - refList.current.push(current) - } - }, [refList]) - - const childrenLength = - Children.count(children) + (selectable ? 1 : 0) + (expandButton ? 1 : 0) - - return ( - <> - { + const { + selectable, + registerExpandableRow, + expandedRowIds, + expandRow, + collapseRow, + registerSelectableRow, + selectedRowIds, + selectRow, + unselectRow, + expandButton, + refList, + inRange, + columns, + } = useListContext() + + const theme = useTheme() + + const expandedRowId = useId() + + const checkboxRef = useRef(null) + + const isSelectDisabled = + disabled || (selectDisabled !== undefined && selectDisabled !== false) + + const hasExpandable = !!expandable + useEffect(() => { + if (hasExpandable) { + const unregisterCallback = registerExpandableRow(id, expanded) + + return unregisterCallback + } + + return undefined + }, [id, hasExpandable, registerExpandableRow, expanded, expandRow]) + + useEffect(() => { + if (!isSelectDisabled) { + const unregisterCallback = registerSelectableRow(id) + + return unregisterCallback + } + + return undefined + }, [id, registerSelectableRow, isSelectDisabled]) + + const toggleRowExpand = useCallback(() => { + if (expandedRowIds[id]) { + collapseRow(id) + } else { + expandRow(id) + } + }, [collapseRow, expandRow, expandedRowIds, id]) + + const canClickRowToExpand = !disabled && !!expandable && !expandButton + + useEffect(() => { + const refAtEffectStart = refList.current + const { current } = checkboxRef + + if (refAtEffectStart && current && !refAtEffectStart.includes(current)) { + refList.current.push(current) + } + }, [refList]) + + const childrenLength = + Children.count(children) + (selectable ? 1 : 0) + (expandButton ? 1 : 0) + + return ( + <> + { + // if (event.key === ' ') { + // toggleRowExpand() + // event.preventDefault() + // } + // } + // : undefined + // } + // tabIndex={canClickRowToExpand ? 0 : -1} + sentiment={sentiment} + aria-disabled={disabled} + aria-expanded={expandable ? expandedRowIds[id] : undefined} + aria-controls={ + expandable && expandedRowIds[id] ? expandedRowId : undefined + } + data-highlight={selectable && !!selectedRowIds[id]} + data-testid={dataTestid} + columns={columns} + columnsStartAt={(selectable ? 1 : 0) + (expandButton ? 1 : 0)} + > + {selectable ? ( + + + + { + if (selectedRowIds[id]) { + unselectRow(id) + } else { + selectRow(id) + } + }} + onClick={() => { + if (selectedRowIds[id]) { + unselectRow(id) + } else { + selectRow(id) + } + }} + disabled={isSelectDisabled} + inRange={inRange.includes(id)} + /> + + + + ) : null} + {expandButton ? ( + +