From e6a173b74174cdf52cdbd4cd6eec6269d0008c3f Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Tue, 7 May 2024 12:05:14 -0400 Subject: [PATCH] Revert "Revert "Add support for nested submenus to `ActionMenu`"" (#4486) * Revert "Revert "Add support for nested submenus to `ActionMenu` (#4386)" (#4472)" This reverts commit 82072eb7169578bb686c07c64b3dd52eac764e38. * just want a change to trigger rebuild --------- Co-authored-by: Siddharth Kshetrapal Co-authored-by: Pavithra Kodmad --- .changeset/wild-students-bow.md | 5 + .../ActionList/ActionListContainerContext.tsx | 1 + packages/react/src/ActionList/Item.tsx | 14 +- .../ActionMenu.features.stories.tsx | 53 +++++- packages/react/src/ActionMenu/ActionMenu.tsx | 96 ++++++++-- .../react/src/__tests__/ActionMenu.test.tsx | 170 +++++++++++++++++- .../src/hooks/useMenuKeyboardNavigation.ts | 31 +++- 7 files changed, 348 insertions(+), 22 deletions(-) create mode 100644 .changeset/wild-students-bow.md diff --git a/.changeset/wild-students-bow.md b/.changeset/wild-students-bow.md new file mode 100644 index 00000000000..111e2d290e9 --- /dev/null +++ b/.changeset/wild-students-bow.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +Adds support for nested submenus to `ActionMenu` diff --git a/packages/react/src/ActionList/ActionListContainerContext.tsx b/packages/react/src/ActionList/ActionListContainerContext.tsx index 1127042aa56..57370225370 100644 --- a/packages/react/src/ActionList/ActionListContainerContext.tsx +++ b/packages/react/src/ActionList/ActionListContainerContext.tsx @@ -14,6 +14,7 @@ type ContextProps = { // eslint-disable-next-line @typescript-eslint/ban-types afterSelect?: Function enableFocusZone?: boolean + defaultTrailingVisual?: React.ReactElement } export const ActionListContainerContext = React.createContext({}) diff --git a/packages/react/src/ActionList/Item.tsx b/packages/react/src/ActionList/Item.tsx index f75a6e5d826..517f188b0ba 100644 --- a/packages/react/src/ActionList/Item.tsx +++ b/packages/react/src/ActionList/Item.tsx @@ -74,6 +74,15 @@ export const Item = React.forwardRef( inlineDescription: [Description, props => props.variant !== 'block'], }) + const {container, afterSelect, selectionAttribute, defaultTrailingVisual} = + React.useContext(ActionListContainerContext) + + // Be sure to avoid rendering the container unless there is a default + const wrappedDefaultTrailingVisual = defaultTrailingVisual ? ( + {defaultTrailingVisual} + ) : null + const trailingVisual = slots.trailingVisual ?? wrappedDefaultTrailingVisual + const { variant: listVariant, role: listRole, @@ -81,7 +90,6 @@ export const Item = React.forwardRef( selectionVariant: listSelectionVariant, } = React.useContext(ListContext) const {selectionVariant: groupSelectionVariant} = React.useContext(GroupContext) - const {container, afterSelect, selectionAttribute} = React.useContext(ActionListContainerContext) const inactive = Boolean(inactiveText) const showInactiveIndicator = inactive && container === undefined @@ -308,7 +316,7 @@ export const Item = React.forwardRef( sx={{display: 'flex', flexDirection: 'column', flexGrow: 1, minWidth: 0}} > ( ) : ( // If it's not inactive, or it has a leading visual that can be replaced, // just render the trailing visual slot. - slots.trailingVisual + trailingVisual ) } diff --git a/packages/react/src/ActionMenu/ActionMenu.features.stories.tsx b/packages/react/src/ActionMenu/ActionMenu.features.stories.tsx index f417e8cb1f4..670e3b49cc8 100644 --- a/packages/react/src/ActionMenu/ActionMenu.features.stories.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.features.stories.tsx @@ -1,6 +1,15 @@ import React from 'react' import {ActionMenu, ActionList, Box} from '../' -import {WorkflowIcon, ArchiveIcon, GearIcon, CopyIcon, RocketIcon, CommentIcon, BookIcon} from '@primer/octicons-react' +import { + WorkflowIcon, + ArchiveIcon, + GearIcon, + CopyIcon, + RocketIcon, + CommentIcon, + BookIcon, + SparkleFillIcon, +} from '@primer/octicons-react' export default { title: 'Components/ActionMenu/Features', @@ -181,3 +190,45 @@ export const InactiveItems = () => ( ) + +export const Submenus = () => ( + + Edit + + + Cut + Copy + Paste + + + + + + + Paste special + + + + + Paste plain text + Paste formulas + Paste with formatting + + + Paste from + + + + Current clipboard + History + Another device + + + + + + + + + +) diff --git a/packages/react/src/ActionMenu/ActionMenu.tsx b/packages/react/src/ActionMenu/ActionMenu.tsx index d33212d8a87..619272c40b1 100644 --- a/packages/react/src/ActionMenu/ActionMenu.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.tsx @@ -1,5 +1,5 @@ -import React, {useEffect, useState} from 'react' -import {TriangleDownIcon} from '@primer/octicons-react' +import React, {useCallback, useContext, useMemo, useEffect, useState} from 'react' +import {TriangleDownIcon, ChevronRightIcon} from '@primer/octicons-react' import type {AnchoredOverlayProps} from '../AnchoredOverlay' import {AnchoredOverlay} from '../AnchoredOverlay' import type {OverlayProps} from '../Overlay' @@ -13,11 +13,16 @@ import type {MandateProps} from '../utils/types' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {Tooltip} from '../TooltipV2/Tooltip' +export type MenuCloseHandler = ( + gesture: 'anchor-click' | 'click-outside' | 'escape' | 'tab' | 'item-select' | 'arrow-left', +) => void + export type MenuContextProps = Pick< AnchoredOverlayProps, 'anchorRef' | 'renderAnchor' | 'open' | 'onOpen' | 'anchorId' > & { - onClose?: (gesture: 'anchor-click' | 'click-outside' | 'escape' | 'tab') => void + onClose?: MenuCloseHandler + isSubmenu?: boolean } const MenuContext = React.createContext({renderAnchor: null, open: false}) @@ -44,9 +49,23 @@ const Menu: React.FC> = ({ onOpenChange, children, }: ActionMenuProps) => { + const parentMenuContext = useContext(MenuContext) + const [combinedOpenState, setCombinedOpenState] = useProvidedStateOrCreate(open, onOpenChange, false) const onOpen = React.useCallback(() => setCombinedOpenState(true), [setCombinedOpenState]) - const onClose = React.useCallback(() => setCombinedOpenState(false), [setCombinedOpenState]) + const onClose: MenuCloseHandler = React.useCallback( + gesture => { + setCombinedOpenState(false) + + // Close the parent stack when an item is selected or the user tabs out of the menu entirely + switch (gesture) { + case 'tab': + case 'item-select': + parentMenuContext.onClose?.(gesture) + } + }, + [setCombinedOpenState, parentMenuContext], + ) const menuButtonChild = React.Children.toArray(children).find( child => React.isValidElement(child) && (child.type === MenuButton || child.type === Anchor), @@ -100,7 +119,18 @@ const Menu: React.FC> = ({ }) return ( - + {contents} ) @@ -108,7 +138,40 @@ const Menu: React.FC> = ({ export type ActionMenuAnchorProps = {children: React.ReactElement; id?: string} const Anchor = React.forwardRef(({children, ...anchorProps}, anchorRef) => { - return React.cloneElement(children, {...anchorProps, ref: anchorRef}) + const {onOpen, isSubmenu} = React.useContext(MenuContext) + + const openSubmenuOnRightArrow: React.KeyboardEventHandler = useCallback( + event => { + children.props.onKeyDown?.(event) + if (isSubmenu && event.key === 'ArrowRight' && !event.defaultPrevented) onOpen?.('anchor-key-press') + }, + [children, isSubmenu, onOpen], + ) + + // Add right chevron icon to submenu anchors rendered using `ActionList.Item` + const parentActionListContext = useContext(ActionListContainerContext) + const thisActionListContext = useMemo( + () => + isSubmenu + ? { + ...parentActionListContext, + defaultTrailingVisual: , + // Default behavior is to close after selecting; we want to open the submenu instead + afterSelect: () => onOpen?.('anchor-click'), + } + : parentActionListContext, + [isSubmenu, onOpen, parentActionListContext], + ) + + return ( + + {React.cloneElement(children, { + ...anchorProps, + ref: anchorRef, + onKeyDown: openSubmenuOnRightArrow, + })} + + ) }) /** this component is syntactical sugar 🍭 */ @@ -133,19 +196,24 @@ type MenuOverlayProps = Partial & const Overlay: React.FC> = ({ children, align = 'start', - side = 'outside-bottom', + side, 'aria-labelledby': ariaLabelledby, ...overlayProps }) => { // we typecast anchorRef as required instead of optional // because we know that we're setting it in context in Menu - const {anchorRef, renderAnchor, anchorId, open, onOpen, onClose} = React.useContext(MenuContext) as MandateProps< - MenuContextProps, - 'anchorRef' - > + const { + anchorRef, + renderAnchor, + anchorId, + open, + onOpen, + onClose, + isSubmenu = false, + } = React.useContext(MenuContext) as MandateProps const containerRef = React.useRef(null) - useMenuKeyboardNavigation(open, onClose, containerRef, anchorRef) + useMenuKeyboardNavigation(open, onClose, containerRef, anchorRef, isSubmenu) // If the menu anchor is an icon button, we need to label the menu by tooltip that also labelled the anchor. const [anchorAriaLabelledby, setAnchorAriaLabelledby] = useState(null) @@ -167,7 +235,7 @@ const Overlay: React.FC> = ({ onOpen={onOpen} onClose={onClose} align={align} - side={side} + side={side ?? (isSubmenu ? 'outside-right' : 'outside-bottom')} overlayProps={overlayProps} focusZoneSettings={{focusOutBehavior: 'wrap'}} > @@ -179,7 +247,7 @@ const Overlay: React.FC> = ({ // If there is a custom aria-labelledby, use that. Otherwise, if exists, use the id that labels the anchor such as tooltip. If none of them exist, use anchor id. listLabelledBy: ariaLabelledby || anchorAriaLabelledby || anchorId, selectionAttribute: 'aria-checked', // Should this be here? - afterSelect: onClose, + afterSelect: () => onClose?.('item-select'), }} > {children} diff --git a/packages/react/src/__tests__/ActionMenu.test.tsx b/packages/react/src/__tests__/ActionMenu.test.tsx index 8ec6fd70624..0c6c9ace83a 100644 --- a/packages/react/src/__tests__/ActionMenu.test.tsx +++ b/packages/react/src/__tests__/ActionMenu.test.tsx @@ -1,4 +1,4 @@ -import {render as HTMLRender, waitFor, act} from '@testing-library/react' +import {render as HTMLRender, waitFor, act, within} from '@testing-library/react' import userEvent from '@testing-library/user-event' import {axe} from 'jest-axe' import React from 'react' @@ -78,6 +78,60 @@ function ExampleWithTooltipV2(actionMenuTrigger: React.ReactElement): JSX.Elemen ) } +function ExampleWithSubmenus(): JSX.Element { + return ( + + + + + Toggle Menu + + + New file + + Copy link + Edit file + + Paste + + + Paste special + + + + Paste plain text + Paste formulas + Paste with formatting + + + Paste from + + + + { + /*noop*/ + }} + > + Current clipboard + + History + Another device + + + + + + + + + + + + + ) +} + describe('ActionMenu', () => { behavesAsComponent({ Component: ActionList, @@ -435,6 +489,7 @@ describe('ActionMenu', () => { expect(button.id).toBe(buttonId) }) + it('should use the tooltip id to name the menu when the anchor is icon button', async () => { const component = HTMLRender( @@ -464,4 +519,117 @@ describe('ActionMenu', () => { expect(toggleButton).toHaveAttribute('aria-labelledby') expect(component.getByRole('menu')).toHaveAttribute('aria-labelledby', toggleButton.getAttribute('aria-labelledby')) }) + + describe('submenus', () => { + it('sets `aria-haspopup` and `aria-expanded` on submenu anchors', async () => { + const component = HTMLRender() + const user = userEvent.setup() + + const baseAnchor = component.getByRole('button', {name: 'Toggle Menu'}) + await user.click(baseAnchor) + + const submenuAnchor = component.getByRole('menuitem', {name: 'Paste special'}) + expect(submenuAnchor).toHaveAttribute('aria-haspopup') + await user.click(submenuAnchor) + expect(submenuAnchor).toHaveAttribute('aria-expanded') + + const subSubmenuAnchor = component.getByRole('menuitem', {name: 'Paste from'}) + expect(subSubmenuAnchor).toHaveAttribute('aria-haspopup') + await user.click(subSubmenuAnchor) + expect(subSubmenuAnchor).toHaveAttribute('aria-expanded') + }) + + it('sets labels on submenus', async () => { + const component = HTMLRender() + const user = userEvent.setup() + + const baseAnchor = component.getByRole('button', {name: 'Toggle Menu'}) + await user.click(baseAnchor) + + const submenuAnchor = component.getByRole('menuitem', {name: 'Paste special'}) + await user.click(submenuAnchor) + const submenu = component.getByRole('menu', {name: 'Paste special'}) + expect(submenu).toBeVisible() + + const subSubmenuAnchor = within(submenu).getByRole('menuitem', {name: 'Paste from'}) + await user.click(subSubmenuAnchor) + const subSubmenu = component.getByRole('menu', {name: 'Paste from'}) + expect(subSubmenu).toBeVisible() + }) + + it('does not open top-level menu on right arrow key press', async () => { + const component = HTMLRender() + const user = userEvent.setup() + + const baseAnchor = component.getByRole('button', {name: 'Toggle Menu'}) + baseAnchor.focus() + + await user.keyboard('{ArrowRight}') + expect(component.queryByRole('menu')).not.toBeInTheDocument() + expect(baseAnchor).not.toHaveAttribute('aria-expanded', 'true') + }) + + it('opens submenus on enter or right arrow key press', async () => { + const component = HTMLRender() + const user = userEvent.setup() + + const baseAnchor = component.getByRole('button', {name: 'Toggle Menu'}) + await user.click(baseAnchor) + + const submenuAnchor = component.getByRole('menuitem', {name: 'Paste special'}) + expect(submenuAnchor).toHaveAttribute('aria-haspopup', 'true') + submenuAnchor.focus() + await user.keyboard('{Enter}') + expect(submenuAnchor).toHaveAttribute('aria-expanded', 'true') + + const subSubmenuAnchor = component.getByRole('menuitem', {name: 'Paste from'}) + subSubmenuAnchor.focus() + await user.keyboard('{ArrowRight}') + expect(subSubmenuAnchor).toHaveAttribute('aria-expanded', 'true') + }) + + it('closes top menu on escape or left arrow key press', async () => { + const component = HTMLRender() + const user = userEvent.setup() + + const baseAnchor = component.getByRole('button', {name: 'Toggle Menu'}) + await user.click(baseAnchor) + + const submenuAnchor = component.getByRole('menuitem', {name: 'Paste special'}) + await user.click(submenuAnchor) + + const subSubmenuAnchor = component.getByRole('menuitem', {name: 'Paste from'}) + await user.click(subSubmenuAnchor) + + expect(subSubmenuAnchor).toHaveAttribute('aria-expanded', 'true') + + await user.keyboard('{Escape}') + expect(subSubmenuAnchor).not.toHaveAttribute('aria-expanded', 'true') + expect(submenuAnchor).toHaveAttribute('aria-expanded', 'true') + + await user.keyboard('{ArrowLeft}') + expect(submenuAnchor).not.toHaveAttribute('aria-expanded', 'true') + + expect(baseAnchor).toHaveAttribute('aria-expanded', 'true') + }) + + it('closes all menus when an item is selected', async () => { + const component = HTMLRender() + const user = userEvent.setup() + + const baseAnchor = component.getByRole('button', {name: 'Toggle Menu'}) + await user.click(baseAnchor) + + const submenuAnchor = component.getByRole('menuitem', {name: 'Paste special'}) + await user.click(submenuAnchor) + + const subSubmenuAnchor = component.getByRole('menuitem', {name: 'Paste from'}) + await user.click(subSubmenuAnchor) + + const subSubmenuItem = component.getByRole('menuitem', {name: 'Current clipboard'}) + await user.click(subSubmenuItem) + + expect(baseAnchor).not.toHaveAttribute('aria-expanded', 'true') + }) + }) }) diff --git a/packages/react/src/hooks/useMenuKeyboardNavigation.ts b/packages/react/src/hooks/useMenuKeyboardNavigation.ts index 8adbb52778a..4000fd578fe 100644 --- a/packages/react/src/hooks/useMenuKeyboardNavigation.ts +++ b/packages/react/src/hooks/useMenuKeyboardNavigation.ts @@ -2,7 +2,7 @@ import React from 'react' import {iterateFocusableElements} from '@primer/behaviors/utils' import {useMenuInitialFocus} from './useMenuInitialFocus' import {useMnemonics} from './useMnemonics' -import type {MenuContextProps} from '../ActionMenu' +import type {MenuCloseHandler} from '../ActionMenu' /** * Keyboard navigation is a mix of 4 hooks @@ -13,14 +13,16 @@ import type {MenuContextProps} from '../ActionMenu' */ export const useMenuKeyboardNavigation = ( open: boolean, - onClose: MenuContextProps['onClose'], + onClose: MenuCloseHandler | undefined, containerRef: React.RefObject, anchorRef: React.RefObject, + isSubmenu: boolean, ) => { useMenuInitialFocus(open, containerRef, anchorRef) useMnemonics(open, containerRef) useCloseMenuOnTab(open, onClose, containerRef, anchorRef) useMoveFocusToMenuItem(open, containerRef, anchorRef) + useCloseSubmenuOnArrow(open, isSubmenu, onClose, containerRef) } /** @@ -29,7 +31,7 @@ export const useMenuKeyboardNavigation = ( */ const useCloseMenuOnTab = ( open: boolean, - onClose: MenuContextProps['onClose'], + onClose: MenuCloseHandler | undefined, containerRef: React.RefObject, anchorRef: React.RefObject, ) => { @@ -50,6 +52,29 @@ const useCloseMenuOnTab = ( }, [open, onClose, containerRef, anchorRef]) } +/** + * Close submenu when left arrow key is pressed + */ +const useCloseSubmenuOnArrow = ( + open: boolean, + isSubmenu: boolean, + onClose: MenuCloseHandler | undefined, + containerRef: React.RefObject, +) => { + React.useEffect(() => { + const container = containerRef.current + + const handler = (event: KeyboardEvent) => { + if (open && isSubmenu && event.key === 'ArrowLeft') onClose?.('arrow-left') + } + + container?.addEventListener('keydown', handler) + return () => { + container?.removeEventListener('keydown', handler) + } + }, [open, onClose, containerRef, isSubmenu]) +} + /** * When Arrow Keys are pressed and the focus is on the anchor, * focus should move to a menu item