Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Action Menu can have its open state be controlled externally #1199

Merged
merged 12 commits into from
May 5, 2021
5 changes: 5 additions & 0 deletions .changeset/pink-lions-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/components": patch
---

Action Menu can have its open state be controlled externally.
18 changes: 10 additions & 8 deletions docs/content/ActionMenu.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,13 @@ An `ActionMenu` is an ActionList-based component for creating a menu of actions

## Component props

| Name | Type | Default | Description |
| :------------ | :------------------------------------ | :---------------: | :------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| items | `ItemProps[]` | `undefined` | Required. A list of item objects conforming to the `ActionList.Item` props interface. |
| renderItem | `(props: ItemProps) => JSX.Element` | `ActionList.Item` | Optional. If defined, each item in `items` will be passed to this function, allowing for `ActionList`-wide custom item rendering. |
| groupMetadata | `GroupProps[]` | `undefined` | Optional. If defined, `ActionList` will group `items` into `ActionList.Group`s separated by `ActionList.Divider` according to their `groupId` property. |
| renderAnchor | `(props: ButtonProps) => JSX.Element` | `Button` | 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. |
| anchorContent | React.ReactNode | `undefined` | Optional. If defined, it will be passed to the trigger as the elements child. |
| onAction | (props: ItemProps) => void | `undefined` | Optional. If defined, this function will be called when a menu item is activated either by a click or a keyboard press. |
| Name | Type | Default | Description |
| :------------ | :------------------------------------ | :---------------: | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| items | `ItemProps[]` | `undefined` | Required. A list of item objects conforming to the `ActionList.Item` props interface. |
| renderItem | `(props: ItemProps) => JSX.Element` | `ActionList.Item` | Optional. If defined, each item in `items` will be passed to this function, allowing for `ActionList`-wide custom item rendering. |
| groupMetadata | `GroupProps[]` | `undefined` | Optional. If defined, `ActionList` will group `items` into `ActionList.Group`s separated by `ActionList.Divider` according to their `groupId` property. |
| renderAnchor | `(props: ButtonProps) => JSX.Element` | `Button` | 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. |
| anchorContent | React.ReactNode | `undefined` | Optional. If defined, it will be passed to the trigger as the elements child. |
| onAction | (props: ItemProps) => void | `undefined` | Optional. If defined, this function will be called when a menu item is activated either by a click or a keyboard press. |
| open | boolean | `undefined` | Optional. If defined, ActionMenu will use this to control the open/closed state of the Overlay instead of controlling the state internally. Should be used in conjunction with the `setOpen` prop. |
| setOpen | (state: boolean) => void | `undefined` | Optional. If defined, ActionMenu will use this to control the open/closed state of the Overlay instead of controlling the state internally. Should be used in conjunction with the `open` prop. |
19 changes: 12 additions & 7 deletions src/ActionMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ import {GroupedListProps, List, ListPropsBase} from './ActionList/List'
import {Item, ItemProps} from './ActionList/Item'
import {Divider} from './ActionList/Divider'
import Button, {ButtonProps} from './Button'
import React, {useCallback, useEffect, useRef, useState} from 'react'
import React, {useCallback, useEffect, useRef} from 'react'
import {AnchoredOverlay} from './AnchoredOverlay'

import {useProvidedStateOrCreate} from './hooks/useProvidedStateOrCreate'
export interface ActionMenuProps extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>, ListPropsBase {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
renderAnchor?: (props: any) => JSX.Element
anchorContent?: React.ReactNode
onAction?: (props: ItemProps, event: React.MouseEvent<HTMLDivElement> | React.KeyboardEvent<HTMLDivElement>) => void
onAction?: (props: ItemProps, event?: React.MouseEvent<HTMLDivElement> | React.KeyboardEvent<HTMLDivElement>) => void
open?: boolean
setOpen?: (s: boolean) => void
}

