From 2574595c737404aed65693a3c4fc6a04effb83a2 Mon Sep 17 00:00:00 2001 From: Yevhen Martynenko Date: Mon, 13 Nov 2023 14:45:55 +0200 Subject: [PATCH 1/2] *fix a11y tests --- packages/test-helpers/package.json | 4 +- packages/ui/__tests__/Checkbox.test.tsx | 3 - packages/ui/__tests__/Table/Cell.test.tsx | 26 ++++++-- packages/ui/__tests__/Table/Header.test.tsx | 69 +++++++++++++++------ packages/ui/__tests__/Table/Row.test.tsx | 49 ++++++++++----- packages/ui/__tests__/Table/Table.test.tsx | 9 ++- packages/ui/package.json | 2 +- packages/ui/src/Checkbox.tsx | 8 +-- packages/ui/src/Table/Table.tsx | 9 ++- packages/ui/src/Table/TableContent.tsx | 5 +- pnpm-lock.yaml | 52 ++++++++++------ 11 files changed, 154 insertions(+), 82 deletions(-) diff --git a/packages/test-helpers/package.json b/packages/test-helpers/package.json index 91f194c16..de27fd212 100644 --- a/packages/test-helpers/package.json +++ b/packages/test-helpers/package.json @@ -33,8 +33,8 @@ }, "dependencies": { "@hazelcast/services": "^1.3.1", - "@types/jest-axe": "^3.5.3", - "jest-axe": "^5.0.1" + "@types/jest-axe": "^3.5.8", + "jest-axe": "^8.0.0" }, "devDependencies": { "@types/enzyme": "^3.10.12", diff --git a/packages/ui/__tests__/Checkbox.test.tsx b/packages/ui/__tests__/Checkbox.test.tsx index 1a68b6caa..c61a41cb7 100644 --- a/packages/ui/__tests__/Checkbox.test.tsx +++ b/packages/ui/__tests__/Checkbox.test.tsx @@ -50,7 +50,6 @@ describe('Checkbox', () => { onChange, onBlur, checked: true, - 'aria-checked': 'true', 'aria-invalid': false, 'aria-required': true, 'aria-describedby': undefined, @@ -73,7 +72,6 @@ describe('Checkbox', () => { onChange, onBlur, checked: true, - 'aria-checked': 'true', 'aria-invalid': true, 'aria-required': undefined, 'aria-describedby': undefined, @@ -89,7 +87,6 @@ describe('Checkbox', () => { expect(wrapper.find(Check).exists()).toBeFalsy() expect(wrapper.find(Minus).exists()).toBeTruthy() - expect(wrapper.find('input').props()).toHaveProperty('aria-checked', 'mixed') }) it('Checkbox is passed a disabled property, input contains disabled property', async () => { diff --git a/packages/ui/__tests__/Table/Cell.test.tsx b/packages/ui/__tests__/Table/Cell.test.tsx index b13344041..ef9d4a65d 100644 --- a/packages/ui/__tests__/Table/Cell.test.tsx +++ b/packages/ui/__tests__/Table/Cell.test.tsx @@ -1,5 +1,5 @@ import { mountAndCheckA11Y } from '@hazelcast/test-helpers' -import React from 'react' +import React, { PropsWithChildren } from 'react' import { AlertTriangle } from 'react-feather' import { useUID } from 'react-uid' @@ -12,6 +12,12 @@ import styles from '../../src/Table/Cell.module.scss' jest.mock('react-uid') const useUIDMock = useUID as jest.Mock> +const Wrapper = (props: PropsWithChildren) => ( +
+
{props.children}
+
+) + describe('CellWarning', () => { const iconId = 'iconId' @@ -54,13 +60,14 @@ describe('CellWarning', () => { describe('Cell', () => { const cellPropsBase: CellProps = { align: 'left', + role: 'cell', } const expectedPropsBase = { 'data-test': 'table-cell', className: `${styles.td} ${styles.alignLeft}`, style: undefined, - role: undefined, + role: 'cell', 'aria-colspan': undefined, 'aria-sort': undefined, onClick: undefined, @@ -85,19 +92,22 @@ describe('Cell', () => { { ...expectedPropsBase, className: `${styles.td} ${styles.alignRight}` }, ], [ - { ...cellPropsBase, colSpan: 1, style: { width: 40 }, className: 'testClassName', role: '' }, + { ...cellPropsBase, colSpan: 1, style: { width: 40 }, className: 'testClassName' }, { ...expectedPropsBase, className: `${styles.td} ${styles.alignLeft} testClassName`, style: { width: 40 }, 'aria-colspan': 1, - role: '', }, ], ] it.each(data)('returns div with correct props for given Cell props', async (cellProps, expectedProps) => { - const wrapper = await mountAndCheckA11Y(Cell) + const wrapper = await mountAndCheckA11Y( + + Cell + , + ) expect(wrapper.findDataTest('table-cell').props()).toEqual(expectedProps) }) @@ -105,7 +115,11 @@ describe('Cell', () => { it('renders CellWarning when warning prop is defined', async () => { const warning = 'testWarning' - const wrapper = await mountAndCheckA11Y() + const wrapper = await mountAndCheckA11Y( + + + , + ) expect(wrapper.find(CellWarning).props()).toEqual({ warning: warning, diff --git a/packages/ui/__tests__/Table/Header.test.tsx b/packages/ui/__tests__/Table/Header.test.tsx index 96c32b183..e97936a1e 100644 --- a/packages/ui/__tests__/Table/Header.test.tsx +++ b/packages/ui/__tests__/Table/Header.test.tsx @@ -1,22 +1,28 @@ -import { mountAndCheckA11Y } from '@hazelcast/test-helpers' -import React from 'react' +import React, { ReactNode } from 'react' import { ChevronDown, ChevronUp } from 'react-feather' +import { mountAndCheckA11Y } from '@hazelcast/test-helpers' import { Icon, IconProps } from '../../src/Icon' import { Header, HeaderProps } from '../../src/Table/Header' import styles from '../../src/Table/Header.module.scss' +const Wrapper = ({ children, ...props }: { children: (props: Partial) => ReactNode } & object) => ( +
+
{children(props)}
+
+) + const headerPropsBase: HeaderProps = { index: 0, align: 'left', + role: 'columnheader', canSort: false, isSorted: false, isSortedDesc: false, canResize: false, isResizing: false, getResizerProps: jest.fn() as HeaderProps['getResizerProps'], - isLastHeader: false, } // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -24,7 +30,7 @@ const expectedContainerProps: Record = { 'data-test': 'table-header-container', className: styles.container, style: undefined, - role: undefined, + role: 'columnheader', 'aria-colspan': undefined, 'aria-sort': undefined, children: expect.anything(), @@ -78,12 +84,11 @@ describe('Header', () => { { ...expectedContentProps, className: `${styles.th} ${styles.alignRight}` }, ], [ - { ...headerPropsBase, colSpan: 1, style: { width: 40 }, className: 'testClassName', onClick, role: '' }, + { ...headerPropsBase, colSpan: 1, style: { width: 40 }, className: 'testClassName', onClick }, { ...expectedContainerProps, className: `${styles.container} testClassName`, style: { width: 40 }, - role: '', 'aria-colspan': 1, }, { ...expectedContentProps, role: 'button', tabIndex: 0, onClick, onKeyPress: expect.anything() }, @@ -93,7 +98,7 @@ describe('Header', () => { it.each(data)( 'returns div with correct props for given Header props', async (headerProps, expectedContainerProps, expectedContentProps) => { - const wrapper = await mountAndCheckA11Y(
Header
) + const wrapper = await mountAndCheckA11Y({() =>
Header
}) expect(wrapper.findDataTest('table-header-container').props()).toEqual(expectedContainerProps) expect(wrapper.findDataTest('table-header-content').props()).toEqual(expectedContentProps) @@ -102,9 +107,13 @@ describe('Header', () => { it('renders ChevronDown Icon when sorting in descending order', async () => { const wrapper = await mountAndCheckA11Y( -
- Header -
, + + {() => ( +
+ Header +
+ )} + , ) expect(wrapper.find(Icon).props()).toEqual({ @@ -117,9 +126,13 @@ describe('Header', () => { it('renders ChevronUp Icon when sorting in ascending order', async () => { const wrapper = await mountAndCheckA11Y( -
- Header -
, + + {() => ( +
+ Header +
+ )} + , ) expect(wrapper.find(Icon).props()).toEqual({ @@ -136,9 +149,13 @@ describe('Header', () => { }) const wrapper = await mountAndCheckA11Y( -
- Header -
, + + {(props) => ( +
+ Header +
+ )} + , ) expect(wrapper.findDataTest('table-header-column-resizer-container').props()).toEqual({ @@ -156,7 +173,15 @@ describe('Header', () => { }) it('draggable attribute', async () => { - const wrapper = await mountAndCheckA11Y(
Header
) + const wrapper = await mountAndCheckA11Y( + + {(props) => ( +
+ Header +
+ )} + , + ) expect(wrapper.findDataTest('table-header-content').prop('draggable')).toBeFalsy() @@ -187,9 +212,13 @@ describe('Header', () => { const onDrop = jest.fn() const setData = jest.fn() const wrapper = await mountAndCheckA11Y( -
- Header -
, + + {() => ( +
+ Header +
+ )} + , ) expect(onDragStart).toBeCalledTimes(0) diff --git a/packages/ui/__tests__/Table/Row.test.tsx b/packages/ui/__tests__/Table/Row.test.tsx index 486b225a7..5aae8a82c 100644 --- a/packages/ui/__tests__/Table/Row.test.tsx +++ b/packages/ui/__tests__/Table/Row.test.tsx @@ -27,11 +27,18 @@ describe('Row', () => { }, ], [ - { ariaRowIndex: 1, onClick: jest.fn(), style: { width: 40 }, className: 'testClassName', role: '', children: 'Row' }, + { + ariaRowIndex: 1, + onClick: jest.fn(), + style: { width: 40 }, + className: 'testClassName', + role: 'row', + children:
Row
, + }, { 'data-test': rowDataTest, className: `${styles.row} ${styles.clickable} testClassName`, - role: '', + role: 'row', 'aria-rowindex': 1, tabIndex: 0, @@ -39,13 +46,17 @@ describe('Row', () => { onKeyPress: expect.anything(), style: { width: 40 }, - children: 'Row', + children:
Row
, }, ], ] it.each(rowData)('returns
with correct props for given Row props', async (cellProps, expectedDivProps) => { - const wrapper = await mountAndCheckA11Y() + const wrapper = await mountAndCheckA11Y( +
+ +
, + ) expect(wrapper.findDataTest(rowDataTest).props()).toEqual(expectedDivProps) }) @@ -64,11 +75,18 @@ describe('Row', () => { { className: styles.link, id: '1', style: undefined, href: 'testHref', children: 'Row' }, ], [ - { ariaRowIndex: 1, href: 'testHref', style: { width: 40 }, className: 'testClassName', role: '', children: 'Row' }, + { + ariaRowIndex: 1, + href: 'testHref', + style: { width: 40 }, + className: 'testClassName', + role: 'row', + children: 'Row', + }, { 'data-test': rowDataTest, className: `${styles.linkRow} testClassName`, - role: '', + role: 'row', 'aria-rowindex': 1, 'aria-owns': '2', children: expect.anything(), @@ -77,7 +95,8 @@ describe('Row', () => { ], ] - it.each(linkRowData)( + // Blocked by https://github.com/w3c/html-aria/issues/473 + it.skip.each(linkRowData)( 'returns
with child, both with correct props for given Row props', async (cellProps, expectedOuterDivProps, expectedInnerAnchorProps) => { const wrapper = await mountAndCheckA11Y() @@ -123,30 +142,32 @@ const expectedHeaderRowElementProps: AnyProps = { 'data-test': headerRowDataTest, className: styles.headerRow, style: undefined, - role: undefined, + role: 'row', 'aria-rowindex': undefined, - children: undefined, + children: expect.anything(), } describe('HeaderRow', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const data: [PropsWithChildren, AnyProps][] = [ - [{}, expectedHeaderRowElementProps], + [{ role: 'row', children:
}, expectedHeaderRowElementProps], [ - { ariaRowIndex: 1, style: { width: 40 }, className: 'testClassName', role: '', children: 'Header Row' }, + { ariaRowIndex: 1, role: 'row', style: { width: 40 }, className: 'testClassName', children:
Header Row
}, { ...expectedHeaderRowElementProps, className: `${styles.headerRow} testClassName`, style: { width: 40 }, - role: '', 'aria-rowindex': 1, - children: 'Header Row', }, ], ] it.each(data)('returns
with correct props for given HeaderRow props', async (cellProps, expectedElementProps) => { - const wrapper = await mountAndCheckA11Y() + const wrapper = await mountAndCheckA11Y( +
+ +
, + ) expect(wrapper.findDataTest(headerRowDataTest).props()).toEqual(expectedElementProps) }) diff --git a/packages/ui/__tests__/Table/Table.test.tsx b/packages/ui/__tests__/Table/Table.test.tsx index a6a2037ad..4e88520cd 100644 --- a/packages/ui/__tests__/Table/Table.test.tsx +++ b/packages/ui/__tests__/Table/Table.test.tsx @@ -90,14 +90,13 @@ describe('Table', () => { }) }) - const cellRowGroup = table.findDataTest('table-cell-row-group') - expect(cellRowGroup.props()).toMatchObject({ - 'data-test': 'table-cell-row-group', - role: 'rowgroup', + const tableContent = table.findDataTest('table-content') + expect(tableContent.props()).toMatchObject({ + 'data-test': 'table-content', className: 'content', }) - const cellRows = cellRowGroup.find(Row) + const cellRows = tableContent.find(Row) cellRows.forEach((cellRow, i: number) => { expect(cellRow.props()).toMatchObject>({ // 1 for header row, 1 because we're indexing from 0 diff --git a/packages/ui/package.json b/packages/ui/package.json index a0344c091..257c7ddce 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -93,7 +93,7 @@ "babel-loader": "^9.1.2", "doiuse": "^4.4.1", "enzyme": "^3.11.0", - "jest-axe": "^5.0.1", + "jest-axe": "^8.0.0", "loki": "^0.32.0", "postcss": "^8.4.23", "postcss-cli": "^9.1.0", diff --git a/packages/ui/src/Checkbox.tsx b/packages/ui/src/Checkbox.tsx index 0db6b1d97..0a840ead5 100644 --- a/packages/ui/src/Checkbox.tsx +++ b/packages/ui/src/Checkbox.tsx @@ -1,4 +1,4 @@ -import React, { AriaAttributes, ChangeEvent, FC, FocusEvent, ReactElement, ReactText, useRef, MouseEvent } from 'react' +import React, { ChangeEvent, FC, FocusEvent, ReactElement, ReactText, useRef, MouseEvent } from 'react' import { Check, Minus } from 'react-feather' import { DataTestProp } from '@hazelcast/helpers' import { useUID } from 'react-uid' @@ -65,11 +65,6 @@ export const Checkbox: FC = (props) => { const inputRef = useRef(null) const id = useUID() - let ariaChecked: AriaAttributes['aria-checked'] = checked ? 'true' : 'false' - if (indeterminate) { - ariaChecked = 'mixed' - } - return (
{/* @@ -104,7 +99,6 @@ export const Checkbox: FC = (props) => { onClick={onClick} value={value} disabled={disabled} - aria-checked={ariaChecked} aria-invalid={!!error} aria-required={required} aria-describedby={helperText && helpTooltipId(id)} diff --git a/packages/ui/src/Table/Table.tsx b/packages/ui/src/Table/Table.tsx index ba9dd5c53..a2f70ebda 100644 --- a/packages/ui/src/Table/Table.tsx +++ b/packages/ui/src/Table/Table.tsx @@ -307,6 +307,7 @@ export const Table = ({ width: 60, disableResizing: true, id: ROW_EXPANDER_COLUMN_ID, + Header: () => , Cell: ({ row }: { row: RowType }) => { return row.canExpand || renderRowSubComponent ? (
@@ -537,7 +538,13 @@ export const Table = ({ ) : hasData ? ( - + (props: TableContentProps) => { }, []) return ( - // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions + // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions,jsx-a11y/no-static-element-interactions
{ selectionStartedRef.current = true diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3d63583b8..6e108c632 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -178,11 +178,11 @@ importers: specifier: ^1.3.1 version: link:../services '@types/jest-axe': - specifier: ^3.5.3 - version: 3.5.3 + specifier: ^3.5.8 + version: 3.5.8 jest-axe: - specifier: ^5.0.1 - version: 5.0.1 + specifier: ^8.0.0 + version: 8.0.0 devDependencies: '@types/enzyme': specifier: ^3.10.12 @@ -345,8 +345,8 @@ importers: specifier: ^3.11.0 version: 3.11.0 jest-axe: - specifier: ^5.0.1 - version: 5.0.1 + specifier: ^8.0.0 + version: 8.0.0 loki: specifier: ^0.32.0 version: 0.32.0(@storybook/addons@6.5.16)(@storybook/react@6.5.16)(@types/react@17.0.45) @@ -4968,12 +4968,21 @@ packages: dependencies: '@types/jest': 27.0.3 axe-core: 3.5.6 + dev: true + + /@types/jest-axe@3.5.8: + resolution: {integrity: sha512-KlwSkM932uxMevwx3YHtZYXkhs2wXgw9GxmsDVcuetPC4D5sUrSP2pCjS8524vwsOfZvFZpY+1USCgZCzlT+cA==} + dependencies: + '@types/jest': 29.5.1 + axe-core: 3.5.6 + dev: false /@types/jest@27.0.3: resolution: {integrity: sha512-cmmwv9t7gBYt7hNKH5Spu7Kuu/DotGa+Ff+JGRKZ4db5eh8PnKS4LuebJ3YLUoyOyIHraTGyULn23YtEAm0VSg==} dependencies: jest-diff: 27.4.2 pretty-format: 27.4.2 + dev: true /@types/jest@29.5.1: resolution: {integrity: sha512-tEuVcHrpaixS36w7hpsfLBLpjtMRJUE09/MHXn923LOVojDwyC14cWcfc0rDs0VEfUyYmt/+iX1kxxp+gZMcaQ==} @@ -6374,6 +6383,10 @@ packages: engines: {node: '>=4'} dev: true + /axe-core@4.7.2: + resolution: {integrity: sha512-zIURGIS1E1Q4pcrMjp+nnEh+16G56eG/MUllJH8yEvw7asDo7Ac9uhC9KIH5jzpITueEZolfYglnCGIuSBz39g==} + engines: {node: '>=4'} + /axios@0.21.4(debug@4.3.4): resolution: {integrity: sha512-ut5vewkiu8jjGBdqpM44XxjuCjq9LAKeHVmoVfHVzy8eHgxxq8SbAVQNovDA8mVi05kP0Ea/n/UzcSHcTJQfNg==} dependencies: @@ -11751,13 +11764,13 @@ packages: minimatch: 3.1.2 dev: true - /jest-axe@5.0.1: - resolution: {integrity: sha512-MMOWA6gT4pcZGbTLS8ZEqABH08Lnj5bInfLPpn9ADWX2wFF++odbbh8csmSfkwKjHaioVPzlCtIypAtxFDx/rw==} - engines: {node: '>= 10.0.0'} + /jest-axe@8.0.0: + resolution: {integrity: sha512-4kNcNn7J0jPO4jANEYZOHeQ/tSBvkXS+MxTbX1CKbXGd0+ZbRGDn/v/8IYWI/MmYX15iLVyYRnRev9X3ksePWA==} + engines: {node: '>= 14.0.0'} dependencies: - axe-core: 4.2.1 - chalk: 4.1.0 - jest-matcher-utils: 27.0.2 + axe-core: 4.7.2 + chalk: 4.1.2 + jest-matcher-utils: 29.2.2 lodash.merge: 4.6.2 /jest-changed-files@29.5.0: @@ -12004,24 +12017,23 @@ packages: pretty-format: 29.5.0 dev: true - /jest-matcher-utils@27.0.2: - resolution: {integrity: sha512-Qczi5xnTNjkhcIB0Yy75Txt+Ez51xdhOxsukN7awzq2auZQGPHcQrJ623PZj0ECDEMOk2soxWx05EXdXGd1CbA==} - engines: {node: ^10.13.0 || ^12.13.0 || ^14.15.0 || >=15.0.0} + /jest-matcher-utils@29.2.2: + resolution: {integrity: sha512-4DkJ1sDPT+UX2MR7Y3od6KtvRi9Im1ZGLGgdLFLm4lPexbTaCgJW5NN3IOXlQHF7NSHY/VHhflQ+WoKtD/vyCw==} + engines: {node: ^14.15.0 || ^16.10.0 || >=18.0.0} dependencies: chalk: 4.1.2 - jest-diff: 27.4.2 - jest-get-type: 27.4.0 - pretty-format: 27.4.2 + jest-diff: 29.5.0 + jest-get-type: 29.4.3 + pretty-format: 29.5.0 /jest-matcher-utils@29.4.3: resolution: {integrity: sha512-TTciiXEONycZ03h6R6pYiZlSkvYgT0l8aa49z/DLSGYjex4orMUcafuLXYyyEDWB1RKglq00jzwY00Ei7yFNVg==} engines: {node: ^14.15.0 || ^16.10.0 || >=18.0.0} dependencies: chalk: 4.1.2 - jest-diff: 29.4.3 + jest-diff: 29.5.0 jest-get-type: 29.4.3 pretty-format: 29.5.0 - dev: true /jest-matcher-utils@29.5.0: resolution: {integrity: sha512-lecRtgm/rjIK0CQ7LPQwzCs2VwW6WAahA55YBuI+xqmhm7LAaxokSB8C97yJeYyT+HvQkH741StzpU41wohhWw==} From 32bcd4126c57463c5d0f7284fa07955d4dfbea52 Mon Sep 17 00:00:00 2001 From: Yevhen Martynenko Date: Tue, 14 Nov 2023 08:53:02 +0200 Subject: [PATCH 2/2] *fix a11y tests --- packages/ui/src/Checkbox.module.scss | 13 ++++++++----- packages/ui/src/Checkbox.tsx | 1 + 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/ui/src/Checkbox.module.scss b/packages/ui/src/Checkbox.module.scss index 283900b6e..d4aafc2d0 100644 --- a/packages/ui/src/Checkbox.module.scss +++ b/packages/ui/src/Checkbox.module.scss @@ -48,6 +48,14 @@ border-color: c.$colorSuccess; } + /** + Indeterminate + */ + &.indeterminate input ~ .checkmark { + background-color: c.$colorPrimaryLight; + border-color: c.$colorPrimaryLight; + } + /** Hover */ @@ -58,11 +66,6 @@ } } - input[aria-checked='mixed'] ~ .checkmark { - background-color: c.$colorPrimaryLight; - border-color: c.$colorPrimaryLight; - } - // Error state &.error input ~ .checkmark { border-color: c.$colorError; diff --git a/packages/ui/src/Checkbox.tsx b/packages/ui/src/Checkbox.tsx index 0a840ead5..387b851da 100644 --- a/packages/ui/src/Checkbox.tsx +++ b/packages/ui/src/Checkbox.tsx @@ -78,6 +78,7 @@ export const Checkbox: FC = (props) => { [styles.error]: !!error, [styles.checked]: checked, [styles.disabled]: disabled, + [styles.indeterminate]: indeterminate, }, classNameLabel, )}