From f45dfdb3a4ba8df518070ca54425a5710611d5c0 Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Thu, 25 Mar 2021 15:17:07 -0400 Subject: [PATCH 001/118] feat: Add DropdownMenu --- src/ActionList/List.tsx | 4 +-- src/DropdownMenu/DropdownMenu.tsx | 43 ++++++++++++++++++++++++++++ src/DropdownMenu/index.ts | 1 + src/Overlay.tsx | 2 +- src/stories/DropdownMenu.stories.tsx | 42 +++++++++++++++++++++++++++ 5 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 src/DropdownMenu/DropdownMenu.tsx create mode 100644 src/DropdownMenu/index.ts create mode 100644 src/stories/DropdownMenu.stories.tsx diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index 3f5a2b57bd2..117397de1d4 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -8,7 +8,7 @@ import {get} from '../constants' type Flatten = T extends (infer U)[] ? U : never -interface UngroupedListProps { +export interface UngroupedListProps { items: ItemProps[] renderItem?: (props: ItemProps) => JSX.Element } @@ -16,7 +16,7 @@ function isUngroupedListProps(props: ListProps): props is UngroupedListProps { return typeof props === 'object' && props !== null && !('groupMetadata' in props) } -interface GroupedListProps extends UngroupedListProps { +export interface GroupedListProps extends UngroupedListProps { groupMetadata: (GroupProps & {groupId: number; header?: HeaderProps})[] items: (ItemProps & {groupId: number})[] } diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx new file mode 100644 index 00000000000..7009bc7892f --- /dev/null +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -0,0 +1,43 @@ +import React, {useCallback, useRef, useState} from 'react' +import {List, GroupedListProps, UngroupedListProps} from '../ActionList/List' +import Overlay from '../Overlay' +import Button, {ButtonProps} from '../Button' +import {Item} from '../ActionList/Item' + +export interface DropdownMenuProps extends Partial>, UngroupedListProps { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + renderAnchor?: (props: any) => JSX.Element +} + +export function DropdownMenu({ + renderAnchor = (props: ButtonProps) => } + items={[{text: '🔵 Cyan'}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}]} + /> ) } From bb8e06da628381ad4d8b8b676e99aeeb368d2d91 Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Fri, 26 Mar 2021 15:27:25 -0400 Subject: [PATCH 003/118] feat: Enable deselection; draw checks by selected items and empty space (indent) by non-selected items --- src/ActionList/Item.tsx | 5 ++++- src/DropdownMenu/DropdownMenu.tsx | 3 ++- src/stories/DropdownMenu.stories.tsx | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index e67b4a1095a..1073003d198 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -1,4 +1,4 @@ -import type {IconProps} from '@primer/octicons-react' +import {CheckIcon, IconProps} from '@primer/octicons-react' import React from 'react' import styled from 'styled-components' import {get} from '../constants' @@ -9,6 +9,7 @@ interface ItemPropsBase extends React.ComponentPropsWithoutRef<'div'> { descriptionVariant?: 'inline' | 'block' leadingVisual?: React.FunctionComponent leadingVisualSize?: 16 | 20 + selected?: boolean size?: 'small' | 'medium' | 'large' variant?: 'default' | 'singleSelection' | 'multiSelection' | 'danger' | 'static' } @@ -67,6 +68,7 @@ export function Item({ text, description, descriptionVariant = 'inline', + selected, leadingVisual: LeadingVisual, size: _size = 'small', variant = 'default', @@ -74,6 +76,7 @@ export function Item({ }: ItemProps): JSX.Element { return ( + {!!selected === selected && {selected && }} {LeadingVisual && ( diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 2acacb19bab..6fb17449a1a 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -28,8 +28,9 @@ export function DropdownMenu({ renderItem={({onClick, ...itemProps}) => renderItem({ ...itemProps, + selected: itemProps.text === selection, onClick: event => { - select(itemProps.text ?? '') + select(itemProps.text === selection ? '' : itemProps.text ?? '') setOpen(false) onClick && onClick(event) } diff --git a/src/stories/DropdownMenu.stories.tsx b/src/stories/DropdownMenu.stories.tsx index 90cab0b3243..e93c293a4a1 100644 --- a/src/stories/DropdownMenu.stories.tsx +++ b/src/stories/DropdownMenu.stories.tsx @@ -36,7 +36,7 @@ export function FavoriteColorStory(): JSX.Element {

Favorite Color

Please select your favorite color:
} + renderAnchor={({children, ...anchorProps}) => } items={[{text: '🔵 Cyan'}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}]} /> From d9576c860e8f70f5b2a6266315f58af7b6b89672 Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Fri, 26 Mar 2021 17:30:36 -0400 Subject: [PATCH 004/118] feat: Add DropdownButton; begin adding required roles and other attributes --- src/ActionList/Item.tsx | 7 ++- src/ActionList/List.tsx | 2 + src/DropdownMenu/DropdownButton.tsx | 14 ++++++ src/DropdownMenu/DropdownMenu.tsx | 20 ++++++-- src/DropdownMenu/index.ts | 1 + src/stories/DropdownMenu.stories.tsx | 12 +++-- src/utils/types.ts | 72 ++++++++++++++++++++++++++++ 7 files changed, 119 insertions(+), 9 deletions(-) create mode 100644 src/DropdownMenu/DropdownButton.tsx diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 1073003d198..c816b2a3ace 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -75,7 +75,12 @@ export function Item({ ...props }: ItemProps): JSX.Element { return ( - + {!!selected === selected && {selected && }} {LeadingVisual && ( diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index 117397de1d4..d079ac646c2 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -5,10 +5,12 @@ import {Divider} from './Divider' import {Header, HeaderProps} from './Header' import styled from 'styled-components' import {get} from '../constants' +import type {AriaRole} from '../utils/types' type Flatten = T extends (infer U)[] ? U : never export interface UngroupedListProps { + role?: AriaRole items: ItemProps[] renderItem?: (props: ItemProps) => JSX.Element } diff --git a/src/DropdownMenu/DropdownButton.tsx b/src/DropdownMenu/DropdownButton.tsx new file mode 100644 index 00000000000..a47e2657d4b --- /dev/null +++ b/src/DropdownMenu/DropdownButton.tsx @@ -0,0 +1,14 @@ +import React from 'react' +import {ChevronDownIcon} from '@primer/octicons-react' +import Button, {ButtonProps} from '../Button/Button' + +export type DropdownButtonProps = ButtonProps + +export const DropdownButton = React.forwardRef>( + ({children, ...props}: React.PropsWithChildren, ref): JSX.Element => ( + + ) +) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 6fb17449a1a..ee2c1b1d4ae 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -1,33 +1,45 @@ import React, {useCallback, useRef, useState} from 'react' import {List, GroupedListProps, UngroupedListProps} from '../ActionList/List' import Overlay from '../Overlay' -import Button, {ButtonProps} from '../Button' +import {DropdownButton, DropdownButtonProps} from './DropdownButton' import {Item} from '../ActionList/Item' -export interface DropdownMenuProps extends Partial>, UngroupedListProps { +export interface DropdownMenuProps + extends Partial>, + UngroupedListProps { // eslint-disable-next-line @typescript-eslint/no-explicit-any renderAnchor?: (props: any) => JSX.Element } export function DropdownMenu({ - renderAnchor = (props: ButtonProps) => } + renderAnchor={({children, 'aria-labelledby': ariaLabelledBy, ...anchorProps}) => ( + + {children || '🎨'} + + )} items={[{text: '🔵 Cyan'}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}]} /> diff --git a/src/utils/types.ts b/src/utils/types.ts index 72c4c38199c..cb945e16ecf 100644 --- a/src/utils/types.ts +++ b/src/utils/types.ts @@ -10,3 +10,75 @@ export type ComponentProps = T extends React.ComponentType ? Props : never : never + +// ref: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques +export type AriaRole = + | 'alert' + | 'alertdialog' + | 'application' + | 'article' + | 'banner' + | 'button' + | 'cell' + | 'checkbox' + | 'columnheader' + | 'combobox' + | 'complementary' + | 'contentinfo' + | 'definition' + | 'dialog' + | 'directory' + | 'document' + | 'feed' + | 'figure' + | 'form' + | 'grid' + | 'gridcell' + | 'group' + | 'heading' + | 'img' + | 'link' + | 'list' + | 'listbox' + | 'listitem' + | 'log' + | 'main' + | 'marquee' + | 'math' + | 'menu' + | 'menubar' + | 'menuitem' + | 'menuitemcheckbox ' + | 'menuitemradio' + | 'navigation' + | 'none' + | 'note' + | 'option' + | 'presentation' + | 'progressbar' + | 'radio' + | 'radiogroup' + | 'region' + | 'row' + | 'rowgroup' + | 'rowheader' + | 'scrollbar' + | 'search' + | 'searchbox' + | 'separator' + | 'slider' + | 'spinbutton' + | 'status' + | 'switch' + | 'tab' + | 'table' + | 'tablist' + | 'tabpanel' + | 'term' + | 'textbox' + | 'timer' + | 'toolbar' + | 'tooltip' + | 'tree' + | 'treegrid' + | 'treeitem' From e3869a7ed004ce645da70b499941469b503a9c5f Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Wed, 7 Apr 2021 11:56:06 -0400 Subject: [PATCH 005/118] fix: Imported types, plus Storybook warnings, plus debug List-level custom Item renderer --- .storybook/preview.js | 15 ++++++++------- src/ActionList/Item.tsx | 7 +------ src/ActionList/List.tsx | 4 ++-- src/DropdownMenu/DropdownMenu.tsx | 9 +++------ src/stories/ActionList.stories.tsx | 15 ++++++++++----- src/stories/DropdownMenu.stories.tsx | 2 +- 6 files changed, 25 insertions(+), 27 deletions(-) diff --git a/.storybook/preview.js b/.storybook/preview.js index c6731064d06..910648f3573 100644 --- a/.storybook/preview.js +++ b/.storybook/preview.js @@ -1,10 +1,11 @@ -import {addons} from "@storybook/addons" +import {addons} from '@storybook/addons' + +addons.setConfig({ + // Some stories may set up keyboard event handlers, which can can be interfered + // with by these keyboard shortcuts. + enableShortcuts: false +}) export const parameters = { - actions: { argTypesRegex: "^on[A-Z].*" }, - options: { - // Some stories may set up keyboard event handlers, which can can be interfered - // with by these keyboard shortcuts. - enableShortcuts: false - } + actions: {argTypesRegex: '^on[A-Z].*'} } diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index aea120305dd..25f82b3946a 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -109,12 +109,7 @@ export function Item({ ...props }: Partial): JSX.Element { return ( - + {!!selected === selected && {selected && }} {LeadingVisual && ( diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index 904158a345b..109661f5dc1 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -10,7 +10,7 @@ import {SystemCssProperties} from '@styled-system/css' /** * Contract for props passed to the `List` component. */ -interface ListPropsBase { +export interface ListPropsBase { /** * A collection of `Item` props and `Item`-level custom `Item` renderers. */ @@ -47,7 +47,7 @@ interface ListPropsBase { /** * Contract for props passed to the `List` component, when its `Item`s are collected in `Group`s. */ -interface GroupedListProps extends ListPropsBase { +export interface GroupedListProps extends ListPropsBase { /** * A collection of `Group` props (except `items`), plus a unique group identifier * and `Group`-level custom `Item` or `Group` renderers. diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index ee2c1b1d4ae..14685dae19b 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -1,14 +1,11 @@ import React, {useCallback, useRef, useState} from 'react' -import {List, GroupedListProps, UngroupedListProps} from '../ActionList/List' +import {List, GroupedListProps, ListPropsBase} from '../ActionList/List' import Overlay from '../Overlay' import {DropdownButton, DropdownButtonProps} from './DropdownButton' import {Item} from '../ActionList/Item' -export interface DropdownMenuProps - extends Partial>, - UngroupedListProps { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - renderAnchor?: (props: any) => JSX.Element +export interface DropdownMenuProps extends Partial>, ListPropsBase { + renderAnchor?: >(props: T) => JSX.Element } export function DropdownMenu({ diff --git a/src/stories/ActionList.stories.tsx b/src/stories/ActionList.stories.tsx index 40d0eb2a1ea..9a7ffff68c3 100644 --- a/src/stories/ActionList.stories.tsx +++ b/src/stories/ActionList.stories.tsx @@ -82,12 +82,17 @@ export function SimpleListStory(): JSX.Element {

Simple List

} + groupMetadata={[ + { + groupId: '0' + } + ]} items={[ - {text: 'New file'}, - ActionList.Divider, - {text: 'Copy link'}, - {text: 'Edit file'}, - {text: 'Delete file', variant: 'danger'} + {groupId: '0', text: 'New file'}, + {groupId: '0', text: 'Copy link'}, + {groupId: '0', text: 'Edit file'}, + {groupId: '0', text: 'Delete file', variant: 'danger'} ]} /> diff --git a/src/stories/DropdownMenu.stories.tsx b/src/stories/DropdownMenu.stories.tsx index e39fd0b88ad..c92dfca77f1 100644 --- a/src/stories/DropdownMenu.stories.tsx +++ b/src/stories/DropdownMenu.stories.tsx @@ -24,7 +24,7 @@ const meta: Meta = { ], parameters: { controls: { - disabled: true + disable: true } } } From b812e739408709effadd4d1b87bc11e15f98d6f7 Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Wed, 7 Apr 2021 12:15:09 -0400 Subject: [PATCH 006/118] fix: Restore 'List'-level custom 'Item' rendering. --- src/ActionList/List.tsx | 8 ++++---- src/stories/ActionList.stories.tsx | 15 +++++---------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index 109661f5dc1..2eda50fbbc3 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -1,10 +1,10 @@ +import type {AriaRole} from '../utils/types' import {Group, GroupProps} from './Group' import {Item, ItemProps} from './Item' import React from 'react' import {Divider} from './Divider' import styled from 'styled-components' import {get} from '../constants' -import type {AriaRole} from '../utils/types' import {SystemCssProperties} from '@styled-system/css' /** @@ -26,14 +26,14 @@ export interface ListPropsBase { * without a `Group`-level or `Item`-level custom `Item` renderer will be * rendered using this function component. */ - renderItem?: (props: ItemProps) => JSX.Element + renderItem?: typeof Item /** * A `List`-level custom `Group` renderer. Every `Group` within this `List` * without a `Group`-level custom `Item` renderer will be rendered using * this function component. */ - renderGroup?: (props: GroupProps) => JSX.Element + renderGroup?: typeof Group /** * Style variations. Usage is discretionary. @@ -130,7 +130,7 @@ export function List(props: ListProps): JSX.Element { * or the default `Item` renderer. */ const renderItem = (itemProps: ItemProps | (Partial & {renderItem: typeof Item})) => - ((('renderItem' in itemProps && itemProps.renderItem) ?? props.renderItem) || Item).call(null, { + (('renderItem' in itemProps && itemProps.renderItem) || props.renderItem || Item).call(null, { ...itemProps, sx: {...itemStyle, ...itemProps.sx} }) diff --git a/src/stories/ActionList.stories.tsx b/src/stories/ActionList.stories.tsx index 9a7ffff68c3..40d0eb2a1ea 100644 --- a/src/stories/ActionList.stories.tsx +++ b/src/stories/ActionList.stories.tsx @@ -82,17 +82,12 @@ export function SimpleListStory(): JSX.Element {

Simple List

} - groupMetadata={[ - { - groupId: '0' - } - ]} items={[ - {groupId: '0', text: 'New file'}, - {groupId: '0', text: 'Copy link'}, - {groupId: '0', text: 'Edit file'}, - {groupId: '0', text: 'Delete file', variant: 'danger'} + {text: 'New file'}, + ActionList.Divider, + {text: 'Copy link'}, + {text: 'Edit file'}, + {text: 'Delete file', variant: 'danger'} ]} /> From 007ce3b2a1318c1e34b1e23a5700129efbae39fb Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Mon, 12 Apr 2021 13:50:35 -0400 Subject: [PATCH 007/118] chore: Merge 'dropdown-with-zones' into 'dropdownmenu' --- src/ActionList/Item.tsx | 7 +++- src/DropdownMenu/DropdownMenu.tsx | 60 +++++++++++++++++++++++++++- src/Overlay.tsx | 65 +++++++++++-------------------- src/__tests__/Overlay.tsx | 2 - src/behaviors/focusTrap.ts | 2 + src/hooks/useFocusTrap.ts | 25 +++++++----- src/hooks/useFocusZone.ts | 35 +++++++++-------- src/hooks/useMouseIntent.ts | 12 ++++-- src/hooks/useOverlay.tsx | 19 +++------ src/stories/Overlay.stories.tsx | 3 -- src/utils/isRefObject.ts | 5 +++ 11 files changed, 142 insertions(+), 93 deletions(-) create mode 100644 src/utils/isRefObject.ts diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 25f82b3946a..cd0077c5c91 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -109,7 +109,12 @@ export function Item({ ...props }: Partial): JSX.Element { return ( - + {!!selected === selected && {selected && }} {LeadingVisual && ( diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 14685dae19b..9a74918a006 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -3,6 +3,9 @@ import {List, GroupedListProps, ListPropsBase} from '../ActionList/List' import Overlay from '../Overlay' import {DropdownButton, DropdownButtonProps} from './DropdownButton' import {Item} from '../ActionList/Item' +import {useFocusTrap} from '../hooks/useFocusTrap' +import {useFocusZone} from '../hooks/useFocusZone' +import {useAnchoredPosition} from '../hooks/useAnchoredPosition' export interface DropdownMenuProps extends Partial>, ListPropsBase { renderAnchor?: >(props: T) => JSX.Element @@ -14,10 +17,56 @@ export function DropdownMenu({ ...listProps }: DropdownMenuProps): JSX.Element { const anchorRef = useRef(null) + const overlayRef = useRef(null) + const anchorId = `dropdownMenuAnchor-${window.crypto.getRandomValues(new Uint8Array(4)).join('')}` const [selection, select] = useState('') const [open, setOpen] = useState(false) const onDismiss = useCallback(() => setOpen(false), [setOpen]) + + const [state, setState] = useState<'closed' | 'buttonFocus' | 'listFocus'>('closed') + const onAnchorKeyDown = useCallback( + (event: React.KeyboardEvent) => { + if (!event.defaultPrevented) { + if (state === 'closed') { + if (event.key === 'ArrowDown' || event.key === 'ArrowUp') { + setState('listFocus') + setOpen(true) + event.preventDefault() + } else if (event.key === ' ' || event.key === 'Enter') { + setState('buttonFocus') + setOpen(true) + event.preventDefault() + } + } else if (state === 'buttonFocus') { + if (['ArrowDown', 'ArrowUp', 'Tab', 'Enter'].indexOf(event.key) !== -1) { + setState('listFocus') + event.preventDefault() + } else if (event.key === 'Escape') { + setState('closed') + setOpen(false) + event.preventDefault() + } + } + } + }, + [state] + ) + const onAnchorClick = useCallback( + (event: React.MouseEvent) => { + if (!event.defaultPrevented && event.button === 0 && !open) { + setOpen(true) + setState('buttonFocus') + } + }, + [open] + ) + + const {position} = useAnchoredPosition({anchorElementRef: anchorRef, floatingElementRef: overlayRef}) + + useFocusZone({containerRef: overlayRef, disabled: !open || state !== 'listFocus'}, [position]) + useFocusTrap({containerRef: overlayRef, disabled: !open || state !== 'listFocus'}, [position]) + return ( <> {renderAnchor({ @@ -26,10 +75,17 @@ export function DropdownMenu({ 'aria-labelledby': anchorId, 'aria-haspopup': 'listbox', children: selection, - onClick: () => setOpen(!open) + onClick: onAnchorClick, + onKeyDown: onAnchorKeyDown })} {open && ( - + props.visibility}; ${COMMON}; ${POSITION}; ${sx}; @@ -57,12 +56,9 @@ export type OverlayProps = { ignoreClickRefs?: React.RefObject[] initialFocusRef?: React.RefObject returnFocusRef: React.RefObject - anchorRef: React.RefObject onClickOutside: (e: TouchOrMouseEvent) => void onEscape: (e: KeyboardEvent) => void - positionSettings?: AnchoredPositionHookSettings - positionDeps?: React.DependencyList -} & Omit, 'visibility' | keyof SystemPositionProps> +} & Omit, keyof SystemPositionProps> /** * An `Overlay` is a flexible floating surface, used to display transient content such as menus, @@ -79,41 +75,26 @@ export type OverlayProps = { * @param width Sets the width of the `Overlay`, pick from our set list of widths, or pass `auto` to automatically set the width based on the content of the `Overlay`. `sm` corresponds to `256px`, `md` corresponds to `320px`, `lg` corresponds to `480px`, and `xl` corresponds to `640px`. * @param height Sets the height of the `Overlay`, pick from our set list of heights, or pass `auto` to automatically set the height based on the content of the `Overlay`. `sm` corresponds to `480px` and `md` corresponds to `640px`. */ -const Overlay = ({ - onClickOutside, - role = 'dialog', - positionSettings, - positionDeps, - anchorRef, - initialFocusRef, - returnFocusRef, - ignoreClickRefs, - onEscape, - ...rest -}: OverlayProps): ReactElement => { - const {position, ...overlayRest} = useOverlay({ - anchorRef, - positionSettings, - positionDeps, - returnFocusRef, - onEscape, - ignoreClickRefs, - onClickOutside, - initialFocusRef - }) - return ( - - - - ) -} +const Overlay = React.forwardRef( + ( + {onClickOutside, role = 'dialog', initialFocusRef, returnFocusRef, ignoreClickRefs, onEscape, ...rest}, + overlayRef + ): ReactElement => { + const overlayProps = useOverlay({ + overlayRef: isRefObject(overlayRef) ? overlayRef : undefined, + returnFocusRef, + onEscape, + ignoreClickRefs, + onClickOutside, + initialFocusRef + }) + return ( + + + + ) + } +) Overlay.defaultProps = { height: 'auto', diff --git a/src/__tests__/Overlay.tsx b/src/__tests__/Overlay.tsx index a5aa2fb6b89..ecf9b112754 100644 --- a/src/__tests__/Overlay.tsx +++ b/src/__tests__/Overlay.tsx @@ -29,8 +29,6 @@ const TestComponent = ({initialFocus, callback}: TestComponentSettings) => { {isOpen && ( ; initialFocusRef: React.RefObject} { const containerRef = useProvidedRefOrCreate(settings?.containerRef) const initialFocusRef = useProvidedRefOrCreate(settings?.initialFocusRef) const disabled = settings?.disabled const abortController = React.useRef() - React.useEffect(() => { - if (containerRef.current instanceof HTMLElement) { - if (!disabled) { - abortController.current = focusTrap(containerRef.current, initialFocusRef.current ?? undefined) - return () => { + React.useEffect( + () => { + if (containerRef.current instanceof HTMLElement) { + if (!disabled) { + abortController.current = focusTrap(containerRef.current, initialFocusRef.current ?? undefined) + return () => { + abortController.current?.abort() + } + } else { abortController.current?.abort() } - } else { - abortController.current?.abort() } - } - }, [containerRef, initialFocusRef, disabled]) + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [containerRef, initialFocusRef, disabled, ...(dependencies ?? [])] + ) return {containerRef, initialFocusRef} } diff --git a/src/hooks/useFocusZone.ts b/src/hooks/useFocusZone.ts index 56a4501bf3c..52860b6f80f 100644 --- a/src/hooks/useFocusZone.ts +++ b/src/hooks/useFocusZone.ts @@ -36,26 +36,29 @@ export function useFocusZone( const disabled = settings?.disabled const abortController = React.useRef() - useEffect(() => { - if ( - containerRef.current instanceof HTMLElement && - (!useActiveDescendant || activeDescendantControlRef.current instanceof HTMLElement) - ) { - if (!disabled) { - const vanillaSettings: FocusZoneSettings = { - ...settings, - activeDescendantControl: activeDescendantControlRef.current ?? undefined - } - abortController.current = focusZone(containerRef.current, vanillaSettings) - return () => { + useEffect( + () => { + if ( + containerRef.current instanceof HTMLElement && + (!useActiveDescendant || activeDescendantControlRef.current instanceof HTMLElement) + ) { + if (!disabled) { + const vanillaSettings: FocusZoneSettings = { + ...settings, + activeDescendantControl: activeDescendantControlRef.current ?? undefined + } + abortController.current = focusZone(containerRef.current, vanillaSettings) + return () => { + abortController.current?.abort() + } + } else { abortController.current?.abort() } - } else { - abortController.current?.abort() } - } + }, // eslint-disable-next-line react-hooks/exhaustive-deps - }, dependencies ?? []) + [disabled, ...(dependencies ?? [])] + ) return {containerRef, activeDescendantControlRef} } diff --git a/src/hooks/useMouseIntent.ts b/src/hooks/useMouseIntent.ts index 115c130542e..55475e4cc8e 100644 --- a/src/hooks/useMouseIntent.ts +++ b/src/hooks/useMouseIntent.ts @@ -1,11 +1,11 @@ /* adapted from: https://github.com/github/github/blob/a959c0d15c29b98c49b881f520c5947fe24eecb9/app/assets/modules/github/behaviors/button-outline.ts */ import {useEffect} from 'react' +let lastActiveElement: Element | null = null +let currentInputIsMouse = false + const useMouseIntent = () => { useEffect(() => { - let lastActiveElement: Element | null = null - let currentInputIsMouse = false - function setClass() { lastActiveElement = document.activeElement if (document.body) { @@ -29,6 +29,8 @@ const useMouseIntent = () => { document.addEventListener('focusin', setClass, {capture: true}) return () => { + lastActiveElement = null + currentInputIsMouse = false document.removeEventListener('keydown', onKeyDown, {capture: true}) document.removeEventListener('focusin', setClass, {capture: true}) document.removeEventListener('mousedown', onMouseDown, {capture: true}) @@ -36,4 +38,8 @@ const useMouseIntent = () => { }, []) } +export function prepareForFocusWithoutMouse() { + currentInputIsMouse = false +} + export default useMouseIntent diff --git a/src/hooks/useOverlay.tsx b/src/hooks/useOverlay.tsx index 483d123b7e3..0945275d2be 100644 --- a/src/hooks/useOverlay.tsx +++ b/src/hooks/useOverlay.tsx @@ -1,8 +1,7 @@ import {useOnOutsideClick, TouchOrMouseEvent} from './useOnOutsideClick' import {useOpenAndCloseFocus} from './useOpenAndCloseFocus' import {useOnEscapePress} from './useOnEscapePress' -import {AnchoredPositionHookSettings, useAnchoredPosition} from './useAnchoredPosition' -import {useRef} from 'react' +import {useProvidedRefOrCreate} from './useProvidedRefOrCreate' export type UseOverlaySettings = { ignoreClickRefs?: React.RefObject[] @@ -10,32 +9,24 @@ export type UseOverlaySettings = { returnFocusRef: React.RefObject onEscape: (e: KeyboardEvent) => void onClickOutside: (e: TouchOrMouseEvent) => void - anchorRef: React.RefObject - positionDeps?: React.DependencyList - positionSettings?: AnchoredPositionHookSettings + overlayRef?: React.RefObject } export type OverlayReturnProps = { ref: React.RefObject - position: {top: number; left: number} | undefined } export const useOverlay = ({ - anchorRef, - positionSettings = {}, - positionDeps, + overlayRef: _overlayRef, returnFocusRef, initialFocusRef, onEscape, ignoreClickRefs, onClickOutside }: UseOverlaySettings): OverlayReturnProps => { - const overlayRef = useRef(null) - positionSettings.anchorElementRef = anchorRef - positionSettings.floatingElementRef = overlayRef + const overlayRef = useProvidedRefOrCreate(_overlayRef) useOpenAndCloseFocus({containerRef: overlayRef, returnFocusRef, initialFocusRef}) useOnOutsideClick({containerRef: overlayRef, ignoreClickRefs, onClickOutside}) useOnEscapePress({onEscape}) - const {position} = useAnchoredPosition(positionSettings, positionDeps) - return {ref: overlayRef, position} + return {ref: overlayRef} } diff --git a/src/stories/Overlay.stories.tsx b/src/stories/Overlay.stories.tsx index a686f66caf6..4cf66e4aec3 100644 --- a/src/stories/Overlay.stories.tsx +++ b/src/stories/Overlay.stories.tsx @@ -54,7 +54,6 @@ export const DropdownOverlay = () => { {isOpen && ( { {isOpen && ( (x: ForwardedRef): x is RefObject { + return !!(x && 'current' in x) +} From 16751fb8e93d66c4b5e3082b69082f2ef66e05f7 Mon Sep 17 00:00:00 2001 From: dgreif Date: Mon, 12 Apr 2021 14:55:00 -0700 Subject: [PATCH 008/118] fix: keep focus on anchor when dropdown menu opens --- src/DropdownMenu/DropdownMenu.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 9a74918a006..684910048cb 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -22,9 +22,12 @@ export function DropdownMenu({ const anchorId = `dropdownMenuAnchor-${window.crypto.getRandomValues(new Uint8Array(4)).join('')}` const [selection, select] = useState('') const [open, setOpen] = useState(false) - const onDismiss = useCallback(() => setOpen(false), [setOpen]) - const [state, setState] = useState<'closed' | 'buttonFocus' | 'listFocus'>('closed') + const onDismiss = useCallback(() => { + setOpen(false) + setState('closed') + }, []) + const onAnchorKeyDown = useCallback( (event: React.KeyboardEvent) => { if (!event.defaultPrevented) { @@ -44,13 +47,13 @@ export function DropdownMenu({ event.preventDefault() } else if (event.key === 'Escape') { setState('closed') - setOpen(false) + onDismiss() event.preventDefault() } } } }, - [state] + [state, onDismiss] ) const onAnchorClick = useCallback( (event: React.MouseEvent) => { @@ -80,6 +83,7 @@ export function DropdownMenu({ })} {open && ( { select(itemProps.text === selection ? '' : itemProps.text ?? '') - setOpen(false) + onDismiss() onClick && onClick(event) } }) From 0029c37c99fc0e4c9f6867eb283d2726de8362d2 Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Tue, 13 Apr 2021 12:06:28 -0400 Subject: [PATCH 009/118] fix: Rename focus-related state and values to distinguish/dedupe from the Overlay open(rendered)/closed(not-rendered) state. --- src/DropdownMenu/DropdownMenu.tsx | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 684910048cb..96efb5fecca 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -20,46 +20,47 @@ export function DropdownMenu({ const overlayRef = useRef(null) const anchorId = `dropdownMenuAnchor-${window.crypto.getRandomValues(new Uint8Array(4)).join('')}` + const [selection, select] = useState('') + const [open, setOpen] = useState(false) - const [state, setState] = useState<'closed' | 'buttonFocus' | 'listFocus'>('closed') + const [focusType, setFocusType] = useState(null) const onDismiss = useCallback(() => { - setOpen(false) - setState('closed') - }, []) + setOpen(false) + setFocusType(null) + }, []) const onAnchorKeyDown = useCallback( (event: React.KeyboardEvent) => { if (!event.defaultPrevented) { - if (state === 'closed') { + if (!open) { if (event.key === 'ArrowDown' || event.key === 'ArrowUp') { - setState('listFocus') + setFocusType('list') setOpen(true) event.preventDefault() } else if (event.key === ' ' || event.key === 'Enter') { - setState('buttonFocus') + setFocusType('anchor') setOpen(true) event.preventDefault() } - } else if (state === 'buttonFocus') { + } else if (focusType === 'anchor') { if (['ArrowDown', 'ArrowUp', 'Tab', 'Enter'].indexOf(event.key) !== -1) { - setState('listFocus') + setFocusType('list') event.preventDefault() } else if (event.key === 'Escape') { - setState('closed') onDismiss() event.preventDefault() } } } }, - [state, onDismiss] + [open, focusType] ) const onAnchorClick = useCallback( (event: React.MouseEvent) => { if (!event.defaultPrevented && event.button === 0 && !open) { setOpen(true) - setState('buttonFocus') + setFocusType('anchor') } }, [open] @@ -67,8 +68,8 @@ export function DropdownMenu({ const {position} = useAnchoredPosition({anchorElementRef: anchorRef, floatingElementRef: overlayRef}) - useFocusZone({containerRef: overlayRef, disabled: !open || state !== 'listFocus'}, [position]) - useFocusTrap({containerRef: overlayRef, disabled: !open || state !== 'listFocus'}, [position]) + useFocusZone({containerRef: overlayRef, disabled: !open || focusType !== 'list'}, [position]) + useFocusTrap({containerRef: overlayRef, disabled: !open || focusType !== 'list'}, [position]) return ( <> From 5edc4472a5b47c132f2687d78befb012c8769ec8 Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Tue, 13 Apr 2021 12:11:06 -0400 Subject: [PATCH 010/118] chore: Run Prettier --- src/DropdownMenu/DropdownMenu.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 96efb5fecca..5bbc90a69ff 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -26,9 +26,9 @@ export function DropdownMenu({ const [open, setOpen] = useState(false) const [focusType, setFocusType] = useState(null) const onDismiss = useCallback(() => { - setOpen(false) - setFocusType(null) - }, []) + setOpen(false) + setFocusType(null) + }, []) const onAnchorKeyDown = useCallback( (event: React.KeyboardEvent) => { From f7bd5293801023044712e01e1773e68634b5b7fd Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Tue, 13 Apr 2021 12:12:51 -0400 Subject: [PATCH 011/118] fix: Add 'onDismiss' to the dependency list for 'onAnchorKeyDown' to delint --- src/DropdownMenu/DropdownMenu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 5bbc90a69ff..7d6c2b36632 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -54,7 +54,7 @@ export function DropdownMenu({ } } }, - [open, focusType] + [open, focusType, onDismiss] ) const onAnchorClick = useCallback( (event: React.MouseEvent) => { From fd369a3c5c3451e064c9bf5343d160eb88028fda Mon Sep 17 00:00:00 2001 From: dgreif Date: Tue, 13 Apr 2021 15:52:59 -0700 Subject: [PATCH 012/118] fix: force render after overlay ref is set --- src/DropdownMenu/DropdownMenu.tsx | 9 +++++---- src/Overlay.tsx | 13 ++++++++----- src/hooks/useAnchoredPosition.ts | 6 ++++-- src/hooks/useCombinedRefs.ts | 29 +++++++++++++++++++++++++++++ src/hooks/useRenderForcingRef.ts | 19 +++++++++++++++++++ 5 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 src/hooks/useCombinedRefs.ts create mode 100644 src/hooks/useRenderForcingRef.ts diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 7d6c2b36632..f3676e00741 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -6,6 +6,7 @@ import {Item} from '../ActionList/Item' import {useFocusTrap} from '../hooks/useFocusTrap' import {useFocusZone} from '../hooks/useFocusZone' import {useAnchoredPosition} from '../hooks/useAnchoredPosition' +import {useRenderForcingRef} from '../hooks/useRenderForcingRef' export interface DropdownMenuProps extends Partial>, ListPropsBase { renderAnchor?: >(props: T) => JSX.Element @@ -17,7 +18,7 @@ export function DropdownMenu({ ...listProps }: DropdownMenuProps): JSX.Element { const anchorRef = useRef(null) - const overlayRef = useRef(null) + const [overlayRef, updateOverlayRef] = useRenderForcingRef() const anchorId = `dropdownMenuAnchor-${window.crypto.getRandomValues(new Uint8Array(4)).join('')}` @@ -68,8 +69,8 @@ export function DropdownMenu({ const {position} = useAnchoredPosition({anchorElementRef: anchorRef, floatingElementRef: overlayRef}) - useFocusZone({containerRef: overlayRef, disabled: !open || focusType !== 'list'}, [position]) - useFocusTrap({containerRef: overlayRef, disabled: !open || focusType !== 'list'}, [position]) + useFocusZone({containerRef: overlayRef, disabled: !open || focusType !== 'list' || !position}) + useFocusTrap({containerRef: overlayRef, disabled: !open || focusType !== 'list' || !position}) return ( <> @@ -88,7 +89,7 @@ export function DropdownMenu({ returnFocusRef={anchorRef} onClickOutside={onDismiss} onEscape={onDismiss} - ref={overlayRef} + ref={updateOverlayRef} {...position} > ( ( {onClickOutside, role = 'dialog', initialFocusRef, returnFocusRef, ignoreClickRefs, onEscape, ...rest}, - overlayRef + forwardedRef ): ReactElement => { + const overlayRef = useRef(null) + const combinedRef = useCombinedRefs(overlayRef, forwardedRef) + const overlayProps = useOverlay({ - overlayRef: isRefObject(overlayRef) ? overlayRef : undefined, + overlayRef, returnFocusRef, onEscape, ignoreClickRefs, @@ -90,7 +93,7 @@ const Overlay = React.forwardRef( }) return ( - + ) } diff --git a/src/hooks/useAnchoredPosition.ts b/src/hooks/useAnchoredPosition.ts index f7b4e373faf..1e5f6e6be43 100644 --- a/src/hooks/useAnchoredPosition.ts +++ b/src/hooks/useAnchoredPosition.ts @@ -18,7 +18,7 @@ export interface AnchoredPositionHookSettings extends Partial */ export function useAnchoredPosition( settings?: AnchoredPositionHookSettings, - dependencies?: React.DependencyList + dependencies: React.DependencyList = [] ): { floatingElementRef: React.RefObject anchorElementRef: React.RefObject @@ -30,9 +30,11 @@ export function useAnchoredPosition( React.useEffect(() => { if (floatingElementRef.current instanceof Element && anchorElementRef.current instanceof Element) { setPosition(getAnchoredPosition(floatingElementRef.current, anchorElementRef.current, settings)) + } else { + setPosition(undefined) } // eslint-disable-next-line react-hooks/exhaustive-deps - }, dependencies ?? []) + }, [floatingElementRef.current, anchorElementRef.current, ...dependencies]) return { floatingElementRef, anchorElementRef, diff --git a/src/hooks/useCombinedRefs.ts b/src/hooks/useCombinedRefs.ts new file mode 100644 index 00000000000..c5915b152c2 --- /dev/null +++ b/src/hooks/useCombinedRefs.ts @@ -0,0 +1,29 @@ +import React, {ForwardedRef, useRef} from 'react' + +/** + * Creates a ref by combining multiple constituent refs. The ref returned by this hook + * should be passed as the ref for the element that needs to be shared. This is + * particularly useful when you are using `React.forwardRef` in your component but you + * also want to be able to access the local element. This is a small anti-pattern, + * though, as it breaks encapsulation. + * @param refs + */ +export function useCombinedRefs(...refs: (ForwardedRef | null | undefined)[]) { + const combinedRef = useRef(null) + + React.useEffect(() => { + for (const ref of refs) { + if (!ref) { + return + } + if (typeof ref === 'function') { + ref(combinedRef.current ?? null) + } else { + ref.current = combinedRef.current ?? null + } + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [...refs, combinedRef.current]) + + return combinedRef +} diff --git a/src/hooks/useRenderForcingRef.ts b/src/hooks/useRenderForcingRef.ts new file mode 100644 index 00000000000..75e74b6672f --- /dev/null +++ b/src/hooks/useRenderForcingRef.ts @@ -0,0 +1,19 @@ +import {MutableRefObject, RefObject, useCallback, useRef, useState} from 'react' + +/** + * @type TRef The type of the RefObject which should be created. + */ +export function useRenderForcingRef() { + const [refCurrent, updateRefCurrent] = useState(null) + const ref = useRef(null) as MutableRefObject + ref.current = refCurrent + + const updateRef = useCallback( + (newRef: TRef | null) => { + ref.current = newRef + updateRefCurrent(newRef) + }, + [ref] + ) + return [ref as RefObject, updateRef] as const +} From 3ec6103c9df5c0ecd91383b725bff21f2b288701 Mon Sep 17 00:00:00 2001 From: dgreif Date: Wed, 14 Apr 2021 09:22:22 -0700 Subject: [PATCH 013/118] fix(dropdown): treat enter and space on item as an activation --- src/DropdownMenu/DropdownMenu.tsx | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index f3676e00741..d477ebb77b0 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -95,18 +95,30 @@ export function DropdownMenu({ - renderItem({ + renderItem={({onClick, onKeyDown, ...itemProps}) => { + const itemActivated = () => { + select(itemProps.text === selection ? '' : itemProps.text ?? '') + onDismiss() + } + + return renderItem({ ...itemProps, role: 'option', selected: itemProps.text === selection, onClick: event => { - select(itemProps.text === selection ? '' : itemProps.text ?? '') - onDismiss() + itemActivated() onClick && onClick(event) + }, + onKeyDown: event => { + if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) { + itemActivated() + } + // prevent "Enter" event from becoming a click on the anchor as overlay closes + event.preventDefault() + onKeyDown && onKeyDown(event) } }) - } + }} /> )} From ccecdd1755ae36a4d17c86d1f16feabaf6f50610 Mon Sep 17 00:00:00 2001 From: dgreif Date: Wed, 14 Apr 2021 09:55:27 -0700 Subject: [PATCH 014/118] fix: only prevent default if item is activated --- src/DropdownMenu/DropdownMenu.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index d477ebb77b0..9fd12bd4c3c 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -112,9 +112,9 @@ export function DropdownMenu({ onKeyDown: event => { if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) { itemActivated() + // prevent "Enter" event from becoming a click on the anchor as overlay closes + event.preventDefault() } - // prevent "Enter" event from becoming a click on the anchor as overlay closes - event.preventDefault() onKeyDown && onKeyDown(event) } }) From 4a545f99e026904313864be2840a7fd4489e23a3 Mon Sep 17 00:00:00 2001 From: dgreif Date: Wed, 14 Apr 2021 09:57:13 -0700 Subject: [PATCH 015/118] docs: basic docs in dropdown menu --- src/DropdownMenu/DropdownMenu.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 9fd12bd4c3c..62935b8d259 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -9,9 +9,19 @@ import {useAnchoredPosition} from '../hooks/useAnchoredPosition' import {useRenderForcingRef} from '../hooks/useRenderForcingRef' export interface DropdownMenuProps extends Partial>, ListPropsBase { + /** + * A custom fuction component used to render the anchor element. + * Will receive `selection` as a prop when an item is activated. + * Uses a `DropdownButton` by default. + */ renderAnchor?: >(props: T) => JSX.Element } +/** + * 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 anchor contents will be updated + * with the selection. + */ export function DropdownMenu({ renderAnchor = (props: T) => , renderItem = Item, From 6edab5a86fdf1518cffcde73ee93bc32ed4ef566 Mon Sep 17 00:00:00 2001 From: dgreif Date: Wed, 14 Apr 2021 10:27:17 -0700 Subject: [PATCH 016/118] docs: add `DropdownMenu` docs --- docs/content/DropdownMenu.mdx | 27 +++++++++++++++++++++++++++ src/DropdownMenu/DropdownMenu.tsx | 2 +- src/index.ts | 2 ++ src/stories/DropdownMenu.stories.tsx | 1 + 4 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 docs/content/DropdownMenu.mdx diff --git a/docs/content/DropdownMenu.mdx b/docs/content/DropdownMenu.mdx new file mode 100644 index 00000000000..78818d3f193 --- /dev/null +++ b/docs/content/DropdownMenu.mdx @@ -0,0 +1,27 @@ +--- +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 anchor contents will be updated with the selection. + +## Example + +```jsx live + ( + + {children || '🎨'} + + )} + items={[{text: '🔵 Cyan'}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}]} +/> +``` + +## Component props + +| Name | Type | Default | Description | +| :------------ | :-------------------------------------------- | :---------------: | :------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| items | `ItemProps[]` | `undefined` | Required. A list of item objects to display in the menu | +| 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. | +| renderItems | `(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. | diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 62935b8d259..059a77fde1a 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -11,7 +11,7 @@ import {useRenderForcingRef} from '../hooks/useRenderForcingRef' export interface DropdownMenuProps extends Partial>, ListPropsBase { /** * A custom fuction component used to render the anchor element. - * Will receive `selection` as a prop when an item is activated. + * Will receive the selected text as `children` prop when an item is activated. * Uses a `DropdownButton` by default. */ renderAnchor?: >(props: T) => JSX.Element diff --git a/src/index.ts b/src/index.ts index f4833e4f27e..2b3344d54e8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -78,6 +78,8 @@ export type { DropdownItemProps, DropdownMenuProps } from './Dropdown' +export {DropdownButton} from './DropdownMenu/DropdownButton' +export {DropdownMenu} from './DropdownMenu/DropdownMenu' export {default as FilteredSearch} from './FilteredSearch' export type {FilteredSearchProps} from './FilteredSearch' export {default as FilterList} from './FilterList' diff --git a/src/stories/DropdownMenu.stories.tsx b/src/stories/DropdownMenu.stories.tsx index c92dfca77f1..3cbe4686d6d 100644 --- a/src/stories/DropdownMenu.stories.tsx +++ b/src/stories/DropdownMenu.stories.tsx @@ -43,6 +43,7 @@ export function FavoriteColorStory(): JSX.Element { )} items={[{text: '🔵 Cyan'}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}]} /> + ) } From 9b8e9fdfcfff6dc891aecc197462e1537adcb8ee Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Wed, 14 Apr 2021 12:48:06 -0500 Subject: [PATCH 017/118] chore: add basic tests to DropdownMenu --- src/DropdownMenu/DropdownButton.tsx | 2 + src/DropdownMenu/DropdownMenu.tsx | 5 +- src/DropdownMenu/index.ts | 4 +- src/__tests__/DropdownMenu.tsx | 117 ++++++++++++++++++ .../__snapshots__/DropdownMenu.tsx.snap | 101 +++++++++++++++ src/utils/randomId.tsx | 3 + src/utils/testing.tsx | 5 + 7 files changed, 234 insertions(+), 3 deletions(-) create mode 100644 src/__tests__/DropdownMenu.tsx create mode 100644 src/__tests__/__snapshots__/DropdownMenu.tsx.snap create mode 100644 src/utils/randomId.tsx diff --git a/src/DropdownMenu/DropdownButton.tsx b/src/DropdownMenu/DropdownButton.tsx index a47e2657d4b..4496ee89c8f 100644 --- a/src/DropdownMenu/DropdownButton.tsx +++ b/src/DropdownMenu/DropdownButton.tsx @@ -12,3 +12,5 @@ export const DropdownButton = React.forwardRef ) ) + +DropdownButton.displayName = 'DropdownMenu.Button' diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 059a77fde1a..ce795d1960c 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -7,6 +7,7 @@ import {useFocusTrap} from '../hooks/useFocusTrap' import {useFocusZone} from '../hooks/useFocusZone' import {useAnchoredPosition} from '../hooks/useAnchoredPosition' import {useRenderForcingRef} from '../hooks/useRenderForcingRef' +import randomId from '../utils/randomId' export interface DropdownMenuProps extends Partial>, ListPropsBase { /** @@ -30,7 +31,7 @@ export function DropdownMenu({ const anchorRef = useRef(null) const [overlayRef, updateOverlayRef] = useRenderForcingRef() - const anchorId = `dropdownMenuAnchor-${window.crypto.getRandomValues(new Uint8Array(4)).join('')}` + const anchorId = `dropdownMenuAnchor-${randomId()}` const [selection, select] = useState('') @@ -135,3 +136,5 @@ export function DropdownMenu({ ) } + +DropdownMenu.displayName = 'DropdownMenu' diff --git a/src/DropdownMenu/index.ts b/src/DropdownMenu/index.ts index 9036546a379..9656e5e120e 100644 --- a/src/DropdownMenu/index.ts +++ b/src/DropdownMenu/index.ts @@ -1,2 +1,2 @@ -export * from './DropdownMenu' -export * from './DropdownButton' +export {DropdownMenu, DropdownMenuProps} from './DropdownMenu' +export {DropdownButton} from './DropdownButton' diff --git a/src/__tests__/DropdownMenu.tsx b/src/__tests__/DropdownMenu.tsx new file mode 100644 index 00000000000..6dfd8473c23 --- /dev/null +++ b/src/__tests__/DropdownMenu.tsx @@ -0,0 +1,117 @@ +import {cleanup, render as HTMLRender, act, fireEvent} from '@testing-library/react' +import 'babel-polyfill' +import {axe, toHaveNoViolations} from 'jest-axe' +import React from 'react' +import theme from '../theme' +import {DropdownMenu, DropdownButton} from '../DropdownMenu' +import {COMMON} from '../constants' +import {behavesAsComponent, checkExports} from '../utils/testing' +import {ThemeProvider} from 'styled-components' +import {BaseStyles} from '..' +import {registerPortalRoot} from '../Portal/index' +import {ItemProps} from '../ActionList/Item' + +expect.extend(toHaveNoViolations) + +const items = [{text: 'Foo'}, {text: 'Bar'}, {text: 'Baz'}, {text: 'Bon'}] as ItemProps[] + +function SimpleDropdownMenu(): JSX.Element { + return ( + + +
X
+ ( + {children || 'Select an Option'} + )} + /> +
+ + + ) +} + +describe('DropdownMenu', () => { + afterEach(() => { + // since the registry is global, reset after each test + registerPortalRoot(undefined) + jest.clearAllMocks() + }) + + behavesAsComponent({Component: DropdownMenu, systemPropArray: [COMMON], options: {skipAs: true, skipSx: true}}) + + checkExports('DropdownMenu', { + default: undefined, + DropdownMenu, + DropdownButton + }) + + it('should have no axe violations', async () => { + const {container} = HTMLRender() + const results = await axe(container) + expect(results).toHaveNoViolations() + cleanup() + }) + + it('should trigger the overlay on trigger click', async () => { + const menu = HTMLRender() + let portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__') + expect(portalRoot).toBeNull() + const anchor = await menu.findByText('Select an Option') + await anchor?.click() + portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__') + expect(portalRoot).toBeTruthy() + const itemText = items + .map((i: ItemProps) => { + if (i.hasOwnProperty('text')) return i?.text + }) + .join('') + expect(portalRoot?.textContent?.trim()).toEqual(itemText) + }) + + it('should dismiss the overlay on dropdown item click', async () => { + const menu = HTMLRender() + let portalRoot = await menu.baseElement.querySelector('#__primerPortalRoot__') + expect(portalRoot).toBeNull() + const anchor = await menu.findByText('Select an Option') + await anchor.click() + portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__') + expect(portalRoot).toBeTruthy() + const menuItem = await menu.queryByText('Baz') + act(() => { + fireEvent.click(menuItem as Element) + }) + // portal is closed after click + expect(portalRoot?.textContent).toEqual('') // menu items are hidden + }) + + it('option should be selected when chosen from the dropdown menu', async () => { + const menu = HTMLRender() + let portalRoot = await menu.baseElement.querySelector('#__primerPortalRoot__') + expect(portalRoot).toBeNull() + const anchor = await menu.findByText('Select an Option') + await anchor.click() + portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__') + expect(portalRoot).toBeTruthy() + const menuItem = await menu.queryByText('Baz') + act(() => { + fireEvent.click(menuItem as Element) + }) + expect(anchor?.textContent).toEqual('Baz') + }) + + it('should dismiss the overlay on clicking outside overlay', async () => { + const menu = HTMLRender() + let portalRoot = await menu.baseElement.querySelector('#__primerPortalRoot__') + expect(portalRoot).toBeNull() + const anchor = await menu.findByText('Select an Option') + await anchor.click() + portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__') + expect(portalRoot).toBeTruthy() + const somethingElse = (await menu.baseElement.querySelector('#something-else')) as HTMLElement + await somethingElse?.click() + // portal is closed after click + expect(portalRoot?.textContent).toEqual('') // menu items are hidden + }) +}) diff --git a/src/__tests__/__snapshots__/DropdownMenu.tsx.snap b/src/__tests__/__snapshots__/DropdownMenu.tsx.snap new file mode 100644 index 00000000000..467430fb201 --- /dev/null +++ b/src/__tests__/__snapshots__/DropdownMenu.tsx.snap @@ -0,0 +1,101 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`DropdownMenu renders consistently 1`] = ` +.c0 { + position: relative; + display: inline-block; + padding: 6px 16px; + font-family: inherit; + font-weight: 600; + line-height: 20px; + white-space: nowrap; + vertical-align: middle; + cursor: pointer; + -webkit-user-select: none; + -moz-user-select: none; + -ms-user-select: none; + user-select: none; + border-radius: 6px; + -webkit-appearance: none; + -moz-appearance: none; + appearance: none; + -webkit-text-decoration: none; + text-decoration: none; + text-align: center; + font-size: 14px; + color: #24292e; + background-color: #fafbfc; + border: 1px solid rgba(27,31,35,0.15); + box-shadow: 0 1px 0 rgba(27,31,35,0.04),inset 0 1px 0 rgba(255,255,255,0.25); +} + +.c0:hover { + -webkit-text-decoration: none; + text-decoration: none; +} + +.c0:focus { + outline: none; +} + +.c0:disabled { + cursor: default; +} + +.c0:disabled svg { + opacity: 0.6; +} + +.c0:hover { + background-color: #f3f4f6; + border-color: rgba(27,31,35,0.15); +} + +.c0:focus { + border-color: rgba(27,31,35,0.15); + box-shadow: 0 0 0 3px rgba(3,102,214,0.3); +} + +.c0:active { + background-color: #edeff2; + box-shadow: inset 0 0.15em 0.3em rgba(27,31,35,0.15); +} + +.c0:disabled { + color: #959da5; + background-color: #fafbfc; + border-color: rgba(27,31,35,0.15); +} + + +`; diff --git a/src/utils/randomId.tsx b/src/utils/randomId.tsx new file mode 100644 index 00000000000..95c055a4c65 --- /dev/null +++ b/src/utils/randomId.tsx @@ -0,0 +1,3 @@ +const randomId = () => Math.random().toString(36).substring(4) + +export default randomId diff --git a/src/utils/testing.tsx b/src/utils/testing.tsx index 266dc04f988..56801b5564f 100644 --- a/src/utils/testing.tsx +++ b/src/utils/testing.tsx @@ -5,6 +5,8 @@ import enzyme from 'enzyme' import Adapter from '@wojtekmaj/enzyme-adapter-react-17' import {ThemeProvider} from '..' import {default as defaultTheme} from '../theme' +import randomId from '../utils/randomId' +jest.mock('../utils/randomId') type ComputedStyles = Record> @@ -206,6 +208,9 @@ interface BehavesAsComponent { } export function behavesAsComponent({Component, toRender, options}: BehavesAsComponent) { + // @ts-expect-error: ignore jest mock implementation + randomId.mockImplementation(() => 'abcdefg') + options = options || {} const getElement = () => (toRender ? toRender() : ) From e89cf1e640ac82adc6dce18258866069093614e7 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Wed, 14 Apr 2021 13:45:07 -0500 Subject: [PATCH 018/118] fix: add placeholder value to DropdownMenu --- docs/content/DropdownMenu.mdx | 10 ++-------- src/DropdownMenu/DropdownMenu.tsx | 7 ++++++- src/__tests__/DropdownMenu.tsx | 7 +------ src/__tests__/__snapshots__/DropdownMenu.tsx.snap | 1 - src/stories/DropdownMenu.stories.tsx | 10 +--------- 5 files changed, 10 insertions(+), 25 deletions(-) diff --git a/docs/content/DropdownMenu.mdx b/docs/content/DropdownMenu.mdx index 78818d3f193..357f023b643 100644 --- a/docs/content/DropdownMenu.mdx +++ b/docs/content/DropdownMenu.mdx @@ -7,14 +7,7 @@ A `DropdownMenu` provides an anchor (button by default) that will open a floatin ## Example ```jsx live - ( - - {children || '🎨'} - - )} - items={[{text: '🔵 Cyan'}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}]} -/> + ``` ## Component props @@ -22,6 +15,7 @@ A `DropdownMenu` provides an anchor (button by default) that will open a floatin | Name | Type | Default | Description | | :------------ | :-------------------------------------------- | :---------------: | :------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | items | `ItemProps[]` | `undefined` | Required. A list of item objects to display in the menu | +| 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. | | renderItems | `(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. | diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index ce795d1960c..6c9907faecb 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -16,6 +16,10 @@ export interface DropdownMenuProps extends Partial>(props: T) => JSX.Element + /** + * A placeholder value to display on the trigger button when no selection has been made. + */ + placeholder?: string } /** @@ -26,6 +30,7 @@ export interface DropdownMenuProps extends Partial(props: T) => , renderItem = Item, + placeholder, ...listProps }: DropdownMenuProps): JSX.Element { const anchorRef = useRef(null) @@ -90,7 +95,7 @@ export function DropdownMenu({ id: anchorId, 'aria-labelledby': anchorId, 'aria-haspopup': 'listbox', - children: selection, + children: selection || placeholder, onClick: onAnchorClick, onKeyDown: onAnchorKeyDown })} diff --git a/src/__tests__/DropdownMenu.tsx b/src/__tests__/DropdownMenu.tsx index 6dfd8473c23..1feda61fa22 100644 --- a/src/__tests__/DropdownMenu.tsx +++ b/src/__tests__/DropdownMenu.tsx @@ -20,12 +20,7 @@ function SimpleDropdownMenu(): JSX.Element {
X
- ( - {children || 'Select an Option'} - )} - /> +
diff --git a/src/__tests__/__snapshots__/DropdownMenu.tsx.snap b/src/__tests__/__snapshots__/DropdownMenu.tsx.snap index 467430fb201..f746c8dcc1d 100644 --- a/src/__tests__/__snapshots__/DropdownMenu.tsx.snap +++ b/src/__tests__/__snapshots__/DropdownMenu.tsx.snap @@ -75,7 +75,6 @@ exports[`DropdownMenu renders consistently 1`] = ` onClick={[Function]} onKeyDown={[Function]} > -

Favorite Color

Please select your favorite color:
- ( - - {children || '🎨'} - - )} - items={[{text: '🔵 Cyan'}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}]} - /> - + ) } From ff6e266f46508ae85735a21a4b363ce2fcbc2623 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Wed, 14 Apr 2021 13:48:28 -0500 Subject: [PATCH 019/118] fix: fix linter by removing unused import --- src/stories/DropdownMenu.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stories/DropdownMenu.stories.tsx b/src/stories/DropdownMenu.stories.tsx index fee7117a563..033a7780ea6 100644 --- a/src/stories/DropdownMenu.stories.tsx +++ b/src/stories/DropdownMenu.stories.tsx @@ -2,7 +2,7 @@ import {Meta} from '@storybook/react' import React from 'react' import {theme, ThemeProvider} from '..' import BaseStyles from '../BaseStyles' -import {DropdownButton, DropdownMenu} from '../DropdownMenu' +import {DropdownMenu} from '../DropdownMenu' import {registerPortalRoot} from '../Portal' const meta: Meta = { From 275ef22a66aa8ea8339b618a3f8549548cd32f2d Mon Sep 17 00:00:00 2001 From: dgreif Date: Wed, 14 Apr 2021 14:16:50 -0700 Subject: [PATCH 020/118] refactor: allow DropdownMenu state to be controlled by parent --- src/ActionList/Item.tsx | 3 ++- src/ActionList/List.tsx | 19 ++++++++++++++----- src/DropdownMenu/DropdownMenu.tsx | 27 ++++++++++++++++++++------- src/__tests__/DropdownMenu.tsx | 12 ++++++++++-- src/stories/DropdownMenu.stories.tsx | 6 +++++- 5 files changed, 51 insertions(+), 16 deletions(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index cd0077c5c91..c386ff6d708 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -3,6 +3,7 @@ import React from 'react' import styled from 'styled-components' import {get} from '../constants' import sx, {SxProp} from '../sx' +import {ItemInput} from './List' /** * Contract for props passed to the `Item` component. @@ -107,7 +108,7 @@ export function Item({ leadingVisual: LeadingVisual, variant = 'default', ...props -}: Partial): JSX.Element { +}: Partial & {item: ItemInput}): JSX.Element { return ( & {renderItem: typeof Item}) + /** * Contract for props passed to the `List` component. */ @@ -14,7 +16,7 @@ export interface ListPropsBase { /** * A collection of `Item` props and `Item`-level custom `Item` renderers. */ - items: (ItemProps | (Partial & {renderItem: typeof Item}))[] + items: ItemInput[] /** * The ARIA role describing the function of `List` component. `listbox` is a common value. @@ -129,10 +131,11 @@ export function List(props: ListProps): JSX.Element { * An `Item`-level, `Group`-level, or `List`-level custom `Item` renderer, * or the default `Item` renderer. */ - const renderItem = (itemProps: ItemProps | (Partial & {renderItem: typeof Item})) => + const renderItem = (itemProps: ItemInput, item: ItemInput) => (('renderItem' in itemProps && itemProps.renderItem) || props.renderItem || Item).call(null, { ...itemProps, - sx: {...itemStyle, ...itemProps.sx} + sx: {...itemStyle, ...itemProps.sx}, + item }) /** @@ -144,7 +147,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(renderItem)}] + groups = [{items: props.items?.map(item => renderItem(item, item))}] } else { // When `groupMetadata` is provided, collect rendered `Item`s into their associated `Group`s. @@ -165,7 +168,13 @@ export function List(props: ListProps): JSX.Element { ...group, items: [ ...(group?.items ?? []), - renderItem({...(group && 'renderItem' in group && {renderItem: group.renderItem}), ...itemProps}) + renderItem( + { + ...(group && 'renderItem' in group && {renderItem: group.renderItem}), + ...itemProps + }, + itemProps + ) ] }) } diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 6c9907faecb..2459cfdbad4 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -1,5 +1,5 @@ import React, {useCallback, useRef, useState} from 'react' -import {List, GroupedListProps, ListPropsBase} from '../ActionList/List' +import {List, GroupedListProps, ListPropsBase, ItemInput} from '../ActionList/List' import Overlay from '../Overlay' import {DropdownButton, DropdownButtonProps} from './DropdownButton' import {Item} from '../ActionList/Item' @@ -16,10 +16,22 @@ export interface DropdownMenuProps extends Partial>(props: T) => JSX.Element + /** * A placeholder value to display on the trigger button when no selection has been made. */ placeholder?: string + + /** + * An `ItemProps` item from the list of `items` which is currently selected. This item will receive a checkmark next to it in the menu. + */ + selectedItem?: ItemInput + + /** + * 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. + */ + setSelectedItem?: (item?: ItemInput) => unknown } /** @@ -31,6 +43,8 @@ export function DropdownMenu({ renderAnchor = (props: T) => , renderItem = Item, placeholder, + selectedItem, + setSelectedItem, ...listProps }: DropdownMenuProps): JSX.Element { const anchorRef = useRef(null) @@ -38,8 +52,6 @@ export function DropdownMenu({ const anchorId = `dropdownMenuAnchor-${randomId()}` - const [selection, select] = useState('') - const [open, setOpen] = useState(false) const [focusType, setFocusType] = useState(null) const onDismiss = useCallback(() => { @@ -95,7 +107,7 @@ export function DropdownMenu({ id: anchorId, 'aria-labelledby': anchorId, 'aria-haspopup': 'listbox', - children: selection || placeholder, + children: selectedItem?.text ?? placeholder, onClick: onAnchorClick, onKeyDown: onAnchorKeyDown })} @@ -111,16 +123,17 @@ export function DropdownMenu({ { + renderItem={({onClick, onKeyDown, item, ...itemProps}) => { const itemActivated = () => { - select(itemProps.text === selection ? '' : itemProps.text ?? '') + setSelectedItem?.(item === selectedItem ? undefined : item) onDismiss() } return renderItem({ ...itemProps, + item, role: 'option', - selected: itemProps.text === selection, + selected: item === selectedItem, onClick: event => { itemActivated() onClick && onClick(event) diff --git a/src/__tests__/DropdownMenu.tsx b/src/__tests__/DropdownMenu.tsx index 1feda61fa22..1511574d4e9 100644 --- a/src/__tests__/DropdownMenu.tsx +++ b/src/__tests__/DropdownMenu.tsx @@ -10,17 +10,25 @@ import {ThemeProvider} from 'styled-components' import {BaseStyles} from '..' import {registerPortalRoot} from '../Portal/index' import {ItemProps} from '../ActionList/Item' +import {ItemInput} from '../ActionList/List' expect.extend(toHaveNoViolations) -const items = [{text: 'Foo'}, {text: 'Bar'}, {text: 'Baz'}, {text: 'Bon'}] as ItemProps[] +const items = [{text: 'Foo'}, {text: 'Bar'}, {text: 'Baz'}, {text: 'Bon'}] as ItemInput[] function SimpleDropdownMenu(): JSX.Element { + const [selectedItem, setSelectedItem] = React.useState() + return (
X
- +
diff --git a/src/stories/DropdownMenu.stories.tsx b/src/stories/DropdownMenu.stories.tsx index 033a7780ea6..c350f0506c0 100644 --- a/src/stories/DropdownMenu.stories.tsx +++ b/src/stories/DropdownMenu.stories.tsx @@ -1,6 +1,7 @@ import {Meta} from '@storybook/react' import React from 'react' import {theme, ThemeProvider} from '..' +import {ItemProps} from '../ActionList' import BaseStyles from '../BaseStyles' import {DropdownMenu} from '../DropdownMenu' import {registerPortalRoot} from '../Portal' @@ -31,11 +32,14 @@ const meta: Meta = { export default meta export function FavoriteColorStory(): JSX.Element { + const items = React.useMemo(() => [{text: '🔵 Cyan'}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}], []) + const [selectedItem, setSelectedItem] = React.useState() + return ( <>

Favorite Color

Please select your favorite color:
- + ) } From a789c3fa29e580ee152cf041740b5150302713c8 Mon Sep 17 00:00:00 2001 From: dgreif Date: Wed, 14 Apr 2021 15:59:51 -0700 Subject: [PATCH 021/118] docs: remove positioning form overlay docs --- docs/content/Overlay.mdx | 39 +++++++++++++++++---------------------- src/Overlay.tsx | 3 --- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/docs/content/Overlay.mdx b/docs/content/Overlay.mdx index 83173adc6b7..fc97fff6c14 100644 --- a/docs/content/Overlay.mdx +++ b/docs/content/Overlay.mdx @@ -7,18 +7,17 @@ An `Overlay` is a flexible floating surface, used to display transient content s Behaviors include: - Rendering the overlay in a React Portal so that it always renders on top of other content on the page -- Positioning the overlay according to passed in settings, using our context-aware positioning algorithms - Trapping focus - Calling a user provided function when the user presses `Escape` - Calling a user provided function when the user clicks outside of the container -- Focusing either user provided element, or the first focusable element in the container when it is opened +- Focusing either user provided element, or the first focusable element in the container when it is opened - Returning focus to an element when container is closed ## Accessibility considerations - The `Overlay` must either have: - - A value set for the `aria-labelledby` attribute that refers to a visible title. - - An `aria-label` attribute + - A value set for the `aria-labelledby` attribute that refers to a visible title. + - An `aria-label` attribute - If the `Overlay` should also have a longer description, use `aria-describedby` - The `Overlay` component has a `role="dialog"` set on it, if you are using `Overlay` for alerts, you can pass in `role="alertdialog"` instead. Please read the [W3C guidelines](https://www.w3.org/TR/wai-aria-1.1/#alertdialog) to determine which role is best for your use case - The `Overlay` component has `aria-modal` set to `true` by default and should not be overridden as all `Overlay`s behave as modals. @@ -39,9 +38,8 @@ const Demo = () => { open overlay {/* be sure to conditionally render the Overlay. This helps with performance and is required. */} - {isOpen && + {isOpen && ( { > Are you sure you would like to delete this item? - + - } - + )} ) } -render() +render() ``` ## System props @@ -70,15 +67,13 @@ render() ## Component props -| Name | Type | Default | Description | -| :--- | :----- | :-----: | :---------------------------------- | -| positionSettings | See the [`PositionSettings interface`]() section of the `anchoredPosition` docs | `{side: 'outside-bottom', align: 'start', anchorOffset: 4, alignmentOffset: 4, allowOutOfBounds: false }` | Optional. Settings used to position the `Overlay`. If none are provided, `Overlay` is positioned on the bottom left of the `anchorRef`. | -| positionDeps | `React.DependencyList` | `undefined` | Optional. If defined, the position of the `Overlay` will only be recalulated when one of the dependencies in this array changes. | -| ignoreClickRefs | `React.RefObject []` | `undefined` | Optional. An array of ref objects to ignore clicks on in the `onOutsideClick` behavior. This is often used to ignore clicking on the element that toggles the open/closed state for the `Overlay` to prevent the `Overlay` from being toggled twice. | -| initialFocusRef | `React.RefObject` | `undefined` | Optional. Ref for the element to focus when the `Overlay` is opened. If nothing is provided, the first focusable element in the `Overlay` body is focused. | -| anchorRef | `React.RefObject` | `undefined` | Required. Element the `Overlay` should be anchored to. | -| returnFocusRef | `React.RefObject` | `undefined` | Required. Ref for the element to focus when the `Overlay` is closed. | -| onClickOutside | `function` | `undefined` | Required. Function to call when clicking outside of the `Overlay`. Typically this function sets the `Overlay` visibility state to `false`. | -| onEscape | `function` | `undefined` | Required. Function to call when user presses `Escape`. Typically this function sets the `Overlay` visibility state to `false`. | -| width | `'sm', 'md', 'lg', 'xl', 'auto'` | `auto` | Sets the width of the `Overlay`, pick from our set list of widths, or pass `auto` to automatically set the width based on the content of the `Overlay`. `sm` corresponds to `256px`, `md` corresponds to `320px`, `lg` corresponds to `480px`, and `xl` corresponds to `640px`. | -| height | `'sm', 'md', 'auto'` | `auto` | Sets the height of the `Overlay`, pick from our set list of heights, or pass `auto` to automatically set the height based on the content of the `Overlay`. `sm` corresponds to `480px` and `md` corresponds to `640px`. | +| Name | Type | Default | Description | +| :-------------- | :-------------------------------- | :---------: | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| ignoreClickRefs | `React.RefObject []` | `undefined` | Optional. An array of ref objects to ignore clicks on in the `onOutsideClick` behavior. This is often used to ignore clicking on the element that toggles the open/closed state for the `Overlay` to prevent the `Overlay` from being toggled twice. | +| initialFocusRef | `React.RefObject` | `undefined` | Optional. Ref for the element to focus when the `Overlay` is opened. If nothing is provided, the first focusable element in the `Overlay` body is focused. | +| anchorRef | `React.RefObject` | `undefined` | Required. Element the `Overlay` should be anchored to. | +| returnFocusRef | `React.RefObject` | `undefined` | Required. Ref for the element to focus when the `Overlay` is closed. | +| onClickOutside | `function` | `undefined` | Required. Function to call when clicking outside of the `Overlay`. Typically this function sets the `Overlay` visibility state to `false`. | +| onEscape | `function` | `undefined` | Required. Function to call when user presses `Escape`. Typically this function sets the `Overlay` visibility state to `false`. | +| width | `'sm', 'md', 'lg', 'xl', 'auto'` | `auto` | Sets the width of the `Overlay`, pick from our set list of widths, or pass `auto` to automatically set the width based on the content of the `Overlay`. `sm` corresponds to `256px`, `md` corresponds to `320px`, `lg` corresponds to `480px`, and `xl` corresponds to `640px`. | +| height | `'sm', 'md', 'auto'` | `auto` | Sets the height of the `Overlay`, pick from our set list of heights, or pass `auto` to automatically set the height based on the content of the `Overlay`. `sm` corresponds to `480px` and `md` corresponds to `640px`. | diff --git a/src/Overlay.tsx b/src/Overlay.tsx index eb2f087e2ed..3eb50a7154e 100644 --- a/src/Overlay.tsx +++ b/src/Overlay.tsx @@ -64,11 +64,8 @@ export type OverlayProps = { * An `Overlay` is a flexible floating surface, used to display transient content such as menus, * selection options, dialogs, and more. Overlays use shadows to express elevation. The `Overlay` * component handles all behaviors needed by overlay UIs as well as the common styles that all overlays * should have. - * @param positionSettings Settings for calculating the anchored position. - * @param positionDeps Dependencies to determine when to re-calculate the position of the overlay. * @param ignoreClickRefs Optional. An array of ref objects to ignore clicks on in the `onOutsideClick` behavior. This is often used to ignore clicking on the element that toggles the open/closed state for the `Overlay` to prevent the `Overlay` from being toggled twice. * @param initialFocusRef Optional. Ref for the element to focus when the `Overlay` is opened. If nothing is provided, the first focusable element in the `Overlay` body is focused. - * @param anchorRef Required. Element the `Overlay` should be anchored to. * @param returnFocusRef Required. Ref for the element to focus when the `Overlay` is closed. * @param onClickOutside Required. Function to call when clicking outside of the `Overlay`. Typically this function sets the `Overlay` visibility state to `false`. * @param onEscape Required. Function to call when user presses `Escape`. Typically this function sets the `Overlay` visibility state to `false`. From 9fd1e23debdf579d5a0b0b536cacd391fb15bd5e Mon Sep 17 00:00:00 2001 From: dgreif Date: Wed, 14 Apr 2021 16:22:46 -0700 Subject: [PATCH 022/118] fix: typescript fixes --- src/DropdownMenu/index.ts | 3 ++- src/__tests__/DropdownMenu.tsx | 3 +-- src/stories/DropdownMenu.stories.tsx | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/DropdownMenu/index.ts b/src/DropdownMenu/index.ts index 9656e5e120e..d56587e9a30 100644 --- a/src/DropdownMenu/index.ts +++ b/src/DropdownMenu/index.ts @@ -1,2 +1,3 @@ -export {DropdownMenu, DropdownMenuProps} from './DropdownMenu' +export {DropdownMenu} from './DropdownMenu' +export type {DropdownMenuProps} from './DropdownMenu' export {DropdownButton} from './DropdownButton' diff --git a/src/__tests__/DropdownMenu.tsx b/src/__tests__/DropdownMenu.tsx index 1511574d4e9..dd38a6770f1 100644 --- a/src/__tests__/DropdownMenu.tsx +++ b/src/__tests__/DropdownMenu.tsx @@ -9,7 +9,6 @@ import {behavesAsComponent, checkExports} from '../utils/testing' import {ThemeProvider} from 'styled-components' import {BaseStyles} from '..' import {registerPortalRoot} from '../Portal/index' -import {ItemProps} from '../ActionList/Item' import {ItemInput} from '../ActionList/List' expect.extend(toHaveNoViolations) @@ -66,7 +65,7 @@ describe('DropdownMenu', () => { portalRoot = menu.baseElement.querySelector('#__primerPortalRoot__') expect(portalRoot).toBeTruthy() const itemText = items - .map((i: ItemProps) => { + .map((i: ItemInput) => { if (i.hasOwnProperty('text')) return i?.text }) .join('') diff --git a/src/stories/DropdownMenu.stories.tsx b/src/stories/DropdownMenu.stories.tsx index c350f0506c0..f2e5aeeb551 100644 --- a/src/stories/DropdownMenu.stories.tsx +++ b/src/stories/DropdownMenu.stories.tsx @@ -1,7 +1,7 @@ import {Meta} from '@storybook/react' import React from 'react' import {theme, ThemeProvider} from '..' -import {ItemProps} from '../ActionList' +import {ItemInput} from '../ActionList/List' import BaseStyles from '../BaseStyles' import {DropdownMenu} from '../DropdownMenu' import {registerPortalRoot} from '../Portal' @@ -33,7 +33,7 @@ export default meta export function FavoriteColorStory(): JSX.Element { const items = React.useMemo(() => [{text: '🔵 Cyan'}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}], []) - const [selectedItem, setSelectedItem] = React.useState() + const [selectedItem, setSelectedItem] = React.useState() return ( <> From 8eb67b485e2248f0ee040617d94718d9d2f0f6ae Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Wed, 14 Apr 2021 22:51:08 -0500 Subject: [PATCH 023/118] chore: delete isRefObject --- src/utils/isRefObject.ts | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 src/utils/isRefObject.ts diff --git a/src/utils/isRefObject.ts b/src/utils/isRefObject.ts deleted file mode 100644 index 6ef517ce95c..00000000000 --- a/src/utils/isRefObject.ts +++ /dev/null @@ -1,5 +0,0 @@ -import type {ForwardedRef, RefObject} from 'react' - -export function isRefObject(x: ForwardedRef): x is RefObject { - return !!(x && 'current' in x) -} From 19045631e7f06f0e1f9a3edbcde15456d639dfff Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Thu, 15 Apr 2021 07:54:06 -0500 Subject: [PATCH 024/118] add selectedItem to docs --- docs/content/DropdownMenu.mdx | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/docs/content/DropdownMenu.mdx b/docs/content/DropdownMenu.mdx index 357f023b643..033efd6075b 100644 --- a/docs/content/DropdownMenu.mdx +++ b/docs/content/DropdownMenu.mdx @@ -12,10 +12,14 @@ A `DropdownMenu` provides an anchor (button by default) that will open a floatin ## Component props -| Name | Type | Default | Description | -| :------------ | :-------------------------------------------- | :---------------: | :------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| items | `ItemProps[]` | `undefined` | Required. A list of item objects to display in the menu | -| 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. | -| renderItems | `(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. | +| setSelectedItem? | (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. | +| renderItems | `(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. | From d3cc00d29e8bbeddafb06d93b374998ceffa5e04 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Thu, 15 Apr 2021 07:57:42 -0500 Subject: [PATCH 025/118] remove random dash in docs --- docs/content/DropdownMenu.mdx | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/content/DropdownMenu.mdx b/docs/content/DropdownMenu.mdx index 033efd6075b..2601ff70f1f 100644 --- a/docs/content/DropdownMenu.mdx +++ b/docs/content/DropdownMenu.mdx @@ -12,8 +12,6 @@ A `DropdownMenu` provides an anchor (button by default) that will open a floatin ## Component props -- - | Name | Type | Default | Description | | :--------------- | :-------------------------------------------- | :---------------: | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | items | `ItemProps[]` | `undefined` | Required. A list of item objects to display in the menu | From 718b99d34fd4d33d894b071a246974d473c0226c Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Thu, 15 Apr 2021 09:21:48 -0500 Subject: [PATCH 026/118] make DropdownMenu a controlled component in the docs --- docs/content/DropdownMenu.mdx | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/content/DropdownMenu.mdx b/docs/content/DropdownMenu.mdx index 2601ff70f1f..64891bfcbd9 100644 --- a/docs/content/DropdownMenu.mdx +++ b/docs/content/DropdownMenu.mdx @@ -7,7 +7,21 @@ A `DropdownMenu` provides an anchor (button by default) that will open a floatin ## Example ```jsx live - + + {([selectedItem, setSelectedItem]) => ( + ( + + {children} + + )} + placeholder="🎨" + items={[{text: '🔵 Cyan'}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}]} + selectedItem={selectedItem} + setSelectedItem={setSelectedItem} + /> + )} + ``` ## Component props From 6c0ea6514c85919c8064d6de24f4ffef069ff10b Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Thu, 15 Apr 2021 09:24:36 -0500 Subject: [PATCH 027/118] restore custom anchor for DropdownMenu in storybook story --- src/stories/DropdownMenu.stories.tsx | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/stories/DropdownMenu.stories.tsx b/src/stories/DropdownMenu.stories.tsx index f2e5aeeb551..16f89b0b259 100644 --- a/src/stories/DropdownMenu.stories.tsx +++ b/src/stories/DropdownMenu.stories.tsx @@ -3,7 +3,7 @@ import React from 'react' import {theme, ThemeProvider} from '..' import {ItemInput} from '../ActionList/List' import BaseStyles from '../BaseStyles' -import {DropdownMenu} from '../DropdownMenu' +import {DropdownMenu, DropdownButton} from '../DropdownMenu' import {registerPortalRoot} from '../Portal' const meta: Meta = { @@ -39,7 +39,17 @@ export function FavoriteColorStory(): JSX.Element { <>

Favorite Color

Please select your favorite color:
- + ( + + {children} + + )} + placeholder="🎨" + items={items} + selectedItem={selectedItem} + setSelectedItem={setSelectedItem} + /> ) } From ae943e4d42c5437a74d1450e24c3638ffdf2587a Mon Sep 17 00:00:00 2001 From: dgreif Date: Thu, 15 Apr 2021 08:48:48 -0700 Subject: [PATCH 028/118] fix: avoid signal monkey patch during gatsby server side render --- src/polyfills/eventListenerSignal.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/polyfills/eventListenerSignal.ts b/src/polyfills/eventListenerSignal.ts index 9d8d54148ec..450053f5f66 100644 --- a/src/polyfills/eventListenerSignal.ts +++ b/src/polyfills/eventListenerSignal.ts @@ -33,6 +33,10 @@ function featureSupported(): boolean { } function monkeyPatch() { + if (typeof window === 'undefined') { + return + } + const originalAddEventListener = EventTarget.prototype.addEventListener EventTarget.prototype.addEventListener = function (name, originalCallback, optionsOrCapture) { if ( From 4fd91cf4532eeedf3c977bf0aeade3b2f51e46ce Mon Sep 17 00:00:00 2001 From: dgreif Date: Thu, 15 Apr 2021 09:07:43 -0700 Subject: [PATCH 029/118] docs: memoize dropdown menu items in example --- docs/content/DropdownMenu.mdx | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/docs/content/DropdownMenu.mdx b/docs/content/DropdownMenu.mdx index 64891bfcbd9..8aa750bb351 100644 --- a/docs/content/DropdownMenu.mdx +++ b/docs/content/DropdownMenu.mdx @@ -6,9 +6,12 @@ A `DropdownMenu` provides an anchor (button by default) that will open a floatin ## Example -```jsx live - - {([selectedItem, setSelectedItem]) => ( +```javascript live noinline +function DemoComponent() { + const items = React.useMemo(() => [{text: '🔵 Cyan', id: 5}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}], []) + const [selectedItem, setSelectedItem] = React.useState() + + return ( ( @@ -16,12 +19,14 @@ A `DropdownMenu` provides an anchor (button by default) that will open a floatin )} placeholder="🎨" - items={[{text: '🔵 Cyan'}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}]} + items={items} selectedItem={selectedItem} setSelectedItem={setSelectedItem} /> - )} - + ) +} + +render() ``` ## Component props From 398a9d8232e20e464310bbf75ce09ec2c053564b Mon Sep 17 00:00:00 2001 From: dgreif Date: Thu, 15 Apr 2021 09:27:53 -0700 Subject: [PATCH 030/118] docs: useRenderForcingRef --- src/hooks/useRenderForcingRef.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/hooks/useRenderForcingRef.ts b/src/hooks/useRenderForcingRef.ts index 75e74b6672f..60ddeb56fd1 100644 --- a/src/hooks/useRenderForcingRef.ts +++ b/src/hooks/useRenderForcingRef.ts @@ -1,19 +1,22 @@ import {MutableRefObject, RefObject, useCallback, useRef, useState} from 'react' /** + * There are certain situations where a ref might be set after the current render cycle for a + * component has finished. e.g. a forward ref from a conditionally rendered child component. + * In these situations, we need to force a re-render, which is done here by the useState hook. * @type TRef The type of the RefObject which should be created. */ export function useRenderForcingRef() { - const [refCurrent, updateRefCurrent] = useState(null) + const [refCurrent, setRefCurrent] = useState(null) const ref = useRef(null) as MutableRefObject ref.current = refCurrent - const updateRef = useCallback( + const setRef = useCallback( (newRef: TRef | null) => { ref.current = newRef - updateRefCurrent(newRef) + setRefCurrent(newRef) }, [ref] ) - return [ref as RefObject, updateRef] as const + return [ref as RefObject, setRef] as const } From d3d9b87564aa76ef9cbed18fa2702c79f580f9b8 Mon Sep 17 00:00:00 2001 From: dgreif Date: Thu, 15 Apr 2021 09:47:17 -0700 Subject: [PATCH 031/118] docs: fix renderItems typo --- docs/content/ActionList.mdx | 2 +- docs/content/DropdownMenu.mdx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/ActionList.mdx b/docs/content/ActionList.mdx index 51e77f16699..ef14a338e00 100644 --- a/docs/content/ActionList.mdx +++ b/docs/content/ActionList.mdx @@ -63,5 +63,5 @@ An `ActionList` is a list of items which can be activated or selected. `ActionLi | Name | Type | Default | Description | | :------------ | :---------------------------------- | :---------------: | :------------------------------------------------------------------------------------------------------------------------------------------------------ | | items | `ItemProps[]` | `undefined` | Required. A list of item objects conforming to the `ActionList.Item` props interface. | -| renderItems | `(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. | +| 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. | diff --git a/docs/content/DropdownMenu.mdx b/docs/content/DropdownMenu.mdx index 8aa750bb351..7eaa67cab6c 100644 --- a/docs/content/DropdownMenu.mdx +++ b/docs/content/DropdownMenu.mdx @@ -38,5 +38,5 @@ render() | setSelectedItem? | (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. | -| renderItems | `(props: ItemProps) => JSX.Element` | `ActionList.Item` | Optional. If defined, each item in `items` will be passed to this function, allowing for custom item rendering. | +| 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. | From 55161b5916619901aca65c6f166049d15a9039d9 Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Wed, 24 Feb 2021 12:02:31 -0500 Subject: [PATCH 032/118] feat: Style 'ActionListSectionHeader' --- src/ActionList/ActionListSectionHeader.tsx | 52 ++++++++++++++++++++++ src/ActionList/StyledDiv.tsx | 7 +++ src/ActionList/variables.ts | 20 +++++++++ 3 files changed, 79 insertions(+) create mode 100644 src/ActionList/ActionListSectionHeader.tsx create mode 100644 src/ActionList/StyledDiv.tsx create mode 100644 src/ActionList/variables.ts diff --git a/src/ActionList/ActionListSectionHeader.tsx b/src/ActionList/ActionListSectionHeader.tsx new file mode 100644 index 00000000000..e044fb16e0a --- /dev/null +++ b/src/ActionList/ActionListSectionHeader.tsx @@ -0,0 +1,52 @@ +import React from 'react' +import {StyledDiv} from './StyledDiv' +import {s8, s20, s32, gray100, gray200, textSecondary} from './variables' +import type {SystemStyleObject} from '@styled-system/css' + +export interface ActionListSectionHeaderProps extends React.ComponentPropsWithoutRef<'div'> { + variant: 'subtle' | 'filled' + title: string + auxiliaryText?: string +} + +/** Styles used by all variants. */ +const sharedStyles: SystemStyleObject = { + padding: `${(s32 - s20) / 2}px ${s8}px`, + fontSize: '12px', + fontWeight: 'bold', + color: textSecondary +} + +/** Styles used by the 'filled' variant. */ +const filledVariantStyles: SystemStyleObject = { + background: gray100, + margin: `${s8}px -${s8}px`, + borderTop: `1px solid ${gray200}`, + borderBottom: `1px solid ${gray200}`, + + '&:first-child': { + marginTop: 0 + } +} + +export function ActionListSectionHeader({ + variant, + title, + auxiliaryText, + children: _children, + ...props +}: ActionListSectionHeaderProps): JSX.Element { + return ( + + {title} + {auxiliaryText && auxiliaryText} + + ) +} diff --git a/src/ActionList/StyledDiv.tsx b/src/ActionList/StyledDiv.tsx new file mode 100644 index 00000000000..497d6b7cf2f --- /dev/null +++ b/src/ActionList/StyledDiv.tsx @@ -0,0 +1,7 @@ +import React from 'react' +import styled from 'styled-components' +import sx, {SxProp} from '../sx' + +export const StyledDiv = styled('div') & SxProp>` + ${sx} +` diff --git a/src/ActionList/variables.ts b/src/ActionList/variables.ts new file mode 100644 index 00000000000..9e78d67b8a8 --- /dev/null +++ b/src/ActionList/variables.ts @@ -0,0 +1,20 @@ +// expanded from src/support/variables/layout.scss +// adds 12px to the scale as of https://github.com/github/design-systems/issues/992 +// +// https://github.com/github/design-systems/issues/1272 + +export const s8 = 8 +export const s20 = 20 +export const s32 = 32 + +// pulled from src/support/variables/color-system.scss + +// -------- Grays -------- + +export const gray100 = '#f6f8fa' +export const gray200 = '#e1e4e8' +export const gray500 = '#6a737d' + +// Typographic colors, from some color mode file + +export const textSecondary = gray500 From 4ff683576e9ae4b246dd015c657a917af8a7a4a6 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Fri, 2 Apr 2021 08:07:04 -0500 Subject: [PATCH 033/118] basic structure for ActionMenu --- docs/content/ActionMenu.mdx | 19 ++++ src/ActionList/Item.tsx | 37 +++++++- src/ActionList/List.tsx | 3 +- src/ActionMenu.tsx | 51 ++++++++++ src/Popover.tsx | 9 +- src/index.ts | 1 + src/stories/ActionMenu.stories.tsx | 147 +++++++++++++++++++++++++++++ 7 files changed, 259 insertions(+), 8 deletions(-) create mode 100644 docs/content/ActionMenu.mdx create mode 100644 src/ActionMenu.tsx create mode 100644 src/stories/ActionMenu.stories.tsx diff --git a/docs/content/ActionMenu.mdx b/docs/content/ActionMenu.mdx new file mode 100644 index 00000000000..587e2b2e326 --- /dev/null +++ b/docs/content/ActionMenu.mdx @@ -0,0 +1,19 @@ +--- +title: ActionMenu +--- + +Action Menu! + +## Default example + +```jsx live + +``` diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index c386ff6d708..505b485ef3e 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -1,9 +1,9 @@ import {CheckIcon, IconProps} from '@primer/octicons-react' import React from 'react' -import styled from 'styled-components' import {get} from '../constants' import sx, {SxProp} from '../sx' import {ItemInput} from './List' +import styled from 'styled-components' /** * Contract for props passed to the `Item` component. @@ -28,10 +28,20 @@ export interface ItemProps extends React.ComponentPropsWithoutRef<'div'>, SxProp descriptionVariant?: 'inline' | 'block' /** - * Icon (or similar) positioned before `Item` text. + * Icon (or similar) positioned after `Item` text. */ leadingVisual?: React.FunctionComponent + /** + * Icon (or similar) positioned after `Item` text. + */ + auxilaryIcon?: React.FunctionComponent + + /** + * Text positioned after `Item` text and optional auxilary icon. + */ + auxilaryText?: string + /** * Style variations associated with various `Item` types. * @@ -72,7 +82,7 @@ const StyledTextContainer = styled.div<{descriptionVariant: ItemProps['descripti flex-direction: ${({descriptionVariant}) => (descriptionVariant === 'inline' ? 'row' : 'column')}; ` -const LeadingVisualContainer = styled.div` +const BaseVisualContainer = styled.div` { /* Match visual height to adjacent text line height. * @@ -93,6 +103,18 @@ const LeadingVisualContainer = styled.div` } ` +const LeadingVisualContainer = styled(BaseVisualContainer)`` + +const AuxilaryTextContainer = styled(BaseVisualContainer)` + color: ${get('colors.icon.tertiary')}; + margin-left: auto; +` + +const AuxilaryVisualContainer = styled(BaseVisualContainer)` + color: ${get('colors.icon.tertiary')}; + margin-left: auto; +` + const DescriptionContainer = styled.span` color: ${get('colors.text.secondary')}; ` @@ -106,7 +128,10 @@ export function Item({ descriptionVariant = 'inline', selected, leadingVisual: LeadingVisual, + auxilaryIcon: AuxilaryIcon, + auxilaryText, variant = 'default', + onClick, ...props }: Partial & {item: ItemInput}): JSX.Element { return ( @@ -126,6 +151,12 @@ export function Item({
{text}
{description && {description}} + {AuxilaryIcon && ( + + + + )} + {auxilaryText && {auxilaryText}} ) } diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index 24e410ef71a..ec85f36fd56 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -1,7 +1,8 @@ +import React from 'react' import type {AriaRole} from '../utils/types' import {Group, GroupProps} from './Group' import {Item, ItemProps} from './Item' -import React from 'react' + import {Divider} from './Divider' import styled from 'styled-components' import {get} from '../constants' diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx new file mode 100644 index 00000000000..335aa87c48b --- /dev/null +++ b/src/ActionMenu.tsx @@ -0,0 +1,51 @@ +import {List, GroupedListProps, UngroupedListProps} from './ActionList/List' +import {Item} from './ActionList/Item' +import Button, {ButtonProps} from './Button' +import React, {useCallback, useRef, useState} from 'react' +import Overlay from './Overlay' + +export interface ActionMenuProps extends Partial>, UngroupedListProps { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + renderAnchor?: (props: any) => JSX.Element + buttonContent?: React.ReactNode +} + +export function ActionMenu({ + buttonContent, + renderAnchor = (props: T) => , + renderItem = Item, + ...listProps +}: ActionMenuProps): JSX.Element { + const anchorRef = useRef(null) + const anchorId = `actionMenuAnchor-${window.crypto.getRandomValues(new Uint8Array(4)).join('')}` + const [open, setOpen] = useState(false) + const onDismiss = useCallback(() => setOpen(false), [setOpen]) + return ( + <> + {renderAnchor({ + ref: anchorRef, + id: anchorId, + 'aria-labelledby': anchorId, + 'aria-haspopup': 'listbox', + onClick: () => setOpen(!open) + })} + {open && ( + + + renderItem({ + ...itemProps, + onClick: event => { + console.log('test renderItem onClick') + setOpen(false) + onClick && onClick(event) + } + }) + } + /> + + )} + + ) +} diff --git a/src/Popover.tsx b/src/Popover.tsx index 49240d33890..9e8939ecbf8 100644 --- a/src/Popover.tsx +++ b/src/Popover.tsx @@ -1,9 +1,10 @@ -import classnames from 'classnames' -import styled from 'styled-components' -import BorderBox from './BorderBox' -import {COMMON, get, LAYOUT, POSITION, SystemCommonProps, SystemLayoutProps, SystemPositionProps} from './constants' +import {COMMON, LAYOUT, POSITION, SystemCommonProps, SystemLayoutProps, SystemPositionProps, get} from './constants' import sx, {SxProp} from './sx' + +import BorderBox from './BorderBox' import {ComponentProps} from './utils/types' +import classnames from 'classnames' +import styled from 'styled-components' type CaretPosition = | 'top' diff --git a/src/index.ts b/src/index.ts index 2b3344d54e8..55d5aa30ef8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -28,6 +28,7 @@ export {useOverlay} from './hooks/useOverlay' // Components export {ActionList} from './ActionList' +export {ActionMenu} from './ActionMenu' export {default as Avatar} from './Avatar' export type {AvatarProps} from './Avatar' export {default as AvatarPair} from './AvatarPair' diff --git a/src/stories/ActionMenu.stories.tsx b/src/stories/ActionMenu.stories.tsx new file mode 100644 index 00000000000..8bbc6c99737 --- /dev/null +++ b/src/stories/ActionMenu.stories.tsx @@ -0,0 +1,147 @@ +import { + ServerIcon, + PlusCircleIcon, + TypographyIcon, + VersionsIcon, + SearchIcon, + NoteIcon, + ProjectIcon, + FilterIcon, + GearIcon +} from '@primer/octicons-react' +import {Meta} from '@storybook/react' +import React from 'react' +import styled from 'styled-components' +import {ThemeProvider} from '..' +import {ActionMenu} from '../ActionMenu' +import {ActionList} from '../ActionList' +import BaseStyles from '../BaseStyles' + +const meta: Meta = { + title: 'Composite components/ActionMenu', + component: ActionMenu, + decorators: [ + (Story: React.ComponentType): JSX.Element => ( + + + + + + ) + ], + parameters: { + controls: { + disabled: true + } + } +} +export default meta + +const ErsatzOverlay = styled.div` + border-radius: 12px; + box-shadow: 0 1px 3px rgba(0, 0, 0, 0.12), 0 8px 24px rgba(149, 157, 165, 0.2); + padding: 8px; +` + +export function ActionsStory(): JSX.Element { + return ( + <> +

Actions

+ + } + items={[ + { + leadingVisual: ServerIcon, + text: 'Open current Codespace', + description: + "Your existing Codespace will be opened to its previous state, and you'll be asked to manually switch to new-branch.", + descriptionVariant: 'block', + onClick: () => console.log('blah!!!') + }, + { + leadingVisual: PlusCircleIcon, + text: 'Create new Codespace', + description: 'Create a brand new Codespace with a fresh image and checkout this branch.', + descriptionVariant: 'block' + } + ]} + /> + + + ) +} +ActionsStory.storyName = 'Actions' + +export function SimpleListStory(): JSX.Element { + return ( + <> +

Simple List

+ + + + + ) +} +SimpleListStory.storyName = 'Simple List' + +export function ComplexListStory(): JSX.Element { + return ( + <> +

Complex List

+ + }, + {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', + renderItem: props => + }, + { + leadingVisual: NoteIcon, + text: 'Table', + description: 'Information-dense table optimized for operations across teams', + descriptionVariant: 'block', + groupId: '2' + }, + { + leadingVisual: ProjectIcon, + text: 'Board', + description: 'Kanban-style board focused on visual states', + descriptionVariant: 'block', + groupId: '2' + }, + { + 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'} + ]} + /> + + + ) +} +ComplexListStory.storyName = 'Complex List' From f921cd50823626e4e360eaead8b85838c935550b Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Fri, 2 Apr 2021 12:49:37 -0500 Subject: [PATCH 034/118] progress --- src/stories/ActionMenu.stories.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/stories/ActionMenu.stories.tsx b/src/stories/ActionMenu.stories.tsx index 8bbc6c99737..ec350f65756 100644 --- a/src/stories/ActionMenu.stories.tsx +++ b/src/stories/ActionMenu.stories.tsx @@ -105,7 +105,7 @@ export function ComplexListStory(): JSX.Element { {groupId: '0'}, {groupId: '1', header: {title: 'Live query', variant: 'subtle'}}, {groupId: '2', header: {title: 'Layout', variant: 'subtle'}}, - {groupId: '3', renderItem: props => }, + {groupId: '3', renderItem: props => }, {groupId: '4'} ]} items={[ @@ -115,7 +115,7 @@ export function ComplexListStory(): JSX.Element { leadingVisual: SearchIcon, text: 'repo:github/memex,github/github', groupId: '1', - renderItem: props => + renderItem: props => }, { leadingVisual: NoteIcon, From 52241d6c281b559b67a9db70922b695f037d8ec6 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Mon, 5 Apr 2021 09:08:07 -0500 Subject: [PATCH 035/118] clean up ActionMenu and add displayNames --- src/ActionMenu.tsx | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index 335aa87c48b..1c731373c4b 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -1,5 +1,6 @@ import {List, GroupedListProps, UngroupedListProps} from './ActionList/List' -import {Item} from './ActionList/Item' +import {Item, ItemProps} from './ActionList/Item' +import {Divider} from './ActionList/Divider' import Button, {ButtonProps} from './Button' import React, {useCallback, useRef, useState} from 'react' import Overlay from './Overlay' @@ -10,16 +11,21 @@ export interface ActionMenuProps extends Partial + +ActionMenuItem.displayName = 'ActionMenu.Item' + +const ActionMenuBase = ({ buttonContent, renderAnchor = (props: T) => , - renderItem = Item, + renderItem = ActionMenuItem, ...listProps -}: ActionMenuProps): JSX.Element { +}: ActionMenuProps): JSX.Element => { const anchorRef = useRef(null) const anchorId = `actionMenuAnchor-${window.crypto.getRandomValues(new Uint8Array(4)).join('')}` const [open, setOpen] = useState(false) const onDismiss = useCallback(() => setOpen(false), [setOpen]) + const onToggle = useCallback(() => setOpen(!open), [setOpen, open]) return ( <> {renderAnchor({ @@ -27,25 +33,18 @@ export function ActionMenu({ id: anchorId, 'aria-labelledby': anchorId, 'aria-haspopup': 'listbox', - onClick: () => setOpen(!open) + onClick: onToggle, + children: buttonContent })} {open && ( - - - renderItem({ - ...itemProps, - onClick: event => { - console.log('test renderItem onClick') - setOpen(false) - onClick && onClick(event) - } - }) - } - /> + + )} ) } + +ActionMenuBase.displayName = 'ActionMenu' + +export const ActionMenu = Object.assign(ActionMenuBase, {Divider: Divider, Item: ActionMenuItem}) From 3c9dec9435dabd43d20d0459b469bf53ff3bcf19 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Mon, 5 Apr 2021 09:08:27 -0500 Subject: [PATCH 036/118] update ActionMenu Stories to reflect changing API --- src/stories/ActionMenu.stories.tsx | 35 ++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/stories/ActionMenu.stories.tsx b/src/stories/ActionMenu.stories.tsx index ec350f65756..703bd6ccb63 100644 --- a/src/stories/ActionMenu.stories.tsx +++ b/src/stories/ActionMenu.stories.tsx @@ -14,6 +14,7 @@ import React from 'react' import styled from 'styled-components' import {ThemeProvider} from '..' import {ActionMenu} from '../ActionMenu' +import Link from '../Link' import {ActionList} from '../ActionList' import BaseStyles from '../BaseStyles' @@ -57,7 +58,7 @@ export function ActionsStory(): JSX.Element { description: "Your existing Codespace will be opened to its previous state, and you'll be asked to manually switch to new-branch.", descriptionVariant: 'block', - onClick: () => console.log('blah!!!') + onClick: () => console.log('Item 1') }, { leadingVisual: PlusCircleIcon, @@ -115,21 +116,24 @@ export function ComplexListStory(): JSX.Element { leadingVisual: SearchIcon, text: 'repo:github/memex,github/github', groupId: '1', - renderItem: props => + renderItem: props => , + onClick: () => console.log('Item 1') }, { leadingVisual: NoteIcon, text: 'Table', description: 'Information-dense table optimized for operations across teams', descriptionVariant: 'block', - groupId: '2' + groupId: '2', + onClick: () => console.log('Item 2') }, { leadingVisual: ProjectIcon, text: 'Board', description: 'Kanban-style board focused on visual states', descriptionVariant: 'block', - groupId: '2' + groupId: '2', + onClick: () => console.log('Item 3') }, { leadingVisual: FilterIcon, @@ -145,3 +149,26 @@ export function ComplexListStory(): JSX.Element { ) } ComplexListStory.storyName = 'Complex List' + +export function CustomTrigger(): JSX.Element { + const customAnchor = (props: any) => + return ( + <> +

Custom Trigger

+ + + + + ) +} +CustomTrigger.storyName = 'Custom Trigger' From 8604e251bd6da544521f893d3160248c1fb9230e Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Mon, 5 Apr 2021 09:08:40 -0500 Subject: [PATCH 037/118] start to ActionMenu docs --- docs/content/ActionMenu.mdx | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/docs/content/ActionMenu.mdx b/docs/content/ActionMenu.mdx index 587e2b2e326..5a44191e1a6 100644 --- a/docs/content/ActionMenu.mdx +++ b/docs/content/ActionMenu.mdx @@ -1,15 +1,16 @@ --- -title: ActionMenu +title: ActionList --- -Action Menu! +An `ActionList` is a list of items which can be activated or selected. `ActionList` is the base component for many of our menu-type components, including `DropdownMenu` and `ActionMenu`. ## Default example ```jsx live - ``` + +## Component props + +| Name | Type | Default | Description | +| :------------ | :---------------------------------- | :---------------: | :------------------------------------------------------------------------------------------------------------------------------------------------------ | +| items | `ItemProps[]` | `undefined` | Required. A list of item objects conforming to the `ActionList.Item` props interface. | +| renderItems | `(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. | +| buttonContent | React.ReactNode | `undefined` | Optional. If defined, it will be passed to the trigger as the elements child. | From 9f12d0a2216fd5cb1b7ccd980c9d12eeca89863b Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Mon, 5 Apr 2021 09:09:15 -0500 Subject: [PATCH 038/118] tentative updates to ActionList --- src/ActionList/List.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index ec85f36fd56..eabd79238af 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -45,6 +45,12 @@ export interface ListPropsBase { * - `"full"` - `List` children are flush (vertically and horizontally) with `List` edges */ variant?: 'inset' | 'full' + /* + * An option function to run after running an item's onClick function - + * because items can have their own specific click handler, this function + * allows global clean up tasks such as a menu hide after a selection is made + */ + afterSelect?: (e: Event) => void } /** From 9b682f1f487b13144f8b79dad2018418790868ad Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Mon, 5 Apr 2021 09:09:36 -0500 Subject: [PATCH 039/118] ActionMenu tests --- src/__tests__/ActionMenu.tsx | 45 ++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 src/__tests__/ActionMenu.tsx diff --git a/src/__tests__/ActionMenu.tsx b/src/__tests__/ActionMenu.tsx new file mode 100644 index 00000000000..7b058b7ad1e --- /dev/null +++ b/src/__tests__/ActionMenu.tsx @@ -0,0 +1,45 @@ +import {cleanup, render as HTMLRender} from '@testing-library/react' +import 'babel-polyfill' +import {axe, toHaveNoViolations} from 'jest-axe' +import React from 'react' +import theme from '../theme' +import {ActionMenu} from '../ActionMenu' +import {COMMON} from '../constants' +import {behavesAsComponent, checkExports} from '../utils/testing' +import {ThemeProvider} from 'styled-components' +import {BaseStyles} from '..' +expect.extend(toHaveNoViolations) + +function SimpleActionMenu(): JSX.Element { + return ( + + + + + + ) +} + +describe('ActionMenu', () => { + behavesAsComponent({Component: ActionMenu, systemPropArray: [COMMON], options: {skipAs: true, skipSx: true}}) + + checkExports('ActionMenu', { + default: undefined, + ActionMenu + }) + + it('should have no axe violations', async () => { + const {container} = HTMLRender() + const results = await axe(container) + expect(results).toHaveNoViolations() + cleanup() + }) +}) From 44721f8ac4f4565fc4d725417bc6e1d0386831ea Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Mon, 5 Apr 2021 09:18:46 -0500 Subject: [PATCH 040/118] update ActionMenu docs --- docs/content/ActionMenu.mdx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/content/ActionMenu.mdx b/docs/content/ActionMenu.mdx index 5a44191e1a6..bc5d8a11e0f 100644 --- a/docs/content/ActionMenu.mdx +++ b/docs/content/ActionMenu.mdx @@ -1,17 +1,17 @@ --- -title: ActionList +title: ActionMenu --- -An `ActionList` is a list of items which can be activated or selected. `ActionList` is the base component for many of our menu-type components, including `DropdownMenu` and `ActionMenu`. +An `ActionMenu` is a simple ActionList based component for define selectable menus. ## Default example ```jsx live - console.log('Do Something!')}, + ActionMenu.Divider, {text: 'Copy link'}, {text: 'Edit file'}, {text: 'Delete file', variant: 'danger'} From 73caa51c001e69fc3ff4b5cc13e4282c9a23330d Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Mon, 5 Apr 2021 09:38:16 -0500 Subject: [PATCH 041/118] remove commented code --- src/ActionList/List.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index eabd79238af..4b02edda31a 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -144,6 +144,7 @@ export function List(props: ListProps): JSX.Element { sx: {...itemStyle, ...itemProps.sx}, item }) + } /** * An array of `Group`s, each with an associated `Header` and with an array of `Item`s belonging to that `Group`. From da8b54bc82cbb194f9d89e044ce6291ed77cc60c Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Mon, 5 Apr 2021 22:18:47 -0500 Subject: [PATCH 042/118] refigure ActionMenuProps interface --- src/ActionMenu.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index 1c731373c4b..975de80e158 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -1,14 +1,15 @@ -import {List, GroupedListProps, UngroupedListProps} from './ActionList/List' +import {List, ListPropsBase, GroupedListProps} from './ActionList/List' import {Item, ItemProps} from './ActionList/Item' import {Divider} from './ActionList/Divider' import Button, {ButtonProps} from './Button' import React, {useCallback, useRef, useState} from 'react' import Overlay from './Overlay' -export interface ActionMenuProps extends Partial>, UngroupedListProps { +export interface ActionMenuProps extends ListPropsBase, GroupedListProps { // eslint-disable-next-line @typescript-eslint/no-explicit-any renderAnchor?: (props: any) => JSX.Element buttonContent?: React.ReactNode + renderItem?: (props: ItemProps) => JSX.Element } const ActionMenuItem = (props: ItemProps) => @@ -45,6 +46,4 @@ const ActionMenuBase = ({ ) } -ActionMenuBase.displayName = 'ActionMenu' - export const ActionMenu = Object.assign(ActionMenuBase, {Divider: Divider, Item: ActionMenuItem}) From 786220a34e78c7cf27ef3b9d2843c8979da7c0e3 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Mon, 5 Apr 2021 22:32:32 -0500 Subject: [PATCH 043/118] implement styling fix, storybook fix --- src/ActionMenu.tsx | 2 +- src/stories/ActionMenu.stories.tsx | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index 975de80e158..12dace331aa 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -38,7 +38,7 @@ const ActionMenuBase = ({ children: buttonContent })} {open && ( - + )} diff --git a/src/stories/ActionMenu.stories.tsx b/src/stories/ActionMenu.stories.tsx index 703bd6ccb63..028db73cde8 100644 --- a/src/stories/ActionMenu.stories.tsx +++ b/src/stories/ActionMenu.stories.tsx @@ -17,18 +17,22 @@ import {ActionMenu} from '../ActionMenu' import Link from '../Link' import {ActionList} from '../ActionList' import BaseStyles from '../BaseStyles' +import {registerPortalRoot} from '../Portal' const meta: Meta = { title: 'Composite components/ActionMenu', component: ActionMenu, decorators: [ - (Story: React.ComponentType): JSX.Element => ( - - - - - - ) + (Story: React.ComponentType): JSX.Element => { + registerPortalRoot(undefined) + return ( + + + + + + ) + } ], parameters: { controls: { From d3300c44595f4ad63dea7ff8c83113e28c6635b3 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Tue, 6 Apr 2021 23:37:39 -0500 Subject: [PATCH 044/118] ActionMenu supports keyboard keys --- src/ActionList/Item.tsx | 4 +++- src/ActionList/List.tsx | 22 ++++++++++------- src/ActionMenu.tsx | 31 ++++++++++++++++++++---- src/stories/ActionMenu.stories.tsx | 38 +++++++++++++++++++++++------- 4 files changed, 73 insertions(+), 22 deletions(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 505b485ef3e..5f9ff2f31e8 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -68,10 +68,12 @@ const StyledItem = styled.div<{variant: ItemProps['variant']} & SxProp>` color: ${({variant}) => (variant === 'danger' ? get('colors.text.danger') : 'inherit')}; @media (hover: hover) and (pointer: fine) { - :hover { + :hover, + :focus { background: ${props => props.variant === 'danger' ? get('colors.bg.danger') : get('colors.selectMenu.tapHighlight')}; cursor: pointer; + outline: none; } } diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index 4b02edda31a..9faa6c9a7ae 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -45,12 +45,8 @@ export interface ListPropsBase { * - `"full"` - `List` children are flush (vertically and horizontally) with `List` edges */ variant?: 'inset' | 'full' - /* - * An option function to run after running an item's onClick function - - * because items can have their own specific click handler, this function - * allows global clean up tasks such as a menu hide after a selection is made - */ - afterSelect?: (e: Event) => void + + role?: string } /** @@ -138,14 +134,22 @@ export function List(props: ListProps): JSX.Element { * An `Item`-level, `Group`-level, or `List`-level custom `Item` renderer, * or the default `Item` renderer. */ +<<<<<<< HEAD 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: ItemProps | (Partial & {renderItem: typeof Item})) => { + console.log('ey') + return ((('renderItem' in itemProps ? itemProps.renderItem : null) ?? props.renderItem) || Item).call(null, { + ...itemProps, + sx: {...itemStyle, ...itemProps.sx}, + tabIndex: 0 +>>>>>>> ActionMenu supports keyboard keys }) } - /** * An array of `Group`s, each with an associated `Header` and with an array of `Item`s belonging to that `Group`. */ @@ -190,8 +194,10 @@ export function List(props: ListProps): JSX.Element { groups = [...groupMap.values()] } + const {containerRef} = useFocusZone({bindKeys: FocusKeys.ArrowVertical | FocusKeys.HomeAndEnd}) + return ( - + {groups?.map(({header, ...groupProps}, index) => ( <> {renderGroup({ diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index 12dace331aa..2d2fab81532 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -4,12 +4,12 @@ import {Divider} from './ActionList/Divider' import Button, {ButtonProps} from './Button' import React, {useCallback, useRef, useState} from 'react' import Overlay from './Overlay' - export interface ActionMenuProps extends ListPropsBase, GroupedListProps { // eslint-disable-next-line @typescript-eslint/no-explicit-any renderAnchor?: (props: any) => JSX.Element buttonContent?: React.ReactNode renderItem?: (props: ItemProps) => JSX.Element + onActivate?: (props: ItemProps) => void } const ActionMenuItem = (props: ItemProps) => @@ -19,7 +19,8 @@ ActionMenuItem.displayName = 'ActionMenu.Item' const ActionMenuBase = ({ buttonContent, renderAnchor = (props: T) => , - renderItem = ActionMenuItem, + renderItem = Item, + onActivate, ...listProps }: ActionMenuProps): JSX.Element => { const anchorRef = useRef(null) @@ -35,11 +36,33 @@ const ActionMenuBase = ({ 'aria-labelledby': anchorId, 'aria-haspopup': 'listbox', onClick: onToggle, - children: buttonContent + children: buttonContent, + tabIndex: 0 })} {open && ( - + + renderItem({ + ...itemProps, + role: 'menuitem', + onKeyPress: event => { + if (event.key == 'Enter' || event.key == 'Space') { + onActivate && onActivate(itemProps) + setOpen(false) + } + }, + onClick: event => { + console.log('itemProps', itemProps) + onActivate && onActivate(itemProps) + onClick && onClick(event) + setOpen(false) + } + }) + } + /> )} diff --git a/src/stories/ActionMenu.stories.tsx b/src/stories/ActionMenu.stories.tsx index 028db73cde8..fb60cf37495 100644 --- a/src/stories/ActionMenu.stories.tsx +++ b/src/stories/ActionMenu.stories.tsx @@ -10,7 +10,7 @@ import { GearIcon } from '@primer/octicons-react' import {Meta} from '@storybook/react' -import React from 'react' +import React, {useState} from 'react' import styled from 'styled-components' import {ThemeProvider} from '..' import {ActionMenu} from '../ActionMenu' @@ -49,11 +49,17 @@ const ErsatzOverlay = styled.div` ` export function ActionsStory(): JSX.Element { + const [option, setOption] = useState('Select an option') + const onActivate = itemProps => { + setOption(itemProps.text) + } return ( <>

Actions

+

Last option activated: {option}

} items={[ { @@ -61,8 +67,7 @@ export function ActionsStory(): JSX.Element { text: 'Open current Codespace', description: "Your existing Codespace will be opened to its previous state, and you'll be asked to manually switch to new-branch.", - descriptionVariant: 'block', - onClick: () => console.log('Item 1') + descriptionVariant: 'block' }, { leadingVisual: PlusCircleIcon, @@ -79,11 +84,17 @@ export function ActionsStory(): JSX.Element { ActionsStory.storyName = 'Actions' export function SimpleListStory(): JSX.Element { + const [option, setOption] = useState('Select an option') + const onActivate = itemProps => { + setOption(itemProps.text) + } return ( <>

Simple List

+

Last option activated: {option}

{ + setOption(itemProps.text) + } return ( <>

Complex List

+

Last option activated: {option}

, - onClick: () => console.log('Item 1') + renderItem: props => }, { leadingVisual: NoteIcon, text: 'Table', description: 'Information-dense table optimized for operations across teams', descriptionVariant: 'block', - groupId: '2', - onClick: () => console.log('Item 2') + groupId: '2' }, { leadingVisual: ProjectIcon, text: 'Board', description: 'Kanban-style board focused on visual states', descriptionVariant: 'block', - groupId: '2', - onClick: () => console.log('Item 3') + groupId: '2' }, { leadingVisual: FilterIcon, @@ -156,11 +170,17 @@ ComplexListStory.storyName = 'Complex List' export function CustomTrigger(): JSX.Element { const customAnchor = (props: any) => + const [option, setOption] = useState('Select an option') + const onActivate = itemProps => { + setOption(itemProps.text) + } return ( <>

Custom Trigger

+

Last option activated: {option}

Date: Wed, 7 Apr 2021 09:36:59 -0500 Subject: [PATCH 045/118] Fix some TypeScript errors --- src/ActionList/List.tsx | 5 +++++ src/stories/ActionMenu.stories.tsx | 9 +++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index 9faa6c9a7ae..1f1d5955106 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -46,7 +46,12 @@ export interface ListPropsBase { */ variant?: 'inset' | 'full' + // ** role?: string + + tabIndex?: number + + ref?: React.RefObject } /** diff --git a/src/stories/ActionMenu.stories.tsx b/src/stories/ActionMenu.stories.tsx index fb60cf37495..12649159d5d 100644 --- a/src/stories/ActionMenu.stories.tsx +++ b/src/stories/ActionMenu.stories.tsx @@ -18,6 +18,7 @@ import Link from '../Link' import {ActionList} from '../ActionList' import BaseStyles from '../BaseStyles' import {registerPortalRoot} from '../Portal' +import {ItemProps} from '../ActionList/Item' const meta: Meta = { title: 'Composite components/ActionMenu', @@ -50,7 +51,7 @@ const ErsatzOverlay = styled.div` export function ActionsStory(): JSX.Element { const [option, setOption] = useState('Select an option') - const onActivate = itemProps => { + const onActivate = (itemProps: ItemProps) => { setOption(itemProps.text) } return ( @@ -85,7 +86,7 @@ ActionsStory.storyName = 'Actions' export function SimpleListStory(): JSX.Element { const [option, setOption] = useState('Select an option') - const onActivate = itemProps => { + const onActivate = (itemProps: ItemProps) => { setOption(itemProps.text) } return ( @@ -112,7 +113,7 @@ SimpleListStory.storyName = 'Simple List' export function ComplexListStory(): JSX.Element { const [option, setOption] = useState('Select an option') - const onActivate = itemProps => { + const onActivate = (itemProps: ItemProps) => { setOption(itemProps.text) } return ( @@ -171,7 +172,7 @@ ComplexListStory.storyName = 'Complex List' export function CustomTrigger(): JSX.Element { const customAnchor = (props: any) => const [option, setOption] = useState('Select an option') - const onActivate = itemProps => { + const onActivate = (itemProps: ItemProps) => { setOption(itemProps.text) } return ( From d509b91595a2912bb59a45ee6c6f1f3aac6b631e Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Wed, 7 Apr 2021 09:43:18 -0500 Subject: [PATCH 046/118] linter fixes --- src/ActionList/List.tsx | 12 ------------ src/ActionMenu.tsx | 1 - 2 files changed, 13 deletions(-) diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index 1f1d5955106..dfc2c4d96ca 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -139,22 +139,12 @@ export function List(props: ListProps): JSX.Element { * An `Item`-level, `Group`-level, or `List`-level custom `Item` renderer, * or the default `Item` renderer. */ -<<<<<<< HEAD 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: ItemProps | (Partial & {renderItem: typeof Item})) => { - console.log('ey') - return ((('renderItem' in itemProps ? itemProps.renderItem : null) ?? props.renderItem) || Item).call(null, { - ...itemProps, - sx: {...itemStyle, ...itemProps.sx}, - tabIndex: 0 ->>>>>>> ActionMenu supports keyboard keys }) - } /** * An array of `Group`s, each with an associated `Header` and with an array of `Item`s belonging to that `Group`. */ @@ -199,8 +189,6 @@ export function List(props: ListProps): JSX.Element { groups = [...groupMap.values()] } - const {containerRef} = useFocusZone({bindKeys: FocusKeys.ArrowVertical | FocusKeys.HomeAndEnd}) - return ( {groups?.map(({header, ...groupProps}, index) => ( diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index 2d2fab81532..f79b9ddf4d2 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -55,7 +55,6 @@ const ActionMenuBase = ({ } }, onClick: event => { - console.log('itemProps', itemProps) onActivate && onActivate(itemProps) onClick && onClick(event) setOpen(false) From d298159ebd23ccde0d8bc608529942297cdad07a Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Wed, 7 Apr 2021 11:41:03 -0500 Subject: [PATCH 047/118] passing tests for ActionMenu --- package.json | 1 + src/ActionMenu.tsx | 8 +- src/__tests__/ActionMenu.tsx | 2 + .../__snapshots__/ActionMenu.tsx.snap | 79 +++++++++++++++++++ yarn.lock | 7 ++ 5 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 src/__tests__/__snapshots__/ActionMenu.tsx.snap diff --git a/package.json b/package.json index e9105f12a28..6aa75f8998d 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "@types/styled-system__theme-get": "5.0.1", "classnames": "^2.2.5", "deepmerge": "4.2.2", + "get-random-values": "1.2.2", "polished": "3.5.2", "react-is": "16.10.2", "styled-system": "5.1.2" diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index f79b9ddf4d2..d25c9332fd3 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -4,6 +4,7 @@ import {Divider} from './ActionList/Divider' import Button, {ButtonProps} from './Button' import React, {useCallback, useRef, useState} from 'react' import Overlay from './Overlay' +import getRandomValues from 'get-random-values' export interface ActionMenuProps extends ListPropsBase, GroupedListProps { // eslint-disable-next-line @typescript-eslint/no-explicit-any renderAnchor?: (props: any) => JSX.Element @@ -24,7 +25,7 @@ const ActionMenuBase = ({ ...listProps }: ActionMenuProps): JSX.Element => { const anchorRef = useRef(null) - const anchorId = `actionMenuAnchor-${window.crypto.getRandomValues(new Uint8Array(4)).join('')}` + const anchorId = `actionMenuAnchor-${getRandomValues(new Uint8Array(4)).join('')}` const [open, setOpen] = useState(false) const onDismiss = useCallback(() => setOpen(false), [setOpen]) const onToggle = useCallback(() => setOpen(!open), [setOpen, open]) @@ -35,6 +36,7 @@ const ActionMenuBase = ({ id: anchorId, 'aria-labelledby': anchorId, 'aria-haspopup': 'listbox', + 'aria-label': 'menu', onClick: onToggle, children: buttonContent, tabIndex: 0 @@ -42,8 +44,8 @@ const ActionMenuBase = ({ {open && ( renderItem({ ...itemProps, @@ -68,4 +70,6 @@ const ActionMenuBase = ({ ) } +ActionMenuBase.displayName = 'ActionMenu' + export const ActionMenu = Object.assign(ActionMenuBase, {Divider: Divider, Item: ActionMenuItem}) diff --git a/src/__tests__/ActionMenu.tsx b/src/__tests__/ActionMenu.tsx index 7b058b7ad1e..524a5d9a5c8 100644 --- a/src/__tests__/ActionMenu.tsx +++ b/src/__tests__/ActionMenu.tsx @@ -8,6 +8,8 @@ import {COMMON} from '../constants' import {behavesAsComponent, checkExports} from '../utils/testing' import {ThemeProvider} from 'styled-components' import {BaseStyles} from '..' +import getRandomValues from 'get-random-values' + expect.extend(toHaveNoViolations) function SimpleActionMenu(): JSX.Element { diff --git a/src/__tests__/__snapshots__/ActionMenu.tsx.snap b/src/__tests__/__snapshots__/ActionMenu.tsx.snap new file mode 100644 index 00000000000..0e92271b5f1 --- /dev/null +++ b/src/__tests__/__snapshots__/ActionMenu.tsx.snap @@ -0,0 +1,79 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`ActionMenu renders consistently 1`] = ` +.c0 { + position: relative; + display: inline-block; + padding: 6px 16px; + font-family: inherit; + font-weight: 600; + line-height: 20px; + white-space: nowrap; + vertical-align: middle; + cursor: pointer; + -webkit-user-select: none; + -moz-user-select: none; + -ms-user-select: none; + user-select: none; + border-radius: 6px; + -webkit-appearance: none; + -moz-appearance: none; + appearance: none; + -webkit-text-decoration: none; + text-decoration: none; + text-align: center; + font-size: 14px; + color: #24292e; + background-color: #fafbfc; + border: 1px solid rgba(27,31,35,0.15); + box-shadow: 0 1px 0 rgba(27,31,35,0.04),inset 0 1px 0 rgba(255,255,255,0.25); +} + +.c0:hover { + -webkit-text-decoration: none; + text-decoration: none; +} + +.c0:focus { + outline: none; +} + +.c0:disabled { + cursor: default; +} + +.c0:disabled svg { + opacity: 0.6; +} + +.c0:hover { + background-color: #f3f4f6; + border-color: rgba(27,31,35,0.15); +} + +.c0:focus { + border-color: rgba(27,31,35,0.15); + box-shadow: 0 0 0 3px rgba(3,102,214,0.3); +} + +.c0:active { + background-color: #edeff2; + box-shadow: inset 0 0.15em 0.3em rgba(27,31,35,0.15); +} + +.c0:disabled { + color: #959da5; + background-color: #fafbfc; + border-color: rgba(27,31,35,0.15); +} + +, + triggerContent, + renderAnchor = (props: T) => , renderItem = Item, onActivate, ...listProps @@ -38,7 +38,7 @@ const ActionMenuBase = ({ 'aria-haspopup': 'listbox', 'aria-label': 'menu', onClick: onToggle, - children: buttonContent, + children: triggerContent, tabIndex: 0 })} {open && ( diff --git a/src/__tests__/ActionMenu.tsx b/src/__tests__/ActionMenu.tsx index 1d01fc9f0bf..a97da0f1fc5 100644 --- a/src/__tests__/ActionMenu.tsx +++ b/src/__tests__/ActionMenu.tsx @@ -26,7 +26,7 @@ function SimpleActionMenu(): JSX.Element {
X
- +
diff --git a/src/stories/ActionMenu.stories.tsx b/src/stories/ActionMenu.stories.tsx index 12649159d5d..76804474bde 100644 --- a/src/stories/ActionMenu.stories.tsx +++ b/src/stories/ActionMenu.stories.tsx @@ -61,7 +61,7 @@ export function ActionsStory(): JSX.Element { } + triggerContent={} items={[ { leadingVisual: ServerIcon, @@ -96,7 +96,7 @@ export function SimpleListStory(): JSX.Element { Date: Thu, 8 Apr 2021 09:29:13 -0500 Subject: [PATCH 051/118] linter fixes --- @types/get-random-values/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/@types/get-random-values/index.d.ts b/@types/get-random-values/index.d.ts index e63b55e83ad..fe88efc8ce5 100644 --- a/@types/get-random-values/index.d.ts +++ b/@types/get-random-values/index.d.ts @@ -1,4 +1,4 @@ declare module 'get-random-values' { function random(arr: number[] | Uint8Array | Uint16Array): number[] | Uint8Array | Uint16Array export = random -} \ No newline at end of file +} From ba7fc30e82ae6b7e64f76118fd8b5952bc122e47 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Thu, 8 Apr 2021 09:33:43 -0500 Subject: [PATCH 052/118] remove cruft from ActionList rebase --- src/ActionList/ActionListSectionHeader.tsx | 52 ---------------------- src/ActionList/StyledDiv.tsx | 7 --- src/ActionList/variables.ts | 20 --------- src/Popover.tsx | 9 ++-- 4 files changed, 4 insertions(+), 84 deletions(-) delete mode 100644 src/ActionList/ActionListSectionHeader.tsx delete mode 100644 src/ActionList/StyledDiv.tsx delete mode 100644 src/ActionList/variables.ts diff --git a/src/ActionList/ActionListSectionHeader.tsx b/src/ActionList/ActionListSectionHeader.tsx deleted file mode 100644 index e044fb16e0a..00000000000 --- a/src/ActionList/ActionListSectionHeader.tsx +++ /dev/null @@ -1,52 +0,0 @@ -import React from 'react' -import {StyledDiv} from './StyledDiv' -import {s8, s20, s32, gray100, gray200, textSecondary} from './variables' -import type {SystemStyleObject} from '@styled-system/css' - -export interface ActionListSectionHeaderProps extends React.ComponentPropsWithoutRef<'div'> { - variant: 'subtle' | 'filled' - title: string - auxiliaryText?: string -} - -/** Styles used by all variants. */ -const sharedStyles: SystemStyleObject = { - padding: `${(s32 - s20) / 2}px ${s8}px`, - fontSize: '12px', - fontWeight: 'bold', - color: textSecondary -} - -/** Styles used by the 'filled' variant. */ -const filledVariantStyles: SystemStyleObject = { - background: gray100, - margin: `${s8}px -${s8}px`, - borderTop: `1px solid ${gray200}`, - borderBottom: `1px solid ${gray200}`, - - '&:first-child': { - marginTop: 0 - } -} - -export function ActionListSectionHeader({ - variant, - title, - auxiliaryText, - children: _children, - ...props -}: ActionListSectionHeaderProps): JSX.Element { - return ( - - {title} - {auxiliaryText && auxiliaryText} - - ) -} diff --git a/src/ActionList/StyledDiv.tsx b/src/ActionList/StyledDiv.tsx deleted file mode 100644 index 497d6b7cf2f..00000000000 --- a/src/ActionList/StyledDiv.tsx +++ /dev/null @@ -1,7 +0,0 @@ -import React from 'react' -import styled from 'styled-components' -import sx, {SxProp} from '../sx' - -export const StyledDiv = styled('div') & SxProp>` - ${sx} -` diff --git a/src/ActionList/variables.ts b/src/ActionList/variables.ts deleted file mode 100644 index 9e78d67b8a8..00000000000 --- a/src/ActionList/variables.ts +++ /dev/null @@ -1,20 +0,0 @@ -// expanded from src/support/variables/layout.scss -// adds 12px to the scale as of https://github.com/github/design-systems/issues/992 -// -// https://github.com/github/design-systems/issues/1272 - -export const s8 = 8 -export const s20 = 20 -export const s32 = 32 - -// pulled from src/support/variables/color-system.scss - -// -------- Grays -------- - -export const gray100 = '#f6f8fa' -export const gray200 = '#e1e4e8' -export const gray500 = '#6a737d' - -// Typographic colors, from some color mode file - -export const textSecondary = gray500 diff --git a/src/Popover.tsx b/src/Popover.tsx index 9e8939ecbf8..49240d33890 100644 --- a/src/Popover.tsx +++ b/src/Popover.tsx @@ -1,10 +1,9 @@ -import {COMMON, LAYOUT, POSITION, SystemCommonProps, SystemLayoutProps, SystemPositionProps, get} from './constants' -import sx, {SxProp} from './sx' - -import BorderBox from './BorderBox' -import {ComponentProps} from './utils/types' import classnames from 'classnames' import styled from 'styled-components' +import BorderBox from './BorderBox' +import {COMMON, get, LAYOUT, POSITION, SystemCommonProps, SystemLayoutProps, SystemPositionProps} from './constants' +import sx, {SxProp} from './sx' +import {ComponentProps} from './utils/types' type CaretPosition = | 'top' From cd72015cbee9040f0eea87dbf6d65ef707c868ef Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Thu, 8 Apr 2021 09:36:54 -0500 Subject: [PATCH 053/118] small spacing fixes --- src/ActionList/List.tsx | 1 - src/ActionMenu.tsx | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index dfc2c4d96ca..eacab7c39fb 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -2,7 +2,6 @@ import React from 'react' import type {AriaRole} from '../utils/types' import {Group, GroupProps} from './Group' import {Item, ItemProps} from './Item' - import {Divider} from './Divider' import styled from 'styled-components' import {get} from '../constants' diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index e8e39bf763f..9d03e1bc0d7 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -5,6 +5,7 @@ import Button, {ButtonProps} from './Button' import React, {useCallback, useRef, useState} from 'react' import Overlay from './Overlay' import getRandomValues from 'get-random-values' + export interface ActionMenuProps extends ListPropsBase, GroupedListProps { // eslint-disable-next-line @typescript-eslint/no-explicit-any renderAnchor?: (props: any) => JSX.Element From e73eb87d3d2c449c68bb28acc41c40ed466d9404 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Thu, 8 Apr 2021 10:12:22 -0500 Subject: [PATCH 054/118] fix typescript error --- src/stories/ActionMenu.stories.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/stories/ActionMenu.stories.tsx b/src/stories/ActionMenu.stories.tsx index 76804474bde..aa3ca9e057d 100644 --- a/src/stories/ActionMenu.stories.tsx +++ b/src/stories/ActionMenu.stories.tsx @@ -14,7 +14,7 @@ import React, {useState} from 'react' import styled from 'styled-components' import {ThemeProvider} from '..' import {ActionMenu} from '../ActionMenu' -import Link from '../Link' +import Link, {LinkProps} from '../Link' import {ActionList} from '../ActionList' import BaseStyles from '../BaseStyles' import {registerPortalRoot} from '../Portal' @@ -170,7 +170,7 @@ export function ComplexListStory(): JSX.Element { ComplexListStory.storyName = 'Complex List' export function CustomTrigger(): JSX.Element { - const customAnchor = (props: any) => + const customAnchor = (props: LinkProps) => const [option, setOption] = useState('Select an option') const onActivate = (itemProps: ItemProps) => { setOption(itemProps.text) From cd008de2ddd81bd776423e5bf60841d66515c7e1 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Thu, 8 Apr 2021 10:53:10 -0500 Subject: [PATCH 055/118] fix some typescript errors --- src/ActionList/List.tsx | 2 +- src/ActionMenu.tsx | 7 +++---- src/__tests__/ActionMenu.tsx | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index eacab7c39fb..42dac595403 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -189,7 +189,7 @@ export function List(props: ListProps): JSX.Element { } return ( - + } {...props}> {groups?.map(({header, ...groupProps}, index) => ( <> {renderGroup({ diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index 9d03e1bc0d7..479cc86bebb 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -6,11 +6,10 @@ import React, {useCallback, useRef, useState} from 'react' import Overlay from './Overlay' import getRandomValues from 'get-random-values' -export interface ActionMenuProps extends ListPropsBase, GroupedListProps { +export interface ActionMenuProps extends Partial>, ListPropsBase { // eslint-disable-next-line @typescript-eslint/no-explicit-any renderAnchor?: (props: any) => JSX.Element triggerContent?: React.ReactNode - renderItem?: (props: ItemProps) => JSX.Element onActivate?: (props: ItemProps) => void } @@ -53,12 +52,12 @@ const ActionMenuBase = ({ role: 'menuitem', onKeyPress: event => { if (event.key == 'Enter' || event.key == 'Space') { - onActivate && onActivate(itemProps) + onActivate && onActivate(itemProps as ItemProps) setOpen(false) } }, onClick: event => { - onActivate && onActivate(itemProps) + onActivate && onActivate(itemProps as ItemProps) onClick && onClick(event) setOpen(false) } diff --git a/src/__tests__/ActionMenu.tsx b/src/__tests__/ActionMenu.tsx index a97da0f1fc5..ae12b543584 100644 --- a/src/__tests__/ActionMenu.tsx +++ b/src/__tests__/ActionMenu.tsx @@ -9,7 +9,7 @@ import {behavesAsComponent, checkExports} from '../utils/testing' import {ThemeProvider} from 'styled-components' import {BaseStyles} from '..' import {registerPortalRoot} from '../Portal/index' -import {ItemProps} from './ActionList/Item' +import {ItemProps} from '../ActionList/Item' expect.extend(toHaveNoViolations) const items = [ @@ -17,7 +17,7 @@ const items = [ {text: 'Copy link'}, {text: 'Edit file'}, {text: 'Delete file', variant: 'danger'} -] as ItemProps +] as ItemProps[] const mockOnActivate = jest.fn() From 9276de471fcd1a12d5b3b9de747a8a053fd0a3d8 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Thu, 8 Apr 2021 12:03:05 -0500 Subject: [PATCH 056/118] use proper AriaRole prop --- src/ActionList/List.tsx | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index 42dac595403..6afea8746b3 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -44,13 +44,6 @@ export interface ListPropsBase { * - `"full"` - `List` children are flush (vertically and horizontally) with `List` edges */ variant?: 'inset' | 'full' - - // ** - role?: string - - tabIndex?: number - - ref?: React.RefObject } /** From fdeada6f27f2dec9a80cf636df7dedece9a85196 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Fri, 9 Apr 2021 15:01:34 -0500 Subject: [PATCH 057/118] revise props on ActionList and remove package dependency --- package.json | 1 - src/ActionList/List.tsx | 2 +- src/ActionMenu.tsx | 4 ++-- src/__tests__/__snapshots__/ActionMenu.tsx.snap | 4 ++-- yarn.lock | 7 ------- 5 files changed, 5 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index 6aa75f8998d..e9105f12a28 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,6 @@ "@types/styled-system__theme-get": "5.0.1", "classnames": "^2.2.5", "deepmerge": "4.2.2", - "get-random-values": "1.2.2", "polished": "3.5.2", "react-is": "16.10.2", "styled-system": "5.1.2" diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index 6afea8746b3..0d028a25ddb 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -12,7 +12,7 @@ export type ItemInput = ItemProps | (Partial & {renderItem: typeof It /** * Contract for props passed to the `List` component. */ -export interface ListPropsBase { +export interface ListPropsBase extends React.ComponentPropsWithRef<'div'> { /** * A collection of `Item` props and `Item`-level custom `Item` renderers. */ diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index 479cc86bebb..2ca0d841301 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -4,7 +4,7 @@ import {Divider} from './ActionList/Divider' import Button, {ButtonProps} from './Button' import React, {useCallback, useRef, useState} from 'react' import Overlay from './Overlay' -import getRandomValues from 'get-random-values' +import randomId from './utils/randomId' export interface ActionMenuProps extends Partial>, ListPropsBase { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -25,7 +25,7 @@ const ActionMenuBase = ({ ...listProps }: ActionMenuProps): JSX.Element => { const anchorRef = useRef(null) - const anchorId = `actionMenuAnchor-${getRandomValues(new Uint8Array(4)).join('')}` + const anchorId = `actionMenuAnchor-${randomId()}` const [open, setOpen] = useState(false) const onDismiss = useCallback(() => setOpen(false), [setOpen]) const onToggle = useCallback(() => setOpen(!open), [setOpen, open]) diff --git a/src/__tests__/__snapshots__/ActionMenu.tsx.snap b/src/__tests__/__snapshots__/ActionMenu.tsx.snap index 0e92271b5f1..e0ae79547f7 100644 --- a/src/__tests__/__snapshots__/ActionMenu.tsx.snap +++ b/src/__tests__/__snapshots__/ActionMenu.tsx.snap @@ -70,9 +70,9 @@ exports[`ActionMenu renders consistently 1`] = ` -
- - -
- - ) -} - it('should focus element passed into function', async () => { const {getByText} = render() await waitFor(() => getByText('no')) const noButton = getByText('no') expect(document.activeElement).toEqual(noButton) }) - -it('should focus first element when nothing is passed', async () => { - const {getByText} = render() - await waitFor(() => getByText('yes')) - const yesButton = getByText('yes') - expect(document.activeElement).toEqual(yesButton) -}) diff --git a/src/hooks/useOpenAndCloseFocus.ts b/src/hooks/useOpenAndCloseFocus.ts index 63aa20952e1..1f4d47189b6 100644 --- a/src/hooks/useOpenAndCloseFocus.ts +++ b/src/hooks/useOpenAndCloseFocus.ts @@ -16,9 +16,6 @@ export function useOpenAndCloseFocus({ const returnRef = returnFocusRef.current if (initialFocusRef && initialFocusRef.current) { initialFocusRef.current.focus() - } else if (containerRef && containerRef.current) { - const firstItem = iterateFocusableElements(containerRef.current).next().value - firstItem?.focus() } return function () { returnRef?.focus() From dd117020f79091c358f83d1cb6bf22d3279dc346 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Tue, 13 Apr 2021 10:48:38 -0500 Subject: [PATCH 063/118] restore implicity initial focus functionality for useOpenAndCloseFocus hook --- src/ActionList/Item.tsx | 11 --- src/ActionMenu.tsx | 72 ++++++++++++++++++-- src/__tests__/hooks/useOpenAndCloseFocus.tsx | 22 ++++++ src/hooks/useOpenAndCloseFocus.ts | 3 + 4 files changed, 90 insertions(+), 18 deletions(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 8a0f0d9ca4d..a5809a434c7 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -84,21 +84,10 @@ const StyledTextContainer = styled.div<{descriptionVariant: ItemProps['descripti flex-direction: ${({descriptionVariant}) => (descriptionVariant === 'inline' ? 'row' : 'column')}; ` -<<<<<<< HEAD const BaseVisualContainer = styled.div` - { - /* Match visual height to adjacent text line height. - * - * TODO: When rem-based spacing on a 4px scale lands, replace - * hardcoded '20px' with '${get('space.s20')}'. - */ - } -======= -const LeadingVisualContainer = styled.div` // Match visual height to adjacent text line height. // TODO: When rem-based spacing on a 4px scale lands, replace // hardcoded '20px' with '${get('space.s20')}'. ->>>>>>> Complete implementation of keyboard nav in DropdownMenu. height: 20px; width: ${get('space.3')}; display: flex; diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index 2ca0d841301..3650a97f525 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -26,9 +26,57 @@ const ActionMenuBase = ({ }: ActionMenuProps): JSX.Element => { const anchorRef = useRef(null) const anchorId = `actionMenuAnchor-${randomId()}` - const [open, setOpen] = useState(false) - const onDismiss = useCallback(() => setOpen(false), [setOpen]) - const onToggle = useCallback(() => setOpen(!open), [setOpen, open]) + const [openState, setOpenState] = useState<'closed' | 'open' | 'ready'>('closed') + const [state, setState] = useState<'closed' | 'buttonFocus' | 'listFocus'>('closed') + + const onDismiss = useCallback(() => { + setOpenState('closed') + setState('closed') + }, [setOpenState]) + + const overlayRef = React.useRef(null) + + const onAnchorKeyDown = useCallback( + (event: React.KeyboardEvent) => { + if (!event.defaultPrevented) { + if (state === 'closed') { + if (event.key === 'ArrowDown' || event.key === 'ArrowUp') { + setState('listFocus') + setOpenState('open') + event.preventDefault() + } else if (event.key === ' ' || event.key === 'Enter') { + setState('buttonFocus') + setOpenState('open') + event.preventDefault() + } + } else if (state === 'buttonFocus') { + if (['ArrowDown', 'ArrowUp', 'Tab', 'Enter'].indexOf(event.key) !== -1) { + setState('listFocus') + event.preventDefault() + } else if (event.key === 'Escape') { + onDismiss() + event.preventDefault() + } + } + } + }, + [state, onDismiss] + ) + + const onAnchorClick = useCallback( + (event: React.MouseEvent) => { + if (!event.defaultPrevented && event.button === 0 && openState === 'closed') { + setOpenState('open') + setState('buttonFocus') + } + }, + [openState] + ) + + useFocusZone({containerRef: overlayRef, disabled: !(openState === 'ready' && state === 'listFocus')}) + useFocusTrap({containerRef: overlayRef, disabled: !(openState === 'ready' && state === 'listFocus')}) + // states: closed, buttonFocus, listFocus + return ( <> {renderAnchor({ @@ -41,8 +89,18 @@ const ActionMenuBase = ({ children: triggerContent, tabIndex: 0 })} - {open && ( - + {openState !== 'closed' && ( + { + setOpenState('ready') + }} + > { if (event.key == 'Enter' || event.key == 'Space') { onActivate && onActivate(itemProps as ItemProps) - setOpen(false) + onDismiss() } }, onClick: event => { onActivate && onActivate(itemProps as ItemProps) onClick && onClick(event) - setOpen(false) + onDismiss() } }) } diff --git a/src/__tests__/hooks/useOpenAndCloseFocus.tsx b/src/__tests__/hooks/useOpenAndCloseFocus.tsx index 20c48e03c4e..57d19af70cd 100644 --- a/src/__tests__/hooks/useOpenAndCloseFocus.tsx +++ b/src/__tests__/hooks/useOpenAndCloseFocus.tsx @@ -18,9 +18,31 @@ const Component = () => { ) } +const ComponentTwo = () => { + const buttonRef = useRef(null) + const containerRef = useRef(null) + useOpenAndCloseFocus({containerRef, returnFocusRef: buttonRef}) + return ( + <> + +
+ + +
+ + ) +} + it('should focus element passed into function', async () => { const {getByText} = render() await waitFor(() => getByText('no')) const noButton = getByText('no') expect(document.activeElement).toEqual(noButton) }) + +it('should focus first element when nothing is passed', async () => { + const {getByText} = render() + await waitFor(() => getByText('yes')) + const yesButton = getByText('yes') + expect(document.activeElement).toEqual(yesButton) +}) diff --git a/src/hooks/useOpenAndCloseFocus.ts b/src/hooks/useOpenAndCloseFocus.ts index 1f4d47189b6..63aa20952e1 100644 --- a/src/hooks/useOpenAndCloseFocus.ts +++ b/src/hooks/useOpenAndCloseFocus.ts @@ -16,6 +16,9 @@ export function useOpenAndCloseFocus({ const returnRef = returnFocusRef.current if (initialFocusRef && initialFocusRef.current) { initialFocusRef.current.focus() + } else if (containerRef && containerRef.current) { + const firstItem = iterateFocusableElements(containerRef.current).next().value + firstItem?.focus() } return function () { returnRef?.focus() From 2746286d1b80f8c1c6a8fe380a1014cbf6c8399f Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Tue, 13 Apr 2021 11:06:28 -0500 Subject: [PATCH 064/118] add focusTrap and focusZone dependencies, correct anchorClick handler --- src/ActionMenu.tsx | 4 +++- src/hooks/useOverlay.tsx | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index 3650a97f525..4a9723ea1b8 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -5,6 +5,8 @@ import Button, {ButtonProps} from './Button' import React, {useCallback, useRef, useState} from 'react' import Overlay from './Overlay' import randomId from './utils/randomId' +import {useFocusTrap} from './hooks/useFocusTrap' +import {useFocusZone} from './hooks/useFocusZone' export interface ActionMenuProps extends Partial>, ListPropsBase { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -85,7 +87,7 @@ const ActionMenuBase = ({ 'aria-labelledby': anchorId, 'aria-haspopup': 'listbox', 'aria-label': 'menu', - onClick: onToggle, + onClick: onAnchorClick, children: triggerContent, tabIndex: 0 })} diff --git a/src/hooks/useOverlay.tsx b/src/hooks/useOverlay.tsx index dcf7c1e80c4..fb564701beb 100644 --- a/src/hooks/useOverlay.tsx +++ b/src/hooks/useOverlay.tsx @@ -17,7 +17,6 @@ export type OverlayReturnProps = { } export const useOverlay = ({ - overlayRef: _overlayRef, returnFocusRef, initialFocusRef, onEscape, From 2a3c28865d287a68d886f3ba673e40d0a81c7bc9 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Tue, 13 Apr 2021 11:11:02 -0500 Subject: [PATCH 065/118] add onAnchorKeydown callback to anchor --- src/ActionMenu.tsx | 8 +++++--- src/Overlay.tsx | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index 4a9723ea1b8..4c4220f446d 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -7,6 +7,7 @@ import Overlay from './Overlay' import randomId from './utils/randomId' import {useFocusTrap} from './hooks/useFocusTrap' import {useFocusZone} from './hooks/useFocusZone' +import {useAnchoredPosition} from './hooks/useAnchoredPosition' export interface ActionMenuProps extends Partial>, ListPropsBase { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -75,6 +76,8 @@ const ActionMenuBase = ({ [openState] ) + const {position} = useAnchoredPosition({anchorElementRef: anchorRef, floatingElementRef: overlayRef}) + useFocusZone({containerRef: overlayRef, disabled: !(openState === 'ready' && state === 'listFocus')}) useFocusTrap({containerRef: overlayRef, disabled: !(openState === 'ready' && state === 'listFocus')}) // states: closed, buttonFocus, listFocus @@ -88,6 +91,7 @@ const ActionMenuBase = ({ 'aria-haspopup': 'listbox', 'aria-label': 'menu', onClick: onAnchorClick, + onkeydown: onAnchorKeyDown, children: triggerContent, tabIndex: 0 })} @@ -99,9 +103,7 @@ const ActionMenuBase = ({ onClickOutside={onDismiss} onEscape={onDismiss} ref={overlayRef} - onPositionChanged={() => { - setOpenState('ready') - }} + {...position} > [] initialFocusRef?: React.RefObject + anchorRef?: React.RefObject returnFocusRef: React.RefObject onClickOutside: (e: TouchOrMouseEvent) => void onEscape: (e: KeyboardEvent) => void From d196ecb8109373adf206443e0e55ff119bbead42 Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Tue, 13 Apr 2021 11:37:27 -0500 Subject: [PATCH 066/118] remove some extra cruft from merges --- .vscode/settings.json | 12 ------- @types/get-random-values/index.d.ts | 4 --- src/ActionList/Item.tsx | 6 ++-- src/ActionList/List.tsx | 6 +++- src/ActionMenu.tsx | 37 ++++++++------------- src/Overlay.tsx | 14 ++++---- src/hooks/useAnchoredPosition.ts | 1 - src/hooks/useOverlay.tsx | 2 +- src/stories/Overlay.stories.tsx | 2 -- src/stories/Portal.stories.tsx | 16 ++++----- src/stories/useAnchoredPosition.stories.tsx | 18 +++++----- src/stories/useFocusTrap.stories.tsx | 28 +++++----------- 12 files changed, 55 insertions(+), 91 deletions(-) delete mode 100644 .vscode/settings.json delete mode 100644 @types/get-random-values/index.d.ts diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index 03776c8cce2..00000000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "editor.formatOnSave": true, - "[javascript]": { - "editor.defaultFormatter": "esbenp.prettier-vscode" - }, - "[typescript]": { - "editor.defaultFormatter": "esbenp.prettier-vscode" - }, - "[typescriptreact]": { - "editor.defaultFormatter": "esbenp.prettier-vscode" - } -} diff --git a/@types/get-random-values/index.d.ts b/@types/get-random-values/index.d.ts deleted file mode 100644 index fe88efc8ce5..00000000000 --- a/@types/get-random-values/index.d.ts +++ /dev/null @@ -1,4 +0,0 @@ -declare module 'get-random-values' { - function random(arr: number[] | Uint8Array | Uint16Array): number[] | Uint8Array | Uint16Array - export = random -} diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index a5809a434c7..516a917d703 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -28,7 +28,7 @@ export interface ItemProps extends React.ComponentPropsWithoutRef<'div'>, SxProp descriptionVariant?: 'inline' | 'block' /** - * Icon (or similar) positioned after `Item` text. + * Icon (or similar) positioned before `Item` text. */ leadingVisual?: React.FunctionComponent @@ -68,12 +68,10 @@ const StyledItem = styled.div<{variant: ItemProps['variant']} & SxProp>` color: ${({variant}) => (variant === 'danger' ? get('colors.text.danger') : 'inherit')}; @media (hover: hover) and (pointer: fine) { - :hover, - :focus { + :hover { background: ${props => props.variant === 'danger' ? get('colors.bg.danger') : get('colors.selectMenu.tapHighlight')}; cursor: pointer; - outline: none; } } diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index 0d028a25ddb..99ca4a012e1 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -6,9 +6,13 @@ import {Divider} from './Divider' import styled from 'styled-components' import {get} from '../constants' import {SystemCssProperties} from '@styled-system/css' +<<<<<<< HEAD export type ItemInput = ItemProps | (Partial & {renderItem: typeof Item}) +======= +import type {AriaRole} from '../utils/types' +>>>>>>> remove some extra cruft from merges /** * Contract for props passed to the `List` component. */ @@ -182,7 +186,7 @@ export function List(props: ListProps): JSX.Element { } return ( - } {...props}> + {groups?.map(({header, ...groupProps}, index) => ( <> {renderGroup({ diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index 4c4220f446d..f2ce831cc1e 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -28,16 +28,15 @@ const ActionMenuBase = ({ ...listProps }: ActionMenuProps): JSX.Element => { const anchorRef = useRef(null) + const overlayRef = useRef(null) + const anchorId = `actionMenuAnchor-${randomId()}` - const [openState, setOpenState] = useState<'closed' | 'open' | 'ready'>('closed') + const [open, setOpen] = useState(false) const [state, setState] = useState<'closed' | 'buttonFocus' | 'listFocus'>('closed') - const onDismiss = useCallback(() => { - setOpenState('closed') + setOpen(false) setState('closed') - }, [setOpenState]) - - const overlayRef = React.useRef(null) + }, []) const onAnchorKeyDown = useCallback( (event: React.KeyboardEvent) => { @@ -45,11 +44,11 @@ const ActionMenuBase = ({ if (state === 'closed') { if (event.key === 'ArrowDown' || event.key === 'ArrowUp') { setState('listFocus') - setOpenState('open') + setOpen(true) event.preventDefault() } else if (event.key === ' ' || event.key === 'Enter') { setState('buttonFocus') - setOpenState('open') + setOpen(true) event.preventDefault() } } else if (state === 'buttonFocus') { @@ -57,6 +56,7 @@ const ActionMenuBase = ({ setState('listFocus') event.preventDefault() } else if (event.key === 'Escape') { + setState('closed') onDismiss() event.preventDefault() } @@ -65,22 +65,20 @@ const ActionMenuBase = ({ }, [state, onDismiss] ) - const onAnchorClick = useCallback( (event: React.MouseEvent) => { - if (!event.defaultPrevented && event.button === 0 && openState === 'closed') { - setOpenState('open') + if (!event.defaultPrevented && event.button === 0 && !open) { + setOpen(true) setState('buttonFocus') } }, - [openState] + [open] ) const {position} = useAnchoredPosition({anchorElementRef: anchorRef, floatingElementRef: overlayRef}) - useFocusZone({containerRef: overlayRef, disabled: !(openState === 'ready' && state === 'listFocus')}) - useFocusTrap({containerRef: overlayRef, disabled: !(openState === 'ready' && state === 'listFocus')}) - // states: closed, buttonFocus, listFocus + useFocusZone({containerRef: overlayRef, disabled: !open || state !== 'listFocus'}, [position]) + useFocusTrap({containerRef: overlayRef, disabled: !open || state !== 'listFocus'}, [position]) return ( <> @@ -95,10 +93,9 @@ const ActionMenuBase = ({ children: triggerContent, tabIndex: 0 })} - {openState !== 'closed' && ( + {open && ( { - if (event.key == 'Enter' || event.key == 'Space') { - onActivate && onActivate(itemProps as ItemProps) - onDismiss() - } - }, onClick: event => { onActivate && onActivate(itemProps as ItemProps) onClick && onClick(event) diff --git a/src/Overlay.tsx b/src/Overlay.tsx index 63f429f92c2..547ec2ccc54 100644 --- a/src/Overlay.tsx +++ b/src/Overlay.tsx @@ -1,5 +1,9 @@ import styled from 'styled-components' +<<<<<<< HEAD import React, {ReactElement, useRef} from 'react' +======= +import React, {ReactElement} from 'react' +>>>>>>> remove some extra cruft from merges import {get, COMMON, POSITION, SystemPositionProps, SystemCommonProps} from './constants' import {ComponentProps} from './utils/types' import {useOverlay, TouchOrMouseEvent} from './hooks' @@ -55,7 +59,6 @@ const StyledOverlay = styled.div[] initialFocusRef?: React.RefObject - anchorRef?: React.RefObject returnFocusRef: React.RefObject onClickOutside: (e: TouchOrMouseEvent) => void onEscape: (e: KeyboardEvent) => void @@ -70,7 +73,6 @@ export type OverlayProps = { * @param returnFocusRef Required. Ref for the element to focus when the `Overlay` is closed. * @param onClickOutside Required. Function to call when clicking outside of the `Overlay`. Typically this function sets the `Overlay` visibility state to `false`. * @param onEscape Required. Function to call when user presses `Escape`. Typically this function sets the `Overlay` visibility state to `false`. - * @param onPositionUpdated Optional. * @param width Sets the width of the `Overlay`, pick from our set list of widths, or pass `auto` to automatically set the width based on the content of the `Overlay`. `sm` corresponds to `256px`, `md` corresponds to `320px`, `lg` corresponds to `480px`, and `xl` corresponds to `640px`. * @param height Sets the height of the `Overlay`, pick from our set list of heights, or pass `auto` to automatically set the height based on the content of the `Overlay`. `sm` corresponds to `480px` and `md` corresponds to `640px`. */ @@ -98,9 +100,9 @@ const Overlay = React.forwardRef( } ) -// Overlay.defaultProps = { -// height: 'auto', -// width: 'auto' -// } +Overlay.defaultProps = { + height: 'auto', + width: 'auto' +} export default Overlay diff --git a/src/hooks/useAnchoredPosition.ts b/src/hooks/useAnchoredPosition.ts index 53d83c17fa0..1e5f6e6be43 100644 --- a/src/hooks/useAnchoredPosition.ts +++ b/src/hooks/useAnchoredPosition.ts @@ -28,7 +28,6 @@ export function useAnchoredPosition( const anchorElementRef = useProvidedRefOrCreate(settings?.anchorElementRef) const [position, setPosition] = React.useState<{top: number; left: number} | undefined>(undefined) React.useEffect(() => { - console.log('anchored position effect') if (floatingElementRef.current instanceof Element && anchorElementRef.current instanceof Element) { setPosition(getAnchoredPosition(floatingElementRef.current, anchorElementRef.current, settings)) } else { diff --git a/src/hooks/useOverlay.tsx b/src/hooks/useOverlay.tsx index fb564701beb..0945275d2be 100644 --- a/src/hooks/useOverlay.tsx +++ b/src/hooks/useOverlay.tsx @@ -17,11 +17,11 @@ export type OverlayReturnProps = { } export const useOverlay = ({ + overlayRef: _overlayRef, returnFocusRef, initialFocusRef, onEscape, ignoreClickRefs, - overlayRef: _overlayRef, onClickOutside }: UseOverlaySettings): OverlayReturnProps => { const overlayRef = useProvidedRefOrCreate(_overlayRef) diff --git a/src/stories/Overlay.stories.tsx b/src/stories/Overlay.stories.tsx index 631b4daa1f4..4cf66e4aec3 100644 --- a/src/stories/Overlay.stories.tsx +++ b/src/stories/Overlay.stories.tsx @@ -47,7 +47,6 @@ const DummyItem = styled.button` export const DropdownOverlay = () => { const [isOpen, setIsOpen] = useState(false) const buttonRef = useRef(null) - const ref = React.useRef(null) return ( <> ) ) From 710123afbb5548be9405e42d8465b80894764e43 Mon Sep 17 00:00:00 2001 From: dgreif Date: Thu, 15 Apr 2021 15:13:42 -0700 Subject: [PATCH 087/118] refactor: use ternary for conditional child component rendering --- src/DropdownMenu/DropdownMenu.tsx | 4 ++-- src/__tests__/Overlay.tsx | 4 ++-- src/stories/Overlay.stories.tsx | 8 ++++---- src/stories/Portal.stories.tsx | 8 ++++---- src/stories/useAnchoredPosition.stories.tsx | 4 ++-- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index e715356bd47..5f0b0b07fd1 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -111,7 +111,7 @@ export function DropdownMenu({ onClick: onAnchorClick, onKeyDown: onAnchorKeyDown })} - {open && ( + {open ? ( - )} + ) : null} ) } diff --git a/src/__tests__/Overlay.tsx b/src/__tests__/Overlay.tsx index ecf9b112754..f1071bc33d8 100644 --- a/src/__tests__/Overlay.tsx +++ b/src/__tests__/Overlay.tsx @@ -27,7 +27,7 @@ const TestComponent = ({initialFocus, callback}: TestComponentSettings) => { open overlay - {isOpen && ( + {isOpen ? ( { - )} + ) : null} ) } diff --git a/src/stories/Overlay.stories.tsx b/src/stories/Overlay.stories.tsx index 4cf66e4aec3..890981f04f9 100644 --- a/src/stories/Overlay.stories.tsx +++ b/src/stories/Overlay.stories.tsx @@ -52,7 +52,7 @@ export const DropdownOverlay = () => { - {isOpen && ( + {isOpen ? ( { Delete - )} + ) : null} ) } @@ -85,7 +85,7 @@ export const DialogOverlay = () => { - {isOpen && ( + {isOpen ? ( { - )} + ) : null} ) } diff --git a/src/stories/Portal.stories.tsx b/src/stories/Portal.stories.tsx index c65e38d220f..c12e902f0bb 100644 --- a/src/stories/Portal.stories.tsx +++ b/src/stories/Portal.stories.tsx @@ -65,7 +65,7 @@ export const CustomPortalRootByRegistration: React.FC> = ( <> Root position - {mounted && ( + {mounted ? ( <> Outer container @@ -73,7 +73,7 @@ export const CustomPortalRootByRegistration: React.FC> = ( Portaled content rendered at the outer container. - )} + ) : null} ) @@ -96,7 +96,7 @@ export const MultiplePortalRoots: React.FC> = () => { Outer container - {mounted && ( + {mounted ? ( <> Portaled content rendered at the outer container. Portaled content rendered at the end of the inner container. @@ -104,7 +104,7 @@ export const MultiplePortalRoots: React.FC> = () => { Portaled content rendered at <BaseStyles> root. - )} + ) : null} Inner container diff --git a/src/stories/useAnchoredPosition.stories.tsx b/src/stories/useAnchoredPosition.stories.tsx index 0c510883f48..3bbdc6b9a3e 100644 --- a/src/stories/useAnchoredPosition.stories.tsx +++ b/src/stories/useAnchoredPosition.stories.tsx @@ -295,7 +295,7 @@ export const WithPortal = () => { }> Show the overlay! - {showMenu && ( + {showMenu ? ( } @@ -308,7 +308,7 @@ export const WithPortal = () => { An un-constrained overlay! - )} + ) : null} From c64b97020c6b00efdd24ae97446e0cddf9e0b72b Mon Sep 17 00:00:00 2001 From: dgreif Date: Thu, 15 Apr 2021 15:18:04 -0700 Subject: [PATCH 088/118] test: update snapshot for DropdownMenu --- src/__tests__/__snapshots__/DropdownMenu.tsx.snap | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/__tests__/__snapshots__/DropdownMenu.tsx.snap b/src/__tests__/__snapshots__/DropdownMenu.tsx.snap index f746c8dcc1d..4929092187a 100644 --- a/src/__tests__/__snapshots__/DropdownMenu.tsx.snap +++ b/src/__tests__/__snapshots__/DropdownMenu.tsx.snap @@ -67,6 +67,10 @@ exports[`DropdownMenu renders consistently 1`] = ` border-color: rgba(27,31,35,0.15); } +.c1 { + margin-left: 4px; +} + , + anchorContent, + renderAnchor = (props: T) => , renderItem = Item, onAction, ...listProps @@ -90,7 +90,7 @@ const ActionMenuBase = ({ 'aria-label': 'menu', onClick: onAnchorClick, onKeyDown: onAnchorKeyDown, - children: triggerContent, + children: anchorContent, tabIndex: 0 })} {open ? ( diff --git a/src/__tests__/ActionMenu.tsx b/src/__tests__/ActionMenu.tsx index 46032071336..4831b3778ad 100644 --- a/src/__tests__/ActionMenu.tsx +++ b/src/__tests__/ActionMenu.tsx @@ -25,7 +25,7 @@ function SimpleActionMenu(): JSX.Element {
X
- +
diff --git a/src/stories/ActionMenu.stories.tsx b/src/stories/ActionMenu.stories.tsx index a4f7d228403..01c7b7489f9 100644 --- a/src/stories/ActionMenu.stories.tsx +++ b/src/stories/ActionMenu.stories.tsx @@ -62,7 +62,7 @@ export function ActionsStory(): JSX.Element { } + anchorContent={} items={[ { leadingVisual: ServerIcon, @@ -99,7 +99,7 @@ export function SimpleListStory(): JSX.Element { Date: Thu, 15 Apr 2021 17:44:23 -0500 Subject: [PATCH 093/118] add renderAnchor to ActionMenu docs --- docs/content/ActionMenu.mdx | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/content/ActionMenu.mdx b/docs/content/ActionMenu.mdx index 08471db67b5..aa2865c070c 100644 --- a/docs/content/ActionMenu.mdx +++ b/docs/content/ActionMenu.mdx @@ -22,10 +22,11 @@ An `ActionMenu` is a simple ActionList based component for creating a menu of ac ## 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. | -| 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. | From a2d4df11cdfee7bc46295a6f07690e6d2366b6df Mon Sep 17 00:00:00 2001 From: Van Anderson Date: Thu, 15 Apr 2021 17:39:11 -0500 Subject: [PATCH 094/118] Change wording in docs/content/ActionMenu.mdx from @colebemis: I try to avoid using words like simple and just because it can make readers feel discouraged if they don't find it simple (https://jameshfisher.com/2017/02/22/dont-use-simply/). Co-authored-by: Cole Bemis --- docs/content/ActionMenu.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/ActionMenu.mdx b/docs/content/ActionMenu.mdx index aa2865c070c..d8e195bebe0 100644 --- a/docs/content/ActionMenu.mdx +++ b/docs/content/ActionMenu.mdx @@ -2,7 +2,7 @@ title: ActionMenu --- -An `ActionMenu` is a simple ActionList based component for creating a menu of actions that expands through a trigger button. +An `ActionMenu` is an ActionList-based component for creating a menu of actions that expands through a trigger button. ## Default example From 42e865895c10efe504af3d2c3f9f5e201a441afb Mon Sep 17 00:00:00 2001 From: dgreif Date: Thu, 15 Apr 2021 15:56:28 -0700 Subject: [PATCH 095/118] fix: cleanup after merging master --- src/__tests__/DropdownMenu.tsx | 3 --- src/behaviors/focusTrap.ts | 2 -- src/stories/DropdownMenu.stories.tsx | 4 ---- 3 files changed, 9 deletions(-) diff --git a/src/__tests__/DropdownMenu.tsx b/src/__tests__/DropdownMenu.tsx index ea1ffb4f6fe..41ec2a91d20 100644 --- a/src/__tests__/DropdownMenu.tsx +++ b/src/__tests__/DropdownMenu.tsx @@ -7,7 +7,6 @@ import {DropdownMenu, DropdownButton} from '../DropdownMenu' import {COMMON} from '../constants' import {behavesAsComponent, checkExports} from '../utils/testing' import {BaseStyles, ThemeProvider} from '..' -import {registerPortalRoot} from '../Portal/index' import {ItemInput} from '../ActionList/List' expect.extend(toHaveNoViolations) @@ -35,8 +34,6 @@ function SimpleDropdownMenu(): JSX.Element { describe('DropdownMenu', () => { afterEach(() => { - // since the registry is global, reset after each test - registerPortalRoot(undefined) jest.clearAllMocks() }) diff --git a/src/behaviors/focusTrap.ts b/src/behaviors/focusTrap.ts index eaa866375d3..323af02a952 100644 --- a/src/behaviors/focusTrap.ts +++ b/src/behaviors/focusTrap.ts @@ -1,6 +1,5 @@ import {isTabbable, iterateFocusableElements} from '../utils/iterateFocusableElements' import {polyfill as eventListenerSignalPolyfill} from '../polyfills/eventListenerSignal' -import {prepareForFocusWithoutMouse} from '../hooks/useMouseIntent' eventListenerSignalPolyfill() @@ -84,7 +83,6 @@ export function focusTrap( } else { const toFocus = initialFocus && container.contains(initialFocus) ? initialFocus : getFocusableChild(container) if (toFocus) { - prepareForFocusWithoutMouse() toFocus.focus() return } else { diff --git a/src/stories/DropdownMenu.stories.tsx b/src/stories/DropdownMenu.stories.tsx index 16f89b0b259..d204df8d413 100644 --- a/src/stories/DropdownMenu.stories.tsx +++ b/src/stories/DropdownMenu.stories.tsx @@ -4,16 +4,12 @@ import {theme, ThemeProvider} from '..' import {ItemInput} from '../ActionList/List' import BaseStyles from '../BaseStyles' import {DropdownMenu, DropdownButton} from '../DropdownMenu' -import {registerPortalRoot} from '../Portal' const meta: Meta = { title: 'Composite components/DropdownMenu', component: DropdownMenu, decorators: [ (Story: React.ComponentType): JSX.Element => { - // Since portal roots are registered globally, we need this line so that each storybook - // story works in isolation. - registerPortalRoot(undefined) return ( From 7e74ab11283dd3f7de2668a5aebb777190ef0df4 Mon Sep 17 00:00:00 2001 From: dgreif Date: Thu, 15 Apr 2021 16:01:58 -0700 Subject: [PATCH 096/118] refactor: `setSelectedItem` -> `onChange` --- docs/content/DropdownMenu.mdx | 4 ++-- src/DropdownMenu/DropdownMenu.tsx | 10 +++++----- src/__tests__/DropdownMenu.tsx | 2 +- src/stories/DropdownMenu.stories.tsx | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/content/DropdownMenu.mdx b/docs/content/DropdownMenu.mdx index 7eaa67cab6c..4350b595209 100644 --- a/docs/content/DropdownMenu.mdx +++ b/docs/content/DropdownMenu.mdx @@ -2,7 +2,7 @@ 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 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 @@ -21,7 +21,7 @@ function DemoComponent() { placeholder="🎨" items={items} selectedItem={selectedItem} - setSelectedItem={setSelectedItem} + onChange={setSelectedItem} /> ) } diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 5f0b0b07fd1..fc85a829d2d 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -31,20 +31,20 @@ export interface DropdownMenuProps extends Partial unknown + onChange?: (item?: ItemInput) => unknown } /** * 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 anchor contents will be updated - * with the selection. + * 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. */ export function DropdownMenu({ renderAnchor = (props: T) => , renderItem = Item, placeholder, selectedItem, - setSelectedItem, + onChange, ...listProps }: DropdownMenuProps): JSX.Element { const anchorRef = useRef(null) @@ -125,7 +125,7 @@ export function DropdownMenu({ role="listbox" renderItem={({onClick, onKeyDown, item, ...itemProps}) => { const itemActivated = () => { - setSelectedItem?.(item === selectedItem ? undefined : item) + onChange?.(item === selectedItem ? undefined : item) onDismiss() } diff --git a/src/__tests__/DropdownMenu.tsx b/src/__tests__/DropdownMenu.tsx index 41ec2a91d20..e6ae97388be 100644 --- a/src/__tests__/DropdownMenu.tsx +++ b/src/__tests__/DropdownMenu.tsx @@ -24,7 +24,7 @@ function SimpleDropdownMenu(): JSX.Element { items={items} placeholder="Select an Option" selectedItem={selectedItem} - setSelectedItem={setSelectedItem} + onChange={setSelectedItem} />
diff --git a/src/stories/DropdownMenu.stories.tsx b/src/stories/DropdownMenu.stories.tsx index d204df8d413..ca0112d34a3 100644 --- a/src/stories/DropdownMenu.stories.tsx +++ b/src/stories/DropdownMenu.stories.tsx @@ -44,7 +44,7 @@ export function FavoriteColorStory(): JSX.Element { placeholder="🎨" items={items} selectedItem={selectedItem} - setSelectedItem={setSelectedItem} + onChange={setSelectedItem} /> ) From f8b0609872f79cc4ebcca0c24d34a23f8f26801b Mon Sep 17 00:00:00 2001 From: dgreif Date: Fri, 16 Apr 2021 08:01:25 -0700 Subject: [PATCH 097/118] chore: add comment explaining lack of DropdownMenu type exports --- src/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/index.ts b/src/index.ts index e01751d75e6..2357ce448ce 100644 --- a/src/index.ts +++ b/src/index.ts @@ -78,6 +78,8 @@ export type { DropdownMenuProps } from './Dropdown' export {DropdownButton, DropdownMenu} from './DropdownMenu' +// not exporting new DropdownMenu types yet due to conflict with Dropdown types above +// export type {DropdownButtonProps, DropdownMenuProps} from './DropdownMenu' export {default as FilteredSearch} from './FilteredSearch' export type {FilteredSearchProps} from './FilteredSearch' export {default as FilterList} from './FilterList' From 744092891c1ce3d70960c9ba05fd5873c5fc218c Mon Sep 17 00:00:00 2001 From: Dusty Greif Date: Fri, 16 Apr 2021 08:37:25 -0700 Subject: [PATCH 098/118] refactor: `includes` instead of `indexOf` Co-authored-by: Trevor Gau --- src/DropdownMenu/DropdownMenu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index fc85a829d2d..6b1478c9740 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -73,7 +73,7 @@ export function DropdownMenu({ event.preventDefault() } } else if (focusType === 'anchor') { - if (['ArrowDown', 'ArrowUp', 'Tab', 'Enter'].indexOf(event.key) !== -1) { + if (['ArrowDown', 'ArrowUp', 'Tab', 'Enter'].includes(event.key)) { setFocusType('list') event.preventDefault() } else if (event.key === 'Escape') { From 77a2e38d29851b1d6beada926929ac81b4da0768 Mon Sep 17 00:00:00 2001 From: Dusty Greif Date: Fri, 16 Apr 2021 10:28:51 -0700 Subject: [PATCH 099/118] docs: replace `setSelectedItem` with `onChange` Co-authored-by: Cole Bemis --- docs/content/DropdownMenu.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/DropdownMenu.mdx b/docs/content/DropdownMenu.mdx index 4350b595209..87997da4944 100644 --- a/docs/content/DropdownMenu.mdx +++ b/docs/content/DropdownMenu.mdx @@ -35,7 +35,7 @@ render() | :--------------- | :-------------------------------------------- | :---------------: | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | 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. | -| setSelectedItem? | (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. | +| 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. | From 848d2fd174f0db00e854b1fdb191f8fdb838ebfe Mon Sep 17 00:00:00 2001 From: dgreif Date: Fri, 16 Apr 2021 08:59:27 -0700 Subject: [PATCH 100/118] refactor: default value for dependency arrays --- src/hooks/useFocusTrap.ts | 4 ++-- src/hooks/useFocusZone.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hooks/useFocusTrap.ts b/src/hooks/useFocusTrap.ts index 9e4d0164865..ccdce1a845d 100644 --- a/src/hooks/useFocusTrap.ts +++ b/src/hooks/useFocusTrap.ts @@ -29,7 +29,7 @@ interface FocusTrapHookSettings { */ export function useFocusTrap( settings?: FocusTrapHookSettings, - dependencies?: React.DependencyList + dependencies: React.DependencyList = [] ): {containerRef: React.RefObject; initialFocusRef: React.RefObject} { const containerRef = useProvidedRefOrCreate(settings?.containerRef) const initialFocusRef = useProvidedRefOrCreate(settings?.initialFocusRef) @@ -50,7 +50,7 @@ export function useFocusTrap( } }, // eslint-disable-next-line react-hooks/exhaustive-deps - [containerRef, initialFocusRef, disabled, ...(dependencies ?? [])] + [containerRef, initialFocusRef, disabled, ...dependencies] ) return {containerRef, initialFocusRef} diff --git a/src/hooks/useFocusZone.ts b/src/hooks/useFocusZone.ts index 52860b6f80f..100ee1b024f 100644 --- a/src/hooks/useFocusZone.ts +++ b/src/hooks/useFocusZone.ts @@ -24,7 +24,7 @@ export interface FocusZoneHookSettings extends Omit; activeDescendantControlRef: React.RefObject} { const containerRef = useProvidedRefOrCreate(settings.containerRef) const useActiveDescendant = !!settings.activeDescendantFocus @@ -57,7 +57,7 @@ export function useFocusZone( } }, // eslint-disable-next-line react-hooks/exhaustive-deps - [disabled, ...(dependencies ?? [])] + [disabled, ...dependencies] ) return {containerRef, activeDescendantControlRef} From 9735116d1693b7974b98076ad85637686b368d61 Mon Sep 17 00:00:00 2001 From: Dusty Greif Date: Fri, 16 Apr 2021 10:40:31 -0700 Subject: [PATCH 101/118] refactor: simplify event key checks Co-authored-by: Cole Bemis --- src/DropdownMenu/DropdownMenu.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 6b1478c9740..f4b2a8339ff 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -63,11 +63,11 @@ export function DropdownMenu({ (event: React.KeyboardEvent) => { if (!event.defaultPrevented) { if (!open) { - if (event.key === 'ArrowDown' || event.key === 'ArrowUp') { + if (['ArrowDown', 'ArrowUp'].includes(event.key)) { setFocusType('list') setOpen(true) event.preventDefault() - } else if (event.key === ' ' || event.key === 'Enter') { + } else if ([' ', 'Enter'].includes(event.key)) { setFocusType('anchor') setOpen(true) event.preventDefault() From 04946b4c6e90ad0a5c79b13e571a4417d75c742c Mon Sep 17 00:00:00 2001 From: dgreif Date: Fri, 16 Apr 2021 10:41:27 -0700 Subject: [PATCH 102/118] chore: remove display name for `DropdownButton` --- src/DropdownMenu/DropdownButton.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/DropdownMenu/DropdownButton.tsx b/src/DropdownMenu/DropdownButton.tsx index 032da9e7641..df2a7f36d87 100644 --- a/src/DropdownMenu/DropdownButton.tsx +++ b/src/DropdownMenu/DropdownButton.tsx @@ -13,5 +13,3 @@ export const DropdownButton = React.forwardRef ) ) - -DropdownButton.displayName = 'DropdownMenu.Button' From 75f9ed5a1591fdfa9123ba3c0aa08f6423c27c21 Mon Sep 17 00:00:00 2001 From: Dusty Greif Date: Fri, 16 Apr 2021 10:45:49 -0700 Subject: [PATCH 103/118] refactor: optional chaining for item event handlers Co-authored-by: Cole Bemis --- src/DropdownMenu/DropdownMenu.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index f4b2a8339ff..76b928e7423 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -136,7 +136,7 @@ export function DropdownMenu({ selected: item === selectedItem, onClick: event => { itemActivated() - onClick && onClick(event) + onClick?.(event) }, onKeyDown: event => { if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) { @@ -144,7 +144,7 @@ export function DropdownMenu({ // prevent "Enter" event from becoming a click on the anchor as overlay closes event.preventDefault() } - onKeyDown && onKeyDown(event) + onKeyDown?.(event) } }) }} From 33c43745d14a8a8d8204dad903c251a22aee8888 Mon Sep 17 00:00:00 2001 From: dgreif Date: Fri, 16 Apr 2021 10:44:15 -0700 Subject: [PATCH 104/118] refactor: `itemActivated` -> `handleSelection` --- src/DropdownMenu/DropdownMenu.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 76b928e7423..05ffe54516b 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -124,7 +124,7 @@ export function DropdownMenu({ {...listProps} role="listbox" renderItem={({onClick, onKeyDown, item, ...itemProps}) => { - const itemActivated = () => { + const handleSelection = () => { onChange?.(item === selectedItem ? undefined : item) onDismiss() } @@ -135,12 +135,12 @@ export function DropdownMenu({ role: 'option', selected: item === selectedItem, onClick: event => { - itemActivated() + handleSelection() onClick?.(event) }, onKeyDown: event => { if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) { - itemActivated() + handleSelection() // prevent "Enter" event from becoming a click on the anchor as overlay closes event.preventDefault() } From 8eabeeed67e13b5a8a1d3d1ad2ac33f14c962c9b Mon Sep 17 00:00:00 2001 From: dgreif Date: Fri, 16 Apr 2021 10:53:39 -0700 Subject: [PATCH 105/118] refactor: remove `randomId` in favor of `uniqueId` --- src/DropdownMenu/DropdownMenu.tsx | 4 ++-- src/__tests__/__snapshots__/DropdownMenu.tsx.snap | 4 ++-- src/utils/randomId.tsx | 3 --- src/utils/testing.tsx | 5 ----- 4 files changed, 4 insertions(+), 12 deletions(-) delete mode 100644 src/utils/randomId.tsx diff --git a/src/DropdownMenu/DropdownMenu.tsx b/src/DropdownMenu/DropdownMenu.tsx index 05ffe54516b..e378bab0853 100644 --- a/src/DropdownMenu/DropdownMenu.tsx +++ b/src/DropdownMenu/DropdownMenu.tsx @@ -7,7 +7,7 @@ import {useFocusTrap} from '../hooks/useFocusTrap' import {useFocusZone} from '../hooks/useFocusZone' import {useAnchoredPosition} from '../hooks/useAnchoredPosition' import {useRenderForcingRef} from '../hooks/useRenderForcingRef' -import randomId from '../utils/randomId' +import {uniqueId} from '../utils/uniqueId' export interface DropdownMenuProps extends Partial>, ListPropsBase { /** @@ -50,7 +50,7 @@ export function DropdownMenu({ const anchorRef = useRef(null) const [overlayRef, updateOverlayRef] = useRenderForcingRef() - const anchorId = `dropdownMenuAnchor-${randomId()}` + const anchorId = `dropdownMenuAnchor-${uniqueId()}` const [open, setOpen] = useState(false) const [focusType, setFocusType] = useState(null) diff --git a/src/__tests__/__snapshots__/DropdownMenu.tsx.snap b/src/__tests__/__snapshots__/DropdownMenu.tsx.snap index 4929092187a..0885577466b 100644 --- a/src/__tests__/__snapshots__/DropdownMenu.tsx.snap +++ b/src/__tests__/__snapshots__/DropdownMenu.tsx.snap @@ -73,9 +73,9 @@ exports[`DropdownMenu renders consistently 1`] = `