Skip to content

Commit

Permalink
Merge pull request #1176 from primer/VanAnderson/action-list-fix
Browse files Browse the repository at this point in the history
Clean up test warnings
  • Loading branch information
smockle committed Apr 21, 2021
2 parents 26a9f2d + d6f18c9 commit 2150961
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 51 deletions.
13 changes: 8 additions & 5 deletions docs/content/ActionList.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -30,30 +30,33 @@ An `ActionList` is a list of items which can be activated or selected. `ActionLi
{groupId: '4'}
]}
items={[
{leadingVisual: TypographyIcon, text: 'Rename', groupId: '0'},
{leadingVisual: VersionsIcon, text: 'Duplicate', groupId: '0'},
{leadingVisual: SearchIcon, text: 'repo:github/memex,github/github', groupId: '1'},
{key: '1', leadingVisual: TypographyIcon, text: 'Rename', groupId: '0'},
{key: '2', leadingVisual: VersionsIcon, text: 'Duplicate', groupId: '0'},
{key: '3', leadingVisual: SearchIcon, text: 'repo:github/github', groupId: '1'},
{
key: '4',
leadingVisual: NoteIcon,
text: 'Table',
description: 'Information-dense table optimized for operations across teams',
descriptionVariant: 'block',
groupId: '2'
},
{
key: '5',
leadingVisual: ProjectIcon,
text: 'Board',
description: 'Kanban-style board focused on visual states',
descriptionVariant: 'block',
groupId: '2'
},
{
key: '6',
leadingVisual: FilterIcon,
text: 'Save sort and filters to current view',
groupId: '3'
},
{leadingVisual: FilterIcon, text: 'Save sort and filters to new view', groupId: '3'},
{leadingVisual: GearIcon, text: 'View settings', groupId: '4'}
{key: '7', leadingVisual: FilterIcon, text: 'Save sort and filters to new view', groupId: '3'},
{key: '8', leadingVisual: GearIcon, text: 'View settings', groupId: '4'}
]}
/>
```
Expand Down
21 changes: 12 additions & 9 deletions docs/content/ActionMenu.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ An `ActionMenu` is an ActionList-based component for creating a menu of actions
anchorContent="Menu"
onAction={({text}) => console.log(text)}
items={[
{text: 'New file'},
{text: 'New file', key: 'new-file'},
ActionMenu.Divider,
{text: 'Copy link'},
{text: 'Edit file'},
{text: 'Delete file', variant: 'danger'}
{text: 'Copy link', key: 'copy-link'},
{text: 'Edit file', key: 'edit-file'},
{text: 'Delete file', variant: 'danger', key: 'delete-file'}
]}
/>
```
Expand All @@ -34,30 +34,33 @@ An `ActionMenu` is an ActionList-based component for creating a menu of actions
{groupId: '4'}
]}
items={[
{leadingVisual: TypographyIcon, text: 'Rename', groupId: '0'},
{leadingVisual: VersionsIcon, text: 'Duplicate', groupId: '0'},
{leadingVisual: SearchIcon, text: 'repo:github/github', groupId: '1'},
{key: '1', leadingVisual: TypographyIcon, text: 'Rename', groupId: '0'},
{key: '2', leadingVisual: VersionsIcon, text: 'Duplicate', groupId: '0'},
{key: '3', leadingVisual: SearchIcon, text: 'repo:github/github', groupId: '1'},
{
key: '4',
leadingVisual: NoteIcon,
text: 'Table',
description: 'Information-dense table optimized for operations across teams',
descriptionVariant: 'block',
groupId: '2'
},
{
key: '5',
leadingVisual: ProjectIcon,
text: 'Board',
description: 'Kanban-style board focused on visual states',
descriptionVariant: 'block',
groupId: '2'
},
{
key: '6',
leadingVisual: FilterIcon,
text: 'Save sort and filters to current view',
groupId: '3'
},
{leadingVisual: FilterIcon, text: 'Save sort and filters to new view', groupId: '3'},
{leadingVisual: GearIcon, text: 'View settings', groupId: '4'}
{key: '7', leadingVisual: FilterIcon, text: 'Save sort and filters to new view', groupId: '3'},
{key: '8', leadingVisual: GearIcon, text: 'View settings', groupId: '4'}
]}
/>
```
Expand Down
29 changes: 18 additions & 11 deletions docs/content/DropdownMenu.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,20 @@
title: DropdownMenu
---