const ActionMenuItem = (props: ItemProps) => <Item role="menuitem" {...props} />
Expand All @@ -21,12 +23,14 @@ const ActionMenuBase = ({
renderAnchor = <T extends ButtonProps>(props: T) => <Button {...props} />,
renderItem = Item,
onAction,
open,
setOpen,
...listProps
}: ActionMenuProps): JSX.Element => {
const [open, setOpen] = useState(false)
const onOpen = useCallback(() => setOpen(true), [])
const onClose = useCallback(() => setOpen(false), [])
const pendingActionRef = useRef<() => unknown>()
const [combinedOpenState, setCombinedOpenState] = useProvidedStateOrCreate(open, setOpen, false)
const onOpen = useCallback(() => setCombinedOpenState(true), [setCombinedOpenState])
const onClose = useCallback(() => setCombinedOpenState(false), [setCombinedOpenState])

const renderMenuAnchor = useCallback(
<T extends React.HTMLAttributes<HTMLElement>>(props: T) => {
Expand All @@ -47,6 +51,7 @@ const ActionMenuBase = ({
onAction: (props, event) => {
const actionCallback = itemOnAction ?? onAction
pendingActionRef.current = () => actionCallback?.(props as ItemProps, event)
actionCallback?.(props as ItemProps, event)
onClose()
}
})
Expand All @@ -64,7 +69,7 @@ const ActionMenuBase = ({
}, [open])

return (
<AnchoredOverlay renderAnchor={renderMenuAnchor} open={open} onOpen={onOpen} onClose={onClose}>
<AnchoredOverlay renderAnchor={renderMenuAnchor} open={!!combinedOpenState} onOpen={onOpen} onClose={onClose}>
dgreif marked this conversation as resolved.
Show resolved Hide resolved
<List {...listProps} role="menu" renderItem={renderMenuItem} />
</AnchoredOverlay>
)
Expand Down
39 changes: 39 additions & 0 deletions src/__tests__/hooks/useProvidedStateOrCreate.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import {useProvidedStateOrCreate} from '../../hooks/useProvidedStateOrCreate'
import {render} from '@testing-library/react'
import React, {useState} from 'react'

it('will use the provided state', () => {
const Component = () => {
const [state, setState] = useState('foo')
const [combinedState] = useProvidedStateOrCreate(state, setState, 'bar')
return <div>{combinedState}</div>
}

const doc = render(<Component />)
expect(doc.baseElement.textContent).toEqual('foo')
})

it('will set state correctly when provided a set state method', () => {
const Component = () => {
const [state, setState] = useState('foo')
const [combinedState, setCombinedState] = useProvidedStateOrCreate(state, setState, 'bar')
if (combinedState !== 'baz') setCombinedState('baz')
return <div>{combinedState}</div>
}

const doc = render(<Component />)
expect(doc.baseElement.textContent).toEqual('baz')
})

it('if not provided a state, will use an internal state', () => {
const Component = () => {
const state = undefined
const setState = undefined
const [combinedState, setCombinedState] = useProvidedStateOrCreate(state, setState, '')
if (combinedState !== 'bar') setCombinedState('bar')
return <div>{combinedState}</div>
}

const doc = render(<Component />)
expect(doc.baseElement.textContent).toEqual('bar')
})
27 changes: 27 additions & 0 deletions src/hooks/useProvidedStateOrCreate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import {useCallback, useState} from 'react'

/**
* There are some situations where we want to give users the option to control state externally with their own state handlers
* or default to using internal state handlers. Because of the 'rules-of-hooks', we cannot conditionally make a call to `React.useState`
* only in the situations where the state is not provided as a prop.
* This hook aims to encapsulate that logic, so the consumer doesn't need to be concerned with violating `rules-of-hooks`.
* @param externalState The state to use - if undefined, will use the state from a call to React.useState
* @param setExternalState The setState to use - if undefined, will use the state from a call to React.useState
VanAnderson marked this conversation as resolved.
Show resolved Hide resolved
* @param defaultState The defaultState to use, if using internal state.
*/
export function useProvidedStateOrCreate<T>(
externalState?: T,
setExternalState?: (s: T) => void,
defaultState?: T
): [T | undefined, (s: T) => void] {
const [internalState, setInternalState] = useState(defaultState)
const state = externalState ?? internalState
const setState = useCallback(
(s: T) => {
setInternalState(s)
if (setExternalState) setExternalState(s)
},
[setExternalState]
)
return [state, setState]
}
40 changes: 40 additions & 0 deletions src/stories/ActionMenu.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import styled from 'styled-components'
import {ThemeProvider} from '..'
import {ActionMenu} from '../ActionMenu'
import Link, {LinkProps} from '../Link'
import Button from '../Button'
import {ActionList, ItemProps} from '../ActionList'
import BaseStyles from '../BaseStyles'

Expand Down Expand Up @@ -113,6 +114,45 @@ export function SimpleListStory(): JSX.Element {
}
SimpleListStory.storyName = 'Simple List'

export function ExternalOpenState(): JSX.Element {
const [option, setOption] = useState('Select an option')
const [open, setOpen] = useState(false)
const onAction = (itemProps: ItemProps) => {
setOption(itemProps.text)
}
return (
<>
<h1>Simple List</h1>
<h2>Last option activated: {option}</h2>
<h2>External Open State: {open ? 'Open' : 'Closed'}</h2>
dgreif marked this conversation as resolved.
Show resolved Hide resolved
<div>
<Button onClick={() => setOpen(!open)}>Toggle External State</Button>
</div>
<br />
<ErsatzOverlay>
<ActionMenu
onAction={onAction}
anchorContent="Menu"
open={open}
setOpen={setOpen}
items={[
{text: 'New file', trailingText: '⌘O', disabled: true, leadingVisual: ProjectIcon},
ActionList.Divider,
{text: 'Copy link', trailingText: 'ctrl+C'},
{text: 'Edit file', trailingText: '⌘E'},
{
text: 'Delete file',
variant: 'danger',
trailingText: '⌘D'
}
]}
/>
</ErsatzOverlay>
</>
)
}
ExternalOpenState.storyName = 'External Open State'

export function ComplexListStory(): JSX.Element {
const [option, setOption] = useState('Select an option')
const onAction = (itemProps: ItemProps) => {
Expand Down