diff --git a/src/ActionList2/Header.tsx b/src/ActionList2/Header.tsx deleted file mode 100644 index d04fce24df0..00000000000 --- a/src/ActionList2/Header.tsx +++ /dev/null @@ -1,58 +0,0 @@ -import React from 'react' -import Box from '../Box' -import {SxProp} from '../sx' -import {ListContext} from './List' - -/** - * Contract for props passed to the `Header` component. - */ -export type HeaderProps = { - /** - * Style variations. Usage is discretionary. - * - * - `"filled"` - Superimposed on a background, offset from nearby content - * - `"subtle"` - Relatively less offset from nearby content - */ - variant?: 'subtle' | 'filled' - - /** - * Primary text which names a `Group`. - */ - title?: string - - /** - * Secondary text which provides additional information about a `Group`. - */ - auxiliaryText?: string -} & SxProp - -/** - * Displays the name and description of a `Group`. - */ -export const Header = ({variant = 'subtle', title, auxiliaryText, sx = {}, ...props}: HeaderProps): JSX.Element => { - const {variant: listVariant} = React.useContext(ListContext) - - const styles = { - paddingY: '6px', - paddingX: listVariant === 'full' ? 2 : 3, - fontSize: 0, - fontWeight: 'bold', - color: 'fg.muted', - ...(variant === 'filled' && { - backgroundColor: 'canvas.subtle', - marginX: 0, - marginBottom: 2, - borderTop: '1px solid', - borderBottom: '1px solid', - borderColor: 'neutral.muted' - }), - ...sx - } - - return ( - - {title} - {auxiliaryText && {auxiliaryText}} - - ) -} diff --git a/src/ActionList2/Item.tsx b/src/ActionList2/Item.tsx index 2270e5367e0..d33d69fec03 100644 --- a/src/ActionList2/Item.tsx +++ b/src/ActionList2/Item.tsx @@ -91,7 +91,7 @@ export const Item = React.forwardRef( variant = 'default', disabled = false, selected = undefined, - onSelect = () => null, + onSelect, sx: sxProp = {}, id, _PrivateItemWrapper, @@ -168,6 +168,7 @@ export const Item = React.forwardRef( const clickHandler = React.useCallback( event => { + if (typeof onSelect !== 'function') return if (disabled) return if (!event.defaultPrevented) onSelect(event) }, @@ -176,8 +177,8 @@ export const Item = React.forwardRef( const keyPressHandler = React.useCallback( event => { + if (typeof onSelect !== 'function') return if (disabled) return - if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) { onSelect(event) } diff --git a/src/__tests__/ActionList2.test.tsx b/src/__tests__/ActionList2.test.tsx index 0ffb5e0b4c7..af6a5a1b66f 100644 --- a/src/__tests__/ActionList2.test.tsx +++ b/src/__tests__/ActionList2.test.tsx @@ -1,4 +1,4 @@ -import {cleanup, render as HTMLRender} from '@testing-library/react' +import {cleanup, render as HTMLRender, waitFor, fireEvent} from '@testing-library/react' import 'babel-polyfill' import {axe, toHaveNoViolations} from 'jest-axe' import React from 'react' @@ -6,6 +6,7 @@ import theme from '../theme' import {ActionList} from '../ActionList2' import {behavesAsComponent, checkExports, checkStoriesForAxeViolations} from '../utils/testing' import {BaseStyles, ThemeProvider, SSRProvider} from '..' +import '@testing-library/jest-dom' expect.extend(toHaveNoViolations) function SimpleActionList(): JSX.Element { @@ -26,6 +27,31 @@ function SimpleActionList(): JSX.Element { ) } +const projects = [ + {name: 'Primer Backlog', scope: 'GitHub'}, + {name: 'Primer React', scope: 'github/primer'}, + {name: 'Disabled Project', scope: 'github/primer', disabled: true} +] +function SingleSelectListStory(): JSX.Element { + const [selectedIndex, setSelectedIndex] = React.useState(0) + + return ( + + {projects.map((project, index) => ( + setSelectedIndex(index)} + disabled={project.disabled} + > + {project.name} + + ))} + + ) +} + describe('ActionList', () => { behavesAsComponent({ Component: ActionList, @@ -45,5 +71,86 @@ describe('ActionList', () => { cleanup() }) + it('should fire onSelect on click and keypress', async () => { + const component = HTMLRender() + const options = await waitFor(() => component.getAllByRole('option')) + + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[1]).toHaveAttribute('aria-selected', 'false') + + fireEvent.click(options[1]) + + expect(options[0]).toHaveAttribute('aria-selected', 'false') + expect(options[1]).toHaveAttribute('aria-selected', 'true') + + // We pass keycode here to navigate a implementation detail in react-testing-library + // https://github.com/testing-library/react-testing-library/issues/269#issuecomment-455854112 + fireEvent.keyPress(options[0], {key: 'Enter', charCode: 13}) + + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[1]).toHaveAttribute('aria-selected', 'false') + + fireEvent.keyPress(options[1], {key: ' ', charCode: 32}) + + expect(options[0]).toHaveAttribute('aria-selected', 'false') + expect(options[1]).toHaveAttribute('aria-selected', 'true') + + cleanup() + }) + + it('should skip onSelect on disabled items', async () => { + const component = HTMLRender() + const options = await waitFor(() => component.getAllByRole('option')) + + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[2]).toHaveAttribute('aria-selected', 'false') + + fireEvent.click(options[2]) + + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[2]).toHaveAttribute('aria-selected', 'false') + + fireEvent.keyPress(options[2], {key: 'Enter', charCode: 13}) + + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[2]).toHaveAttribute('aria-selected', 'false') + + cleanup() + }) + + it('should throw when selected is provided without a selectionVariant on parent', async () => { + // we expect console.error to be called, so we suppress that in the test + const mockError = jest.spyOn(console, 'error').mockImplementation(() => jest.fn()) + + expect(() => { + HTMLRender( + + + Primer React + + + ) + }).toThrow('For Item to be selected, ActionList or ActionList.Group needs to have a selectionVariant defined') + + cleanup() + mockError.mockRestore() + }) + + it('should not crash when clicking an item without an onSelect', async () => { + const component = HTMLRender( + + Primer React + + ) + const option = await waitFor(() => component.getByRole('option')) + expect(option).toBeInTheDocument() + + fireEvent.click(option) + fireEvent.keyPress(option, {key: 'Enter', charCode: 13}) + expect(option).toBeInTheDocument() + + cleanup() + }) + checkStoriesForAxeViolations('ActionList2') }) diff --git a/src/stories/ActionList2.stories.tsx b/src/stories/ActionList2.stories.tsx index ad44f827345..92d97dbb200 100644 --- a/src/stories/ActionList2.stories.tsx +++ b/src/stories/ActionList2.stories.tsx @@ -801,7 +801,7 @@ export function AllCombinations(): JSX.Element { L + I + Binline description Block description - + @@ -810,7 +810,7 @@ export function AllCombinations(): JSX.Element { - + @@ -819,7 +819,7 @@ export function AllCombinations(): JSX.Element { - + I + B + Tinline description Block description diff --git a/src/utils/testing.tsx b/src/utils/testing.tsx index 46b887b784e..23b6ded773c 100644 --- a/src/utils/testing.tsx +++ b/src/utils/testing.tsx @@ -5,6 +5,7 @@ import enzyme from 'enzyme' import Adapter from '@wojtekmaj/enzyme-adapter-react-17' import {cleanup, render as HTMLRender} from '@testing-library/react' import {axe, toHaveNoViolations} from 'jest-axe' +import type {Story as StoryType} from '@storybook/react' import {ThemeProvider} from '..' import {default as defaultTheme} from '../theme' @@ -253,7 +254,7 @@ export function checkStoriesForAxeViolations(name: string) { Object.values(Stories).map(Story => { if (typeof Story !== 'function') return - it(`story {Story.storyName} should have no axe violations`, async () => { + it(`story ${(Story as StoryType).storyName} should have no axe violations`, async () => { const {container} = HTMLRender() const results = await axe(container) expect(results).toHaveNoViolations()