A `DropdownMenu` provides an anchor (button by default) that will open a floating menu of selectable items. The menu can be opened and navigated using keyboard or mouse. When an item is selected, the menu will close and the `onChange` callback will be called. If the default anchor button is used, the anchor contents will be updated with the selection.
A `DropdownMenu` provides an anchor (button by default) that will open a floating menu of selectable items. The menu can be opened and navigated using keyboard or mouse. When an item is selected, the menu will close and the `onChange` callback will be called. If the default anchor button is used, the anchor contents will be updated with the selection.

## Example

```javascript live noinline
function DemoComponent() {
const items = React.useMemo(() => [{text: '🔵 Cyan', id: 5}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}], [])
const items = React.useMemo(
() => [
{text: '🔵 Cyan', id: 5, key: 'cyan'},
{text: '🔴 Magenta', key: 'magenta'},
{text: '🟡 Yellow', key: 'yellow'}
],
[]
)
const [selectedItem, setSelectedItem] = React.useState()

return (
Expand All @@ -31,12 +38,12 @@ render(<DemoComponent />)

## Component props

| Name | Type | Default | Description |
| :--------------- | :-------------------------------------------- | :---------------: | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| items | `ItemProps[]` | `undefined` | Required. A list of item objects to display in the menu |
| selectedItem | `ItemInput` | `undefined` | An `ItemProps` item from the list of `items` which is currently selected. This item will receive a checkmark next to it in the menu. |
| onChange? | (item?: ItemInput) => unknown | `undefined` | A callback which receives the selected item or `undefined` when an item is activated in the menu. If the activated item is the same as the current `selectedItem`, `undefined` will be passed. |
| placeholder | `string` | `undefined` | Optional. A placeholder value to display when there is no current selection. |
| renderAnchor | `(props: DropdownButtonProps) => JSX.Element` | `DropdownButton` | Optional. If defined, provided component will be used to render the menu anchor. Will receive the selected `Item` text as `children` prop when an item is activated. |
| renderItem | `(props: ItemProps) => JSX.Element` | `ActionList.Item` | Optional. If defined, each item in `items` will be passed to this function, allowing for custom item rendering. |
| groupMetadata | `GroupProps[]` | `undefined` | Optional. If defined, `DropdownMenu` will group `items` into `ActionList.Group`s separated by `ActionList.Divider` according to their `groupId` property. |
| Name | Type | Default | Description |
| :------------ | :-------------------------------------------- | :---------------: | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| items | `ItemProps[]` | `undefined` | Required. A list of item objects to display in the menu |
| selectedItem | `ItemInput` | `undefined` | An `ItemProps` item from the list of `items` which is currently selected. This item will receive a checkmark next to it in the menu. |
| onChange? | (item?: ItemInput) => unknown | `undefined` | A callback which receives the selected item or `undefined` when an item is activated in the menu. If the activated item is the same as the current `selectedItem`, `undefined` will be passed. |
| placeholder | `string` | `undefined` | Optional. A placeholder value to display when there is no current selection. |
| renderAnchor | `(props: DropdownButtonProps) => JSX.Element` | `DropdownButton` | Optional. If defined, provided component will be used to render the menu anchor. Will receive the selected `Item` text as `children` prop when an item is activated. |
| renderItem | `(props: ItemProps) => JSX.Element` | `ActionList.Item` | Optional. If defined, each item in `items` will be passed to this function, allowing for custom item rendering. |
| groupMetadata | `GroupProps[]` | `undefined` | Optional. If defined, `DropdownMenu` will group `items` into `ActionList.Group`s separated by `ActionList.Divider` according to their `groupId` property. |
5 changes: 5 additions & 0 deletions src/ActionList/Group.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ export interface GroupProps extends React.ComponentPropsWithoutRef<'div'>, SxPro
*/
header?: HeaderProps

/**
* The id of the group.
*/
groupId?: string

/**
* `Items` to render in the `Group`.
*/
Expand Down
32 changes: 20 additions & 12 deletions src/ActionList/List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {Divider} from './Divider'
import styled from 'styled-components'
import {get} from '../constants'
import {SystemCssProperties} from '@styled-system/css'
import {uniqueId} from '../utils/uniqueId'

export type ItemInput = ItemProps | (Partial<ItemProps> & {renderItem: typeof Item})

Expand Down Expand Up @@ -124,19 +125,27 @@ export function List(props: ListProps): JSX.Element {
*/
const renderGroup = (
groupProps: GroupProps | (Partial<GroupProps> & {renderItem?: typeof Item; renderGroup?: typeof Group})
) => ((('renderGroup' in groupProps && groupProps.renderGroup) ?? props.renderGroup) || Group).call(null, groupProps)
) => {
const GroupComponent = (('renderGroup' in groupProps && groupProps.renderGroup) ?? props.renderGroup) || Group
return <GroupComponent {...groupProps} key={groupProps.groupId} />
}

/**
* Render an `Item` using the first of the following renderers that is defined:
* An `Item`-level, `Group`-level, or `List`-level custom `Item` renderer,
* or the default `Item` renderer.
*/
const renderItem = (itemProps: ItemInput, item: ItemInput) =>
(('renderItem' in itemProps && itemProps.renderItem) || props.renderItem || Item).call(null, {
...itemProps,
sx: {...itemStyle, ...itemProps.sx},
item
})
const renderItem = (itemProps: ItemInput, item: ItemInput) => {
const ItemComponent = ('renderItem' in itemProps && itemProps.renderItem) || props.renderItem || Item
return (
<ItemComponent
{...itemProps}
key={itemProps.key || uniqueId()}
sx={{...itemStyle, ...itemProps.sx}}
item={item}
/>
)
}

/**
* An array of `Group`s, each with an associated `Header` and with an array of `Item`s belonging to that `Group`.
Expand All @@ -147,7 +156,7 @@ export function List(props: ListProps): JSX.Element {

if (!isGroupedListProps(props)) {
// When no `groupMetadata`s is provided, collect rendered `Item`s into a single anonymous `Group`.
groups = [{items: props.items?.map(item => renderItem(item, item))}]
groups = [{items: props.items?.map(item => renderItem(item, item)), groupId: uniqueId()}]
} else {
// When `groupMetadata` is provided, collect rendered `Item`s into their associated `Group`s.

Expand Down Expand Up @@ -185,9 +194,8 @@ export function List(props: ListProps): JSX.Element {
return (
<StyledList {...props}>
{groups?.map(({header, ...groupProps}, index) => (
<>
<React.Fragment key={groupProps.groupId}>
{renderGroup({
key: index,
sx: {
...(index === 0 && firstGroupStyle),
...(index === groups.length - 1 && lastGroupStyle)
Expand All @@ -200,8 +208,8 @@ export function List(props: ListProps): JSX.Element {
}),
...groupProps
})}
{index + 1 !== groups.length && <Divider />}
</>
{index + 1 !== groups.length && <Divider key={`${groupProps.groupId}-divider`} />}
</React.Fragment>
))}
</StyledList>
)
Expand Down
30 changes: 22 additions & 8 deletions src/__tests__/ActionMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {cleanup, render as HTMLRender} from '@testing-library/react'
import {cleanup, render as HTMLRender, act, fireEvent} from '@testing-library/react'
import 'babel-polyfill'
import {axe, toHaveNoViolations} from 'jest-axe'
import React from 'react'
Expand Down Expand Up @@ -55,7 +55,9 @@ describe('ActionMenu', () => {
let portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeNull()
const anchor = await menu.findByText('Menu')
await anchor.click()
act(() => {
fireEvent.click(anchor)
})
portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeTruthy()
const itemText = items
Expand All @@ -71,11 +73,15 @@ describe('ActionMenu', () => {
let portalRoot = await menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeNull()
const anchor = await menu.findByText('Menu')
await anchor.click()
act(() => {
fireEvent.click(anchor)
})
portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeTruthy()
const menuItem = await menu.queryByText(items[0].text)
menuItem?.click()
act(() => {
fireEvent.click(menuItem as Element)
})
expect(portalRoot?.textContent).toEqual('') // menu items are hidden
})

Expand All @@ -84,11 +90,15 @@ describe('ActionMenu', () => {
let portalRoot = await menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeNull()
const anchor = await menu.findByText('Menu')
await anchor.click()
act(() => {
fireEvent.click(anchor)
})
portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeTruthy()
const somethingElse = (await menu.baseElement.querySelector('#something-else')) as HTMLElement
await somethingElse?.click()
act(() => {
fireEvent.click(somethingElse)
})
expect(portalRoot?.textContent).toEqual('') // menu items are hidden
})

Expand All @@ -97,11 +107,15 @@ describe('ActionMenu', () => {
let portalRoot = await menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeNull()
const anchor = await menu.findByText('Menu')
await anchor.click()
act(() => {
fireEvent.click(anchor)
})
portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeTruthy()
const menuItem = (await portalRoot?.querySelector("[role='menuitem']")) as HTMLElement
await menuItem?.click()
act(() => {
fireEvent.click(menuItem)
})
// onAction has been called with correct argument
expect(mockOnActivate).toHaveBeenCalledTimes(1)
const arg = mockOnActivate.mock.calls[0][0]
Expand Down
20 changes: 15 additions & 5 deletions src/__tests__/DropdownMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ describe('DropdownMenu', () => {
let portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeNull()
const anchor = await menu.findByText('Select an Option')
await anchor?.click()
act(() => {
fireEvent.click(anchor)
})
portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeTruthy()
const itemText = items
Expand All @@ -73,7 +75,9 @@ describe('DropdownMenu', () => {
let portalRoot = await menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeNull()
const anchor = await menu.findByText('Select an Option')
await anchor.click()
act(() => {
fireEvent.click(anchor)
})
portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeTruthy()
const menuItem = await menu.queryByText('Baz')
Expand All @@ -89,7 +93,9 @@ describe('DropdownMenu', () => {
let portalRoot = await menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeNull()
const anchor = await menu.findByText('Select an Option')
await anchor.click()
act(() => {
fireEvent.click(anchor)
})
portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeTruthy()
const menuItem = await menu.queryByText('Baz')
Expand All @@ -104,11 +110,15 @@ describe('DropdownMenu', () => {
let portalRoot = await menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeNull()
const anchor = await menu.findByText('Select an Option')
await anchor.click()
act(() => {
fireEvent.click(anchor)
})
portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__')
expect(portalRoot).toBeTruthy()
const somethingElse = (await menu.baseElement.querySelector('#something-else')) as HTMLElement
await somethingElse?.click()
act(() => {
fireEvent.click(somethingElse)
})
// portal is closed after click
expect(portalRoot?.textContent).toEqual('') // menu items are hidden
})
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/FormGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('FormGroup', () => {
const {container} = HTMLRender(
<FormGroup>
<FormGroup.Label htmlFor="example-text">Example text</FormGroup.Label>
<input id="example-text" value="Example Value" />
<input id="example-text" value="Example Value" readOnly />
</FormGroup>
)
const results = await axe(container)
Expand Down

1 comment on commit 2150961

@vercel
Copy link

@vercel vercel bot commented on 2150961 Apr 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.