From f599c9e001a1feb3cd2a5115a9578772609340d7 Mon Sep 17 00:00:00 2001 From: Adam Thompson <2414030+TheSonOfThomp@users.noreply.github.com> Date: Wed, 5 Jun 2024 11:20:14 -0400 Subject: [PATCH] Menu descendants (#2358) * create Menu.styles * installs descendants in menu * extract useMenuHeight * init descendants * pass onItemFocus from provider * abstract out useUpdatedChildren * creates useHighlightReducer * cleanup reducer * skip disabled elements * implement descendant in submenu * Update yarn.lock * rm focus-visible styles we always want focus * fix menu item list style * Update Avatar props (#2352) * avatar accepts null text * update generated stories * changeset * Update spotty-ghosts-play.md * fix ts errors * rm deprecated hooks * rm debug text * restructure test suite * Create blue-crews-hope.md * Updates stories * adds controlled story * modernizes spec file * Update Menu.stories.tsx * Update SplitButton.spec.tsx * update split button pkg.json * Update yarn.lock * Delete getNewIndex.ts * add // prettier-ignore * mv HighlightReducer Update getUpdatedIndex.ts * Update .gitignore * add turbo to clean (#2361) * pr * Update .gitignore * create Menu.styles * installs descendants in menu * extract useMenuHeight * init descendants * pass onItemFocus from provider * abstract out useUpdatedChildren * creates useHighlightReducer * cleanup reducer * skip disabled elements * implement descendant in submenu * Update yarn.lock * rm focus-visible styles we always want focus * fix menu item list style * fix ts errors * rm deprecated hooks * rm debug text * restructure test suite * Create blue-crews-hope.md * Updates stories * adds controlled story * modernizes spec file * Update Menu.stories.tsx * rm old highlight reducer * rm unused descendant vars * typo --------- Co-authored-by: Shaneeza --- .changeset/blue-crews-hope.md | 5 + .changeset/spotty-dogs-thank.md | 5 + packages/menu/package.json | 4 +- packages/menu/src/Menu.spec.tsx | 425 ++++++++---------- packages/menu/src/Menu.stories.tsx | 224 +++++---- .../Menu/HighlightReducer/HighlightReducer.ts | 40 ++ .../src/Menu/HighlightReducer/highlight.d.ts | 2 + .../menu/src/Menu/HighlightReducer/index.ts | 2 + .../HighlightReducer/utils/getUpdatedIndex.ts | 92 ++++ .../utils/isDescendantsSet.ts | 8 + packages/menu/src/Menu/Menu.styles.ts | 29 ++ packages/menu/src/Menu/Menu.tsx | 357 ++++----------- packages/menu/src/Menu/utils/useMenuHeight.ts | 28 ++ packages/menu/src/MenuContext/MenuContext.tsx | 11 +- .../menu/src/MenuContext/MenuContext.types.ts | 3 +- packages/menu/src/MenuContext/index.ts | 2 +- .../menu/src/MenuItem/MenuItem.stories.tsx | 10 +- packages/menu/src/MenuItem/MenuItem.styles.ts | 4 + packages/menu/src/MenuItem/MenuItem.tsx | 21 +- packages/menu/src/SubMenu/SubMenu.tsx | 33 +- packages/menu/src/SubMenu/SubMenu.types.ts | 3 +- .../menu/src/SubMenu/useControlledState.ts | 30 ++ packages/menu/src/styles.ts | 14 +- packages/menu/tsconfig.json | 3 + .../src/SplitButton/SplitButton.spec.tsx | 194 ++++---- 25 files changed, 850 insertions(+), 699 deletions(-) create mode 100644 .changeset/blue-crews-hope.md create mode 100644 .changeset/spotty-dogs-thank.md create mode 100644 packages/menu/src/Menu/HighlightReducer/HighlightReducer.ts create mode 100644 packages/menu/src/Menu/HighlightReducer/highlight.d.ts create mode 100644 packages/menu/src/Menu/HighlightReducer/index.ts create mode 100644 packages/menu/src/Menu/HighlightReducer/utils/getUpdatedIndex.ts create mode 100644 packages/menu/src/Menu/HighlightReducer/utils/isDescendantsSet.ts create mode 100644 packages/menu/src/Menu/Menu.styles.ts create mode 100644 packages/menu/src/Menu/utils/useMenuHeight.ts create mode 100644 packages/menu/src/SubMenu/useControlledState.ts diff --git a/.changeset/blue-crews-hope.md b/.changeset/blue-crews-hope.md new file mode 100644 index 0000000000..e301d0e63f --- /dev/null +++ b/.changeset/blue-crews-hope.md @@ -0,0 +1,5 @@ +--- +'@leafygreen-ui/menu': minor +--- + +Internal refactor of Menu to leverage `@leafygreen-ui/descendants`. This improvement will enable faster feature development and bug fixes in the future (coming soon!) diff --git a/.changeset/spotty-dogs-thank.md b/.changeset/spotty-dogs-thank.md new file mode 100644 index 0000000000..578be97bc5 --- /dev/null +++ b/.changeset/spotty-dogs-thank.md @@ -0,0 +1,5 @@ +--- +'@leafygreen-ui/split-button': patch +--- + +Updates Menu. Resolves failing tests due to Menu update diff --git a/packages/menu/package.json b/packages/menu/package.json index f48b4d5b68..fb99945e99 100644 --- a/packages/menu/package.json +++ b/packages/menu/package.json @@ -22,6 +22,7 @@ "access": "public" }, "dependencies": { + "@leafygreen-ui/descendants": "^0.1.1", "@leafygreen-ui/emotion": "^4.0.8", "@leafygreen-ui/hooks": "^8.1.3", "@leafygreen-ui/icon": "^12.5.4", @@ -48,6 +49,7 @@ }, "devDependencies": { "@leafygreen-ui/button": "^21.2.0", - "@lg-tools/storybook-utils": "^0.1.1" + "@lg-tools/storybook-utils": "^0.1.1", + "@storybook/react": "^7.6.17" } } diff --git a/packages/menu/src/Menu.spec.tsx b/packages/menu/src/Menu.spec.tsx index 512aa07332..231d15c2e4 100644 --- a/packages/menu/src/Menu.spec.tsx +++ b/packages/menu/src/Menu.spec.tsx @@ -1,10 +1,11 @@ import React, { createRef } from 'react'; import { act, - getAllByRole as globalGetAllByRole, + fireEvent, render, waitFor, waitForElementToBeRemoved, + within, } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; @@ -12,33 +13,68 @@ import { MenuProps } from './Menu'; import { Menu, MenuItem, MenuSeparator } from '.'; const menuTestId = 'menu-test-id'; -const trigger = ; +const menuTriggerTestId = 'menu-trigger'; +const defaultTrigger = ; function waitForTimeout(timeout = 500) { return new Promise(res => setTimeout(res, timeout)); } -function renderMenu(props: Omit = {}) { - const utils = render( +/** Renders a Menu with the given props */ +function renderMenu({ + trigger = defaultTrigger, + ...rest +}: Omit = {}) { + const renderResult = render( <>
- + Item A Item B + Item C , ); - const backdrop = utils.getByTestId('backdrop'); - return { ...utils, backdrop }; + const backdropEl = renderResult.getByTestId('backdrop'); + const triggerEl = renderResult.getByTestId(menuTriggerTestId); + + /** + * Since menu elements won't exist until component is interacted with, + * call this after opening the menu. + * @returns Object of menu elements + */ + async function findMenuElements(): Promise<{ + menuEl: HTMLElement | null; + menuItemElements: Array; + }> { + const menuEl = await renderResult.findByTestId(menuTestId); + const menuItemElements = await within(menuEl).findAllByRole('menuitem'); + + return { + menuEl, + menuItemElements, + }; + } + + /** Opens the menu, and manually fires transition events */ + async function openMenu() { + userEvent.click(triggerEl); + const menuElements = await findMenuElements(); + fireEvent.transitionEnd(menuElements.menuEl as Element); // JSDOM does not automatically fire these events + return menuElements; + } + + return { ...renderResult, backdropEl, triggerEl, findMenuElements, openMenu }; } describe('packages/menu', () => { - test.todo('trigger renders as a function'); - test.todo('trigger renders as a JSX element'); - test.todo('menu appears when trigger is a function'); - test.todo('menu appears when trigger is a JSX element'); + describe('Rendering', () => { + test.todo('trigger renders as a function'); + test.todo('trigger renders as a JSX element'); + test.todo('menu appears when trigger is a function'); + test.todo('menu appears when trigger is a JSX element'); test('menu appears on DOM when the "open" prop is set', () => { const { getByTestId } = renderMenu({ open: true }); @@ -78,207 +114,111 @@ describe('packages/menu', () => { test('and `initialOpen` is set to true', () => { const { getByText } = renderMenu({ initialOpen: true, - trigger, }); - const menuItem = getByText('Item B'); - expect(menuItem).toBeInTheDocument(); }); - test('and "setOpen" is provided, but "open" prop is not set', async () => { - const { getByTestId, getByText } = renderMenu({ - setOpen: uncontrolledSetOpen, - trigger, + describe('controlled `open`', () => { + const setOpen = jest.fn(); + test('menu renders when `open` prop is set', () => { + const { getByTestId } = renderMenu({ open: true, setOpen }); + const menu = getByTestId(menuTestId); + expect(menu).toBeInTheDocument(); }); - const button = getByTestId('menu-trigger'); - userEvent.click(button); - - const menuItem = getByText('Item B'); - - expect(menuItem).toBeInTheDocument(); - - userEvent.click(button); - - await waitForElementToBeRemoved(menuItem); - expect(menuItem).not.toBeInTheDocument(); - }); - }); - - test('clicking a menuitem does not close the menu', async () => { - const { getByTestId } = renderMenu({ - trigger, - }); - - const button = getByTestId('menu-trigger'); - - userEvent.click(button); - const menu = getByTestId(menuTestId); - - expect(menu).toBeInTheDocument(); - - const menuItem = getByTestId('menu-item-a'); - userEvent.click(menuItem); - - await act(async () => await waitForTimeout()); - expect(menu).toBeInTheDocument(); - }); - - test('pressing enter on a menuitem does not close the menu', async () => { - const { getByTestId } = renderMenu({ - trigger, - }); - - const button = getByTestId('menu-trigger'); - - userEvent.click(button); - const menu = getByTestId(menuTestId); - - expect(menu).toBeInTheDocument(); - - const menuItem = getByTestId('menu-item-a'); - - menuItem.focus(); - userEvent.keyboard('[Enter]'); + test('renders all children', () => { + const { getByText } = renderMenu({ open: true, setOpen }); + const menuItem = getByText('Item A'); + expect(menuItem).toBeInTheDocument(); + }); - await act(async () => await waitForTimeout()); - expect(menu).toBeInTheDocument(); - }); + test('first item is focused', async () => { + const { findMenuElements } = renderMenu({ open: true, setOpen }); + const { menuEl, menuItemElements } = await findMenuElements(); - test('pressing space on a menuitem does not close the menu', async () => { - const { getByTestId } = renderMenu({ - trigger, - }); + // JSDOM does not automatically fire these events + fireEvent.transitionEnd(menuEl as Element); - const button = getByTestId('menu-trigger'); + await waitFor(() => { + const firstItem = menuItemElements[0]; + expect(firstItem).toHaveFocus(); + }); + }); - userEvent.click(button); - const menu = getByTestId(menuTestId); + test('uncontrolled if `open` prop is not set, with `setOpen` callback', async () => { + const { getByTestId, getByText } = renderMenu({ + open: undefined, + setOpen, + trigger: defaultTrigger, + }); - expect(menu).toBeInTheDocument(); + const button = getByTestId('menu-trigger'); + userEvent.click(button); - const menuItem = getByTestId('menu-item-a'); + const menuItem = getByText('Item B'); - menuItem.focus(); - userEvent.keyboard('[Space]'); + expect(menuItem).toBeInTheDocument(); - await act(async () => await waitForTimeout()); - expect(menu).toBeInTheDocument(); - }); + userEvent.click(button); - test('clicking outside the menu closes the menu', async () => { - const { getByTestId, backdrop } = renderMenu({ - trigger, + await waitForElementToBeRemoved(menuItem); + expect(menuItem).not.toBeInTheDocument(); + }); }); - - const button = getByTestId('menu-trigger'); - userEvent.click(button); - const menu = getByTestId(menuTestId); - - expect(menu).toBeInTheDocument(); - - userEvent.click(backdrop); - - await waitForElementToBeRemoved(menu); - expect(menu).not.toBeInTheDocument(); }); - describe('when controlled', () => { - const ControlledExample = () => { - const [open, setOpen] = React.useState(true); - - return ( - <> -
- - Text - - - ); - }; - - test('clicking a menuitem does not close the menu', async () => { - const { getByTestId } = render(); - - const menu = getByTestId('controlled-menu'); - const menuItem = getByTestId('controlled-menu-item'); - - expect(menu).toBeInTheDocument(); - - userEvent.click(menuItem); - - await act(async () => await waitForTimeout()); - expect(menu).toBeInTheDocument(); + describe('Mouse interaction', () => { + test('Clicking trigger opens menu', async () => { + const { triggerEl, findMenuElements } = renderMenu({}); + userEvent.click(triggerEl); + const { menuEl } = await findMenuElements(); + await waitFor(() => { + expect(menuEl).toBeInTheDocument(); + }); }); - test('pressing enter on a menuitem does not close the menu', async () => { - const { getByTestId } = render(); - - const menu = getByTestId('controlled-menu'); - const menuItem = getByTestId('controlled-menu-item'); - - expect(menu).toBeInTheDocument(); - - menuItem.focus(); - userEvent.keyboard('[Enter]'); - - await act(async () => await waitForTimeout()); - expect(menu).toBeInTheDocument(); + test('First item is focused when menu is opened', async () => { + const { triggerEl, findMenuElements } = renderMenu({}); + userEvent.click(triggerEl); + const { menuEl, menuItemElements } = await findMenuElements(); + await waitFor(() => { + // JSDOM does not automatically fire these events + fireEvent.transitionEnd(menuEl as Element); + expect(menuItemElements[0]).toHaveFocus(); + }); }); - test('pressing space on a menuitem does not close the menu', async () => { - const { getByTestId } = render(); + test('Clicking a menuitem does not close the menu', async () => { + const { openMenu } = renderMenu({}); + const { menuEl, menuItemElements } = await openMenu(); - const menu = getByTestId('controlled-menu'); - const menuItem = getByTestId('controlled-menu-item'); - - expect(menu).toBeInTheDocument(); - - menuItem.focus(); - userEvent.keyboard('[Space]'); + expect(menuEl).toBeInTheDocument(); + const firstItem = menuItemElements[0]; + userEvent.click(firstItem!); await act(async () => await waitForTimeout()); - expect(menu).toBeInTheDocument(); + expect(menuEl).toBeInTheDocument(); }); - test('clicking outside the menu closes the menu', async () => { - const { getByTestId } = render(); + test('Clicking outside the menu closes the menu', async () => { + const { openMenu, backdropEl } = renderMenu({}); + const { menuEl } = await openMenu(); - const menu = getByTestId('controlled-menu'); + expect(menuEl).toBeInTheDocument(); - await waitFor(() => { - expect(menu).toBeInTheDocument(); - }); - - const backdrop = getByTestId('backdrop'); - - userEvent.click(backdrop); - - await waitForElementToBeRemoved(menu); - expect(menu).not.toBeInTheDocument(); - }); - }); - - describe('Mouse interaction', () => { - test('Clicking trigger opens menu', () => { - const { getByRole, getByTestId } = renderMenu({ - trigger, - }); - const button = getByRole('button'); - - userEvent.click(button); - const menu = getByTestId(menuTestId); + userEvent.click(backdropEl); + await waitForElementToBeRemoved(menuEl); - expect(menu).toBeInTheDocument(); + expect(menuEl).not.toBeInTheDocument(); }); - test('Click handlers on parent elements fire', async () => { + test('Click handlers on parent elements fire (propagation is not stopped)', async () => { const parentHandler = jest.fn(); const { getByTestId } = render( // eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
- + Item A Item B @@ -295,10 +235,10 @@ describe('packages/menu', () => { }); }); - type Keys = 'esc' | 'tab'; - const closeKeys: Array> = [['esc'], ['tab']]; - describe('Keyboard Interaction', () => { + type Keys = 'esc' | 'tab'; + const closeKeys: Array> = [['esc'], ['tab']]; + const userEventInteraction = (menu: HTMLElement, key: Keys) => { if (key === 'tab') { userEvent.tab(); @@ -309,84 +249,107 @@ describe('packages/menu', () => { describe.each(closeKeys)('%s key', key => { test('Closes menu', async () => { - const { getByRole, getByTestId } = renderMenu({ - trigger, - }); - const button = getByRole('button'); - userEvent.click(button); - const menu = getByTestId(menuTestId); + const { openMenu } = renderMenu({}); + const { menuEl } = await openMenu(); - userEventInteraction(menu, key); - await waitForElementToBeRemoved(menu); - expect(menu).not.toBeInTheDocument(); + userEventInteraction(menuEl!, key); + await waitForElementToBeRemoved(menuEl); + expect(menuEl).not.toBeInTheDocument(); }); + test('Returns focus to trigger {usePortal: true}', async () => { - const { getByRole, getByTestId } = renderMenu({ - trigger, + const { openMenu, triggerEl } = renderMenu({ usePortal: true, }); - const button = getByRole('button'); - userEvent.click(button); - const menu = getByTestId(menuTestId); + const { menuEl } = await openMenu(); - userEventInteraction(menu, key); - await waitForElementToBeRemoved(menu); - expect(button).toHaveFocus(); + userEventInteraction(menuEl!, key); + await waitForElementToBeRemoved(menuEl); + expect(triggerEl).toHaveFocus(); }); test('Returns focus to trigger {usePortal: false}', async () => { - const { getByRole, getByTestId } = renderMenu({ - trigger, + const { openMenu, triggerEl } = renderMenu({ usePortal: false, }); - const button = getByRole('button'); - userEvent.click(button); - const menu = getByTestId(menuTestId); + const { menuEl } = await openMenu(); - userEventInteraction(menu, key); - await waitForElementToBeRemoved(menu); - expect(button).toHaveFocus(); + userEventInteraction(menuEl!, key); + await waitForElementToBeRemoved(menuEl); + expect(triggerEl).toHaveFocus(); }); }); describe('Arrow keys', () => { - let menu: HTMLElement; - let options: Array; - - beforeEach(() => { - const { getByTestId } = renderMenu({ trigger }); - const triggerButton = getByTestId('menu-trigger'); - - userEvent.click(triggerButton); - menu = getByTestId(menuTestId); - options = globalGetAllByRole(menu, 'menuitem'); - }); - describe('Down arrow', () => { - test('highlights the next option in the menu', () => { - userEvent.type(menu, '{arrowdown}'); - expect(options[1]).toHaveFocus(); + test('highlights the next option in the menu', async () => { + const { openMenu } = renderMenu({}); + const { menuEl, menuItemElements } = await openMenu(); + userEvent.type(menuEl!, '{arrowdown}'); + expect(menuItemElements[1]).toHaveFocus(); }); - test('cycles highlight to the top', () => { - // programmatically set focus on last option - options[options.length - 1].focus(); - userEvent.type(menu, '{arrowdown}'); - expect(options[0]).toHaveFocus(); + test('cycles highlight to the top', async () => { + const { openMenu } = renderMenu({}); + const { menuEl, menuItemElements } = await openMenu(); + + for (let i = 0; i < menuItemElements.length; i++) { + userEvent.type(menuEl!, '{arrowdown}'); + } + + expect(menuItemElements[0]).toHaveFocus(); }); }); describe('Up arrow', () => { - test('highlights the previous option in the menu', () => { - // programmatically set focus on second option - options[1].focus(); - userEvent.type(menu, '{arrowup}'); - expect(options[0]).toHaveFocus(); + test('highlights the previous option in the menu', async () => { + const { openMenu } = renderMenu({}); + const { menuEl, menuItemElements } = await openMenu(); + + userEvent.type(menuEl!, '{arrowdown}'); + userEvent.type(menuEl!, '{arrowup}'); + expect(menuItemElements[0]).toHaveFocus(); }); - test('cycles highlight to the bottom', () => { - userEvent.type(menu, '{arrowup}'); - expect(options[options.length - 1]).toHaveFocus(); + test('cycles highlight to the bottom', async () => { + const { openMenu } = renderMenu({}); + const { menuEl, menuItemElements } = await openMenu(); + + const lastOption = menuItemElements[menuItemElements.length - 1]; + userEvent.type(menuEl!, '{arrowup}'); + expect(lastOption).toHaveFocus(); }); }); }); + + test('Enter key on a menuitem does not close the menu', async () => { + const { openMenu } = renderMenu({}); + const { menuEl, menuItemElements } = await openMenu(); + + expect(menuEl).toBeInTheDocument(); + + const firstItem = menuItemElements[0]; + + expect(firstItem).toHaveFocus(); + + userEvent.keyboard('[Enter]'); + + await act(async () => await waitForTimeout()); + expect(menuEl).toBeInTheDocument(); + }); + + test('Space key on a menuitem does not close the menu', async () => { + const { openMenu } = renderMenu({}); + const { menuEl, menuItemElements } = await openMenu(); + + expect(menuEl).toBeInTheDocument(); + + const firstItem = menuItemElements[0]; + + expect(firstItem).toHaveFocus(); + + userEvent.keyboard('[Space]'); + + await act(async () => await waitForTimeout()); + expect(menuEl).toBeInTheDocument(); + }); }); }); diff --git a/packages/menu/src/Menu.stories.tsx b/packages/menu/src/Menu.stories.tsx index 88958cf79d..6e7fad759a 100644 --- a/packages/menu/src/Menu.stories.tsx +++ b/packages/menu/src/Menu.stories.tsx @@ -1,12 +1,12 @@ /* eslint-disable react-hooks/rules-of-hooks */ /* eslint-disable react/display-name */ -import React, { useEffect, useState } from 'react'; +import React, { useState } from 'react'; import { storybookArgTypes, storybookExcludedControlParams, StoryMetaType, } from '@lg-tools/storybook-utils'; -import { StoryFn } from '@storybook/react'; +import { StoryObj } from '@storybook/react'; import Button from '@leafygreen-ui/button'; import { css } from '@leafygreen-ui/emotion'; @@ -15,16 +15,41 @@ import CloudIcon from '@leafygreen-ui/icon/dist/Cloud'; import LeafyGreenProvider from '@leafygreen-ui/leafygreen-provider'; import { Align, Justify } from '@leafygreen-ui/popover'; import { TestUtils } from '@leafygreen-ui/popover'; -import { transitionDuration } from '@leafygreen-ui/tokens'; const { getAlign, getJustify } = TestUtils; import { Size } from './types'; -import { Menu, MenuItem, MenuProps, MenuSeparator, SubMenu } from '.'; +import { + Menu, + MenuItem, + MenuItemProps, + MenuProps, + MenuSeparator, + SubMenu, +} from '.'; + +const getDecoratorStyles = (args: Partial) => { + return css` + width: 256px; + height: 250px; + display: flex; + align-items: ${['left', 'right'].includes(args.align!) + ? 'end' + : getAlign(args.align!, args.justify!)}; + justify-content: ${getJustify(args.align!, args.justify!)}; + `; +}; -const meta: StoryMetaType = { +export default { title: 'Components/Menu', component: Menu, + decorators: [ + (StoryFn, _ctx) => ( + + + + ), + ], parameters: { default: 'LiveExample', controls: { @@ -40,66 +65,6 @@ const meta: StoryMetaType = { chromatic: { disableSnapshot: true, }, - generate: { - combineArgs: { - darkMode: [false, true], - // Popover props - align: Object.values(Align), - justify: Object.values(Justify), - }, - args: { - open: true, - maxHeight: 200, - children: ( - <> - Lorem - } - active={true} - > - Apple - Banana - Carrot - Dragonfruit - Eggplant - Fig - - - ), - }, - excludeCombinations: [ - { - align: [Align.CenterHorizontal, Align.CenterVertical], - }, - { - justify: Justify.Fit, - align: [Align.Left, Align.Right], - }, - ], - decorator: (Instance, ctx) => ( -
- - trigger - - } - /> -
- ), - }, }, args: { open: true, @@ -122,17 +87,16 @@ const meta: StoryMetaType = { }, darkMode: storybookArgTypes.darkMode, }, -}; -export default meta; +} satisfies StoryMetaType; -export const LiveExample: StoryFn = ({ - open: _, - size, - darkMode, - ...args -}: MenuProps & { size: Size }) => { - return ( - +export const LiveExample = { + render: ({ + open: _, + size, + darkMode, + ...args + }: MenuProps & { size: MenuItemProps['size'] }) => { + return ( = ({ Inceptos Risus - - ); -}; -LiveExample.parameters = { - chromatic: { - disableSnapshot: true, + ); }, -}; + parameters: { + chromatic: { + disableSnapshot: true, + }, + }, +} satisfies StoryObj; -export const InitialOpen = () => { - return ( - +export const InitialOpen = { + render: () => { + return ( }>Menu} @@ -213,13 +177,87 @@ export const InitialOpen = () => { Ipsum Adipiscing - - ); -}; -InitialOpen.parameters = { - chromatic: { - disableSnapshot: true, + ); }, -}; + parameters: { + chromatic: { + disableSnapshot: true, + }, + }, +} satisfies StoryObj; + +export const Controlled = { + render: () => { + const [open, setOpen] = useState(true); -export const Generated = () => <>; + return ( + }>Menu} + > + Lorem + Ipsum + Adipiscing + + ); + }, + parameters: { + chromatic: { + disableSnapshot: true, + }, + }, +} satisfies StoryObj; + +export const Generated = { + render: () => <>, + args: { + open: true, + maxHeight: 200, + children: ( + <> + Lorem + } + active={true} + > + Apple + Banana + Carrot + Dragonfruit + Eggplant + Fig + + + ), + trigger: , + }, + parameters: { + generate: { + combineArgs: { + darkMode: [false, true], + align: Object.values(Align), + justify: Object.values(Justify), + }, + + excludeCombinations: [ + { + align: [Align.CenterHorizontal, Align.CenterVertical], + }, + { + justify: Justify.Fit, + align: [Align.Left, Align.Right], + }, + ], + decorator: (Instance, ctx) => ( + +
+ +
+ + ), + }, + }, +} satisfies StoryObj; diff --git a/packages/menu/src/Menu/HighlightReducer/HighlightReducer.ts b/packages/menu/src/Menu/HighlightReducer/HighlightReducer.ts new file mode 100644 index 0000000000..2c6b2cabe5 --- /dev/null +++ b/packages/menu/src/Menu/HighlightReducer/HighlightReducer.ts @@ -0,0 +1,40 @@ +import { type Dispatch, type Reducer, useReducer } from 'react'; + +import { DescendantsList } from '@leafygreen-ui/descendants'; + +import { getUpdatedIndex } from './utils/getUpdatedIndex'; +import { isDescendantsSet } from './utils/isDescendantsSet'; +import type { Direction, Index } from './highlight'; + +const getInitialIndex = (descendants: DescendantsList) => + isDescendantsSet(descendants) ? 0 : undefined; + +/** + * Custom hook that handles setting the highlighted descendant index, + * and fires any `onChange` side effects + */ +export const useHighlightReducer = ( + descendants: DescendantsList, + onChange?: (newIndex: Index) => void, +): [Index, Dispatch] => { + // Initializes a new reducer function + const highlightReducerFunction: Reducer = ( + _index, + direction, + ) => getUpdatedIndex(direction, _index, descendants); + const [index, dispatch] = useReducer>( + highlightReducerFunction, + getInitialIndex(descendants), + ); + + /** + * Custom dispatch that fires any side-effects when the index changes + */ + const updateIndex = (direction: Direction) => { + const updatedIndex = highlightReducerFunction(index, direction); + onChange?.(updatedIndex); + dispatch(direction); + }; + + return [index, updateIndex]; +}; diff --git a/packages/menu/src/Menu/HighlightReducer/highlight.d.ts b/packages/menu/src/Menu/HighlightReducer/highlight.d.ts new file mode 100644 index 0000000000..564826a8d8 --- /dev/null +++ b/packages/menu/src/Menu/HighlightReducer/highlight.d.ts @@ -0,0 +1,2 @@ +export type Index = number | undefined; +export type Direction = 'next' | 'prev' | 'first' | 'last'; diff --git a/packages/menu/src/Menu/HighlightReducer/index.ts b/packages/menu/src/Menu/HighlightReducer/index.ts new file mode 100644 index 0000000000..98ca5a6ca3 --- /dev/null +++ b/packages/menu/src/Menu/HighlightReducer/index.ts @@ -0,0 +1,2 @@ +export type { Direction, Index } from './highlight'; +export { useHighlightReducer } from './HighlightReducer'; diff --git a/packages/menu/src/Menu/HighlightReducer/utils/getUpdatedIndex.ts b/packages/menu/src/Menu/HighlightReducer/utils/getUpdatedIndex.ts new file mode 100644 index 0000000000..23a0ca3b5d --- /dev/null +++ b/packages/menu/src/Menu/HighlightReducer/utils/getUpdatedIndex.ts @@ -0,0 +1,92 @@ +import { DescendantsList } from '@leafygreen-ui/descendants'; +import { isDefined } from '@leafygreen-ui/lib'; + +import type { Direction, Index } from '../highlight'; + +import { isDescendantsSet } from './isDescendantsSet'; + +/** + * Computes the next index given a direction + */ +// prettier-ignore +function getNextIndex(direction: Direction, currentIndex: number, totalItems: number): number; +// prettier-ignore +function getNextIndex(direction: Direction, currentIndex: undefined, totalItems: number): undefined; +// prettier-ignore +function getNextIndex(direction: Direction, currentIndex: Index, totalItems: number): Index { + if (!isDefined(currentIndex)) return currentIndex; + + switch (direction) { + case 'next': + return (currentIndex + 1) % totalItems; + case 'prev': + return (currentIndex - 1 + totalItems) % totalItems; + case 'last': + return totalItems - 1; + case 'first': + default: + return 0; + } +} + +/** + * Finds the index of the subsequent `enabled` descendant element + */ +function getNextEnabledIndex( + direction: Direction, + currentIndex: number, + descendants: DescendantsList, +): Index { + // If all descendants are disabled, then we skip this step + if (descendants.every(d => d.props.disabled)) { + return undefined; + } + + let updatedIndex = getNextIndex(direction, currentIndex, descendants.length); + let item = descendants[updatedIndex]; + + // If the subsequent item is disabled, + // keep searching in that direction for an enabled one + while (item.props.disabled) { + // If the first/last item is disabled + // start the search in the forward/backward direction + const nextDirection: Direction = (() => { + switch (direction) { + case 'first': + return 'next'; + case 'last': + return 'prev'; + default: + return direction; + } + })(); + updatedIndex = getNextIndex( + nextDirection, + updatedIndex, + descendants.length, + ); + item = descendants[updatedIndex]; + } + + return updatedIndex; +} + +export const getUpdatedIndex = ( + direction: Direction, + currentIndex: Index, + descendants: DescendantsList, +): Index => { + // If descendants is not set + // then we don't mutate the index + if (!isDescendantsSet(descendants)) { + return currentIndex; + } + + const updatedIndex = getNextEnabledIndex( + direction, + currentIndex ?? 0, + descendants, + ); + + return updatedIndex; +}; diff --git a/packages/menu/src/Menu/HighlightReducer/utils/isDescendantsSet.ts b/packages/menu/src/Menu/HighlightReducer/utils/isDescendantsSet.ts new file mode 100644 index 0000000000..773fd6b3d8 --- /dev/null +++ b/packages/menu/src/Menu/HighlightReducer/utils/isDescendantsSet.ts @@ -0,0 +1,8 @@ +import { DescendantsList } from '@leafygreen-ui/descendants'; +import { isDefined } from '@leafygreen-ui/lib'; + +export const isDescendantsSet = ( + descendants?: DescendantsList, +): boolean => { + return isDefined(descendants) && descendants.length > 0; +}; diff --git a/packages/menu/src/Menu/Menu.styles.ts b/packages/menu/src/Menu/Menu.styles.ts new file mode 100644 index 0000000000..6512a85635 --- /dev/null +++ b/packages/menu/src/Menu/Menu.styles.ts @@ -0,0 +1,29 @@ +import { css } from '@leafygreen-ui/emotion'; +import { Theme } from '@leafygreen-ui/lib'; +import { palette } from '@leafygreen-ui/palette'; + +export const rootMenuStyle = css` + width: 210px; + border-radius: 12px; + overflow: auto; + padding: 14px 0; +`; + +export const rootMenuThemeStyles: Record = { + [Theme.Light]: css` + background-color: ${palette.black}; + border: 1px solid ${palette.black}; + `, + [Theme.Dark]: css` + background-color: ${palette.gray.dark3}; + border: 1px solid ${palette.gray.dark2}; + `, +}; + +export const scrollContainerStyle = css` + overflow: auto; + margin-block-start: 0px; + margin-block-end: 0px; + padding-inline-start: 0px; + padding: 0px; +`; diff --git a/packages/menu/src/Menu/Menu.tsx b/packages/menu/src/Menu/Menu.tsx index 1f30060bd4..676d73e971 100644 --- a/packages/menu/src/Menu/Menu.tsx +++ b/packages/menu/src/Menu/Menu.tsx @@ -1,57 +1,29 @@ -import React, { - useCallback, - useEffect, - useMemo, - useRef, - useState, -} from 'react'; -import isUndefined from 'lodash/isUndefined'; +import React, { useCallback, useRef, useState } from 'react'; import PropTypes from 'prop-types'; -import { css, cx } from '@leafygreen-ui/emotion'; import { - useAvailableSpace, - useBackdropClick, - useEventListener, - useForceRerender, -} from '@leafygreen-ui/hooks'; + DescendantsProvider, + useInitDescendants, +} from '@leafygreen-ui/descendants'; +import { css, cx } from '@leafygreen-ui/emotion'; +import { useBackdropClick, useEventListener } from '@leafygreen-ui/hooks'; import { useDarkMode } from '@leafygreen-ui/leafygreen-provider'; -import { isComponentType, keyMap, Theme } from '@leafygreen-ui/lib'; -import { palette } from '@leafygreen-ui/palette'; +import { isDefined, keyMap } from '@leafygreen-ui/lib'; import Popover, { Align, Justify } from '@leafygreen-ui/popover'; -import { MenuContext } from '../MenuContext/MenuContext'; -import MenuSeparator from '../MenuSeparator/MenuSeparator'; -import { type SubMenuProps } from '../SubMenu/'; - -import { MenuProps, type SubMenuType } from './Menu.types'; - -const rootMenuStyle = css` - width: 210px; - border-radius: 12px; - overflow: auto; - padding: 14px 0; -`; - -const rootMenuThemeStyles: Record = { - [Theme.Light]: css` - background-color: ${palette.black}; - border: 1px solid ${palette.black}; - `, - [Theme.Dark]: css` - background-color: ${palette.gray.dark3}; - border: 1px solid ${palette.gray.dark2}; - `, -}; +import { + MenuContext, + MenuDescendantsContext, +} from '../MenuContext/MenuContext'; -const scrollContainerStyle = css` - overflow: auto; - list-style: none; - margin-block-start: 0px; - margin-block-end: 0px; - padding-inline-start: 0px; - padding: 0px; -`; +import { useMenuHeight } from './utils/useMenuHeight'; +import { useHighlightReducer } from './HighlightReducer'; +import { + rootMenuStyle, + rootMenuThemeStyles, + scrollContainerStyle, +} from './Menu.styles'; +import { MenuProps } from './Menu.types'; /** * @@ -99,13 +71,12 @@ export const Menu = React.forwardRef(function Menu( ) { const { theme, darkMode } = useDarkMode(darkModeProp); - const hasSetInitialFocus = useRef(false); - const hasSetInitialOpen = useRef(false); + const popoverRef = useRef(null); + const triggerRef = useRef(null); - const [, setClosed] = useState(false); - const currentSubMenuRef = useRef(null); const [uncontrolledOpen, uncontrolledSetOpen] = useState(initialOpen); - const popoverRef = useRef(null); + + const { descendants, dispatch } = useInitDescendants(); const setOpen = (typeof controlledOpen === 'boolean' && controlledSetOpen) || @@ -117,206 +88,60 @@ export const Menu = React.forwardRef(function Menu( } }, [setOpen, shouldClose]); - // Used to trigger a state update when the current subMenu changes since the current subMenu is stored in a ref to avoid extra rerenders on initial load. - const updateCurrentSubMenu = useForceRerender(); - - const triggerRef = useRef(null); - // This hook causes a second re-render on initial load. `useAvailableSpace` uses `useViewportSize` internally, which has internal state that causes re-renders. - const availableSpace = useAvailableSpace(refEl || triggerRef, spacing); - const memoizedAvailableSpace = useMemo( - () => availableSpace, - [availableSpace], - ); - const maxMenuHeightValue = !isUndefined(memoizedAvailableSpace) - ? `${Math.min(memoizedAvailableSpace, maxHeight)}px` - : 'unset'; - - const { updatedChildren, refs } = React.useMemo(() => { - if ( - children == null || - ['boolean', 'number', 'string'].includes(typeof children) - ) { - return { updatedChildren: undefined, refs: [] }; - } - - const titleArr: Array = []; - const refs: Array = []; - - function updateChildren(children: React.ReactNode): React.ReactNode { - return React.Children.map(children, child => { - if (!React.isValidElement(child) || child.props?.disabled) { - return child; - } - - const { props } = child; - - let currentChildRef: HTMLElement | null = null; - - const setRef = (ref: HTMLElement) => { - if (ref == null) { - return; - } - - refs.push(ref); - currentChildRef = ref; - - if (open && hasSetInitialFocus.current === false) { - setFocus(refs[0]); - hasSetInitialFocus.current = true; - } - }; - - const title = props?.title ?? false; - - const onFocus = ({ target }: React.FocusEvent) => { - focusedRef.current = target; - }; - - if (isComponentType(child, 'SubMenu') && title) { - if (titleArr.includes(title)) { - throw new Error('SubMenu titles must be unique'); - } - - titleArr.push(title); - - const shouldOpenActiveSubMenu = - !currentSubMenuRef.current && - props.active && - !hasSetInitialOpen.current; + const maxMenuHeightValue = useMenuHeight({ + refEl: refEl || triggerRef, + spacing, + maxHeight, + }); - // This opens the active submenu on inital load - if (shouldOpenActiveSubMenu) { - // Using a ref here prevents an extra rerender on initial load. - currentSubMenuRef.current = child; - hasSetInitialOpen.current = true; - } - - const isCurrentSubMenu = - (currentSubMenuRef.current?.props as SubMenuProps)?.title === title; - - return React.cloneElement(child, { - ref: setRef, - open: isCurrentSubMenu, - setOpen: (state: boolean) => { - if (currentChildRef) { - focusedRef.current = currentChildRef; - } - - currentSubMenuRef.current = state ? child : null; - hasSetInitialOpen.current = true; - // Force update since the updated currentSubMenu is set in a ref. - updateCurrentSubMenu(); - }, - onKeyDown: (e: React.KeyboardEvent) => { - if (e.key === keyMap.ArrowLeft && isCurrentSubMenu) { - currentSubMenuRef.current = null; - hasSetInitialOpen.current = true; - updateCurrentSubMenu(); - } - - if (e.key === keyMap.ArrowRight) { - currentSubMenuRef.current = child; - hasSetInitialOpen.current = true; - updateCurrentSubMenu(); - } - }, - onFocus, - children: updateChildren(props.children), - onExited: () => { - setClosed(curr => !curr); - }, - }); - } - - if (isComponentType(child, 'MenuItem')) { - return React.cloneElement(child, { - ref: setRef, - onFocus, - }); - } - - if (isComponentType(child, 'FocusableMenuItem')) { - return React.cloneElement(child, { - ref: setRef, - onFocus, - }); - } - - if (isComponentType(child, 'MenuSeparator')) { - return ; - } - - if (props?.children) { - const { children, ...rest } = props; - return React.cloneElement(child, { - children: updateChildren(props.children), - ...rest, - }); - } - - return child; - }); - } - - return { updatedChildren: updateChildren(children), refs }; - }, [children, open, updateCurrentSubMenu]); - - const focusedRef = useRef(refs[0] || null); + useBackdropClick(handleClose, [popoverRef, triggerRef], open); - const setFocus = (el: HTMLElement | null) => { - if (el == null) { - return; - } + // Tracks the currently highlighted (focused) item index + // Fires `.focus()` when the index is updated + const [highlightIndex, updateHighlightIndex] = useHighlightReducer( + descendants, + index => { + if (isDefined(index)) { + descendants[index]?.element?.focus(); + } + }, + ); - focusedRef.current = el; - el.focus(); + // Callback fired when the popover transition finishes. + // Handling on this event ensures that the `descendants` elements + // exist in the DOM before attempting to set `focus` + const handlePopoverOpen = () => { + updateHighlightIndex('first'); }; - useEffect(() => { - if (open) { - hasSetInitialFocus.current = false; - hasSetInitialOpen.current = false; - } - }, [open]); - - useBackdropClick(handleClose, [popoverRef, triggerRef], open); - + // Fired on global keyDown event function handleKeyDown(e: KeyboardEvent) { - let refToFocus: HTMLElement; - switch (e.key) { case keyMap.ArrowDown: e.preventDefault(); // Prevents page scrolling - refToFocus = - refs[(refs.indexOf(focusedRef.current!) + 1) % refs.length]; - - setFocus(refToFocus); + updateHighlightIndex('next'); break; case keyMap.ArrowUp: e.preventDefault(); // Prevents page scrolling - refToFocus = - refs[ - (refs.indexOf(focusedRef.current!) - 1 + refs.length) % refs.length - ]; - setFocus(refToFocus); + updateHighlightIndex('prev'); break; case keyMap.Tab: e.preventDefault(); // Prevent tabbing outside of portal and outside of the DOM when `usePortal={true}` handleClose(); - setFocus((refEl || triggerRef)?.current); // Focus the trigger on close + (refEl || triggerRef)?.current?.focus(); // Focus the trigger on close break; case keyMap.Escape: handleClose(); - setFocus((refEl || triggerRef)?.current); // Focus the trigger on close + (refEl || triggerRef)?.current?.focus(); // Focus the trigger on close break; case keyMap.Space: case keyMap.Enter: if (!open) { - setFocus(refs[0]); + updateHighlightIndex('first'); } break; } @@ -338,50 +163,60 @@ export const Menu = React.forwardRef(function Menu( : { spacing, usePortal }), }; - const providerData = useMemo(() => { - return { theme, darkMode }; - }, [theme, darkMode]); - const popoverContent = ( - - + -
- {/* Need to stop propagation, otherwise Menu will closed automatically when clicked */} - {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events*/} -
    e.stopPropagation()} - ref={popoverRef} +
    - {updatedChildren} -
-
- - + {/* Need to stop propagation, otherwise Menu will closed automatically when clicked */} + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events*/} +
    e.stopPropagation()} + ref={popoverRef} + > + {children} +
+
+ + + ); if (trigger) { const triggerClickHandler = (event?: React.MouseEvent) => { + event?.preventDefault(); setOpen((curr: boolean) => !curr); if (trigger && typeof trigger !== 'function') { diff --git a/packages/menu/src/Menu/utils/useMenuHeight.ts b/packages/menu/src/Menu/utils/useMenuHeight.ts new file mode 100644 index 0000000000..adc04f0ad7 --- /dev/null +++ b/packages/menu/src/Menu/utils/useMenuHeight.ts @@ -0,0 +1,28 @@ +import { useMemo } from 'react'; +import { isUndefined } from 'lodash'; + +import { useAvailableSpace } from '@leafygreen-ui/hooks'; + +interface MenuHeightArgs { + refEl: React.RefObject; + spacing: number; + maxHeight: number; +} + +export const useMenuHeight = ({ + refEl, + spacing, + maxHeight, +}: MenuHeightArgs) => { + // This hook causes a second re-render on initial load. `useAvailableSpace` uses `useViewportSize` internally, which has internal state that causes re-renders. + const availableSpace = useAvailableSpace(refEl, spacing); + + const memoizedAvailableSpace = useMemo( + () => availableSpace, + [availableSpace], + ); + const maxMenuHeightValue = !isUndefined(memoizedAvailableSpace) + ? `${Math.min(memoizedAvailableSpace, maxHeight)}px` + : 'unset'; + return maxMenuHeightValue; +}; diff --git a/packages/menu/src/MenuContext/MenuContext.tsx b/packages/menu/src/MenuContext/MenuContext.tsx index 5e439b3671..67e960f547 100644 --- a/packages/menu/src/MenuContext/MenuContext.tsx +++ b/packages/menu/src/MenuContext/MenuContext.tsx @@ -1,10 +1,17 @@ import { createContext } from 'react'; -import { MenuData } from './MenuContext.types'; +import { createDescendantsContext } from '@leafygreen-ui/descendants'; -export const MenuContext = createContext({ +import { MenuContextData } from './MenuContext.types'; + +export const MenuDescendantsContext = createDescendantsContext( + 'MenuDescendantsContext', +); + +export const MenuContext = createContext({ theme: 'light', darkMode: false, + highlightIndex: undefined, }); export default MenuContext; diff --git a/packages/menu/src/MenuContext/MenuContext.types.ts b/packages/menu/src/MenuContext/MenuContext.types.ts index c41fd06530..ee6b627355 100644 --- a/packages/menu/src/MenuContext/MenuContext.types.ts +++ b/packages/menu/src/MenuContext/MenuContext.types.ts @@ -1,6 +1,7 @@ import { Theme } from '@leafygreen-ui/lib'; -export interface MenuData { +export interface MenuContextData { theme: Theme; darkMode: boolean; + highlightIndex?: number; } diff --git a/packages/menu/src/MenuContext/index.ts b/packages/menu/src/MenuContext/index.ts index a711137fd3..ffd8f7ddfd 100644 --- a/packages/menu/src/MenuContext/index.ts +++ b/packages/menu/src/MenuContext/index.ts @@ -1 +1 @@ -export { default as MenuContext } from './MenuContext'; +export { default as MenuContext, MenuDescendantsContext } from './MenuContext'; diff --git a/packages/menu/src/MenuItem/MenuItem.stories.tsx b/packages/menu/src/MenuItem/MenuItem.stories.tsx index 75ed02cdf2..c34e1fba41 100644 --- a/packages/menu/src/MenuItem/MenuItem.stories.tsx +++ b/packages/menu/src/MenuItem/MenuItem.stories.tsx @@ -6,12 +6,13 @@ import { StoryMetaType } from '@lg-tools/storybook-utils'; import Icon from '@leafygreen-ui/icon'; import { Theme } from '@leafygreen-ui/lib'; +import { MenuProps } from '../Menu/Menu.types'; import { MenuContext } from '../MenuContext'; import { Size } from '../types'; import MenuItem from './MenuItem'; -const meta: StoryMetaType = { +export default { title: 'Components/Menu/MenuItem', component: MenuItem, parameters: { @@ -33,8 +34,8 @@ const meta: StoryMetaType = { return ( @@ -43,7 +44,6 @@ const meta: StoryMetaType = { }, }, }, -}; -export default meta; +} satisfies StoryMetaType>; export const Generated = () => {}; diff --git a/packages/menu/src/MenuItem/MenuItem.styles.ts b/packages/menu/src/MenuItem/MenuItem.styles.ts index 515863bcfb..3ffe873d38 100644 --- a/packages/menu/src/MenuItem/MenuItem.styles.ts +++ b/packages/menu/src/MenuItem/MenuItem.styles.ts @@ -2,6 +2,10 @@ import { css } from '@leafygreen-ui/emotion'; import { Theme } from '@leafygreen-ui/lib'; import { palette } from '@leafygreen-ui/palette'; +export const menuItemContainerStyles = css` + list-style: none; +`; + export const disabledIconStyle: Record = { [Theme.Light]: css` color: ${palette.gray.dark2}; diff --git a/packages/menu/src/MenuItem/MenuItem.tsx b/packages/menu/src/MenuItem/MenuItem.tsx index 517d3e9d70..642b47a140 100644 --- a/packages/menu/src/MenuItem/MenuItem.tsx +++ b/packages/menu/src/MenuItem/MenuItem.tsx @@ -1,6 +1,7 @@ import React, { useContext } from 'react'; import PropTypes from 'prop-types'; +import { useDescendant } from '@leafygreen-ui/descendants'; import { cx } from '@leafygreen-ui/emotion'; import { createUniqueClassName, getNodeTextContent } from '@leafygreen-ui/lib'; import { @@ -8,7 +9,7 @@ import { useInferredPolymorphic, } from '@leafygreen-ui/polymorphic'; -import MenuContext from '../MenuContext/MenuContext'; +import { MenuContext, MenuDescendantsContext } from '../MenuContext'; import { activeDescriptionTextStyle, activeIconStyle, @@ -33,7 +34,11 @@ import { } from '../styles'; import { Size } from '../types'; -import { destructiveIconStyle, disabledIconStyle } from './MenuItem.styles'; +import { + destructiveIconStyle, + disabledIconStyle, + menuItemContainerStyles, +} from './MenuItem.styles'; import { MenuItemProps, Variant } from './MenuItem.types'; const menuItemContainerClassName = createUniqueClassName('menu-item-container'); @@ -52,10 +57,14 @@ export const MenuItem = InferredPolymorphic( variant = Variant.Default, ...rest }, - ref: React.Ref, + fwdRef: React.Ref, ) => { const { Component } = useInferredPolymorphic(as, rest, 'button'); - const { theme } = useContext(MenuContext); + const { theme, highlightIndex: _highlightIndex } = useContext(MenuContext); + const { ref } = useDescendant(MenuDescendantsContext, fwdRef, { + active, + disabled, + }); const hoverStyles = getHoverStyles(menuItemContainerClassName, theme); const focusStyles = getFocusedStyles(menuItemContainerClassName, theme); const isDestructive = variant === Variant.Destructive; @@ -81,7 +90,6 @@ export const MenuItem = InferredPolymorphic( }); const baseProps = { - ref, role: 'menuitem', tabIndex: -1, 'aria-disabled': disabled, @@ -138,8 +146,9 @@ export const MenuItem = InferredPolymorphic( ); return ( -
  • +
  • ( ( @@ -70,21 +72,32 @@ export const SubMenu = InferredPolymorphic( children, onClick, description, - setOpen, className, glyph, onExited = () => {}, - open = false, + open: openProp = false, + setOpen: setOpenProp, active = false, disabled = false, size = Size.Default, as, ...rest }, - ref: React.Ref, + fwdRef: React.Ref, ): React.ReactElement => { const { Component } = useInferredPolymorphic(as, rest, 'button'); - const { theme, darkMode } = useContext(MenuContext); + const { + theme, + darkMode, + highlightIndex: _highlightIndex, + } = useContext(MenuContext); + const { ref } = useDescendant(MenuDescendantsContext, fwdRef, { + active, + disabled, + }); + + const [open, setOpen] = useControlledState(false, openProp, setOpenProp); + const hoverStyles = getHoverStyles(subMenuContainerClassName, theme); const focusStyles = getFocusedStyles(subMenuContainerClassName, theme); @@ -118,10 +131,7 @@ export const SubMenu = InferredPolymorphic( const handleChevronClick = (e: React.MouseEvent) => { // we stop the event from propagating and closing the entire menu e.nativeEvent.stopImmediatePropagation(); - - if (setOpen) { - setOpen(!open); - } + setOpen(o => !o); }; // TODO: This code is duplicated in `MenuItem` @@ -145,7 +155,6 @@ export const SubMenu = InferredPolymorphic( }); const baseProps = { - ref, role: 'menuitem', 'aria-haspopup': true, onClick: onRootClick, @@ -203,6 +212,7 @@ export const SubMenu = InferredPolymorphic( return (
  • ( role="menu" aria-label={title} > + {/* TODO: Remove map. Replace with SubMenu context. Read from this context in MenuItem */} {React.Children.map( children as React.ReactElement, (child, index) => { diff --git a/packages/menu/src/SubMenu/SubMenu.types.ts b/packages/menu/src/SubMenu/SubMenu.types.ts index 0efbe04536..58b01433d0 100644 --- a/packages/menu/src/SubMenu/SubMenu.types.ts +++ b/packages/menu/src/SubMenu/SubMenu.types.ts @@ -1,3 +1,4 @@ +import { type Dispatch, type SetStateAction } from 'react'; import { ExitHandler } from 'react-transition-group/Transition'; import { HTMLElementProps } from '@leafygreen-ui/lib'; @@ -13,7 +14,7 @@ export interface SubMenuProps extends HTMLElementProps<'button'> { /** * Function to set the value of `open` in `` */ - setOpen?: (value: boolean) => void; + setOpen?: Dispatch>; /** * className applied to `SubMenu` root element diff --git a/packages/menu/src/SubMenu/useControlledState.ts b/packages/menu/src/SubMenu/useControlledState.ts new file mode 100644 index 0000000000..53428bade1 --- /dev/null +++ b/packages/menu/src/SubMenu/useControlledState.ts @@ -0,0 +1,30 @@ +import { type Dispatch, type SetStateAction, useEffect, useState } from 'react'; +import isUndefined from 'lodash/isUndefined'; + +import { consoleOnce } from '@leafygreen-ui/lib'; + +export const useControlledState = ( + initialState: T, + controlledState?: T, + setControlledState?: Dispatch>, +): [T, Dispatch>] => { + const isControlled = + !isUndefined(controlledState) && !isUndefined(setControlledState); + const [internalState, setInternalState] = useState(initialState); + + useEffect(() => { + // Log a warning if neither controlled value or initialValue is provided + if (!isControlled && isUndefined(initialState)) { + consoleOnce.error( + `Warning: \`useControlledState\` hook is being used without a \`controlledState\` or \`initialState\`.` + + `This will cause a React warning when the input changes.` + + `Please decide between using a controlled or uncontrolled input element, and provide either a \`controlledState\` or \`initialState\` to \`useControlledState\``, + ); + } + }, [isControlled, initialState]); + + return [ + isControlled ? controlledState : internalState, + isControlled ? setControlledState : setInternalState, + ]; +}; diff --git a/packages/menu/src/styles.ts b/packages/menu/src/styles.ts index b39ca70e6c..f2e4cf1194 100644 --- a/packages/menu/src/styles.ts +++ b/packages/menu/src/styles.ts @@ -305,7 +305,7 @@ export const disabledTextStyle: Record = { */ export const focusedMenuItemContainerStyle: Record = { [Theme.Light]: css` - &:focus-visible { + &:focus { text-decoration: none; background-color: ${palette.blue.dark3}; color: ${palette.white}; @@ -320,7 +320,7 @@ export const focusedMenuItemContainerStyle: Record = { } `, [Theme.Dark]: css` - &:focus-visible { + &:focus { text-decoration: none; background-color: ${palette.blue.dark3}; color: ${palette.blue.light3}; @@ -343,21 +343,21 @@ export const focusedMenuItemContainerStyle: Record = { export const getFocusedStyles = (containerClassName: string, theme: Theme) => { return { textStyle: css` - .${containerClassName}:focus-visible & { + .${containerClassName}:focus & { color: ${theme === Theme.Light ? palette.blue.light3 : palette.blue.light3}; } `, descriptionStyle: css` - .${containerClassName}:focus-visible & { + .${containerClassName}:focus & { color: ${theme === Theme.Light ? palette.blue.light3 : palette.blue.light3}; } `, iconStyle: css` - .${containerClassName}:focus-visible & { + .${containerClassName}:focus & { color: ${theme === Theme.Light ? palette.blue.light3 : palette.blue.light3}; @@ -386,14 +386,14 @@ export const linkStyle = css` export const focusedSubMenuItemBorderStyles: Record = { [Theme.Light]: css` - &:focus-visible { + &:focus { &::after { background-color: ${palette.blue.dark3}; } } `, [Theme.Dark]: css` - &:focus-visible { + &:focus { &::after { background-color: ${palette.blue.dark3}; } diff --git a/packages/menu/tsconfig.json b/packages/menu/tsconfig.json index 6de8bf09a1..38f37e4bd4 100644 --- a/packages/menu/tsconfig.json +++ b/packages/menu/tsconfig.json @@ -15,6 +15,9 @@ ], "exclude": ["**/*.spec.*", "**/*.stories.*"], "references": [ + { + "path": "../descendants" + }, { "path": "../emotion" }, diff --git a/packages/split-button/src/SplitButton/SplitButton.spec.tsx b/packages/split-button/src/SplitButton/SplitButton.spec.tsx index 8a268eba35..a8ed8a1532 100644 --- a/packages/split-button/src/SplitButton/SplitButton.spec.tsx +++ b/packages/split-button/src/SplitButton/SplitButton.spec.tsx @@ -4,6 +4,7 @@ import { getAllByRole as globalGetAllByRole, render, waitForElementToBeRemoved, + within, } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { axe } from 'jest-axe'; @@ -34,17 +35,49 @@ const defaultProps = { }; function renderSplitButton(props = {}) { - const utils = render( + const renderResult = render( , ); - const wrapper = utils.container.firstChild as HTMLElement; - const primaryButton = utils.getByTestId('split-button'); + const wrapper = renderResult.container.firstChild as HTMLElement; + const primaryButton = renderResult.getByTestId('split-button'); const menuTrigger = primaryButton.nextSibling as HTMLElement; + + /** + * Since menu elements won't exist until component is interacted with, + * call this after opening the menu. + * @returns Object of menu elements + */ + // TODO: Consolidate with Menu component util + async function findMenuElements(): Promise<{ + menuEl: HTMLElement | null; + menuItemElements: Array; + }> { + const menuEl = await renderResult.findByTestId(menuTestId); + const menuItemElements = await within(menuEl).findAllByRole('menuitem'); + + return { + menuEl, + menuItemElements, + }; + } + + /** + * Opens the menu, and manually fires transition events + */ + async function openMenu() { + userEvent.click(menuTrigger); + const menuElements = await findMenuElements(); + fireEvent.transitionEnd(menuElements.menuEl as Element); // JSDOM does not automatically fire these events + return menuElements; + } + return { - ...utils, + ...renderResult, primaryButton, menuTrigger, wrapper, + findMenuElements, + openMenu, }; } @@ -162,6 +195,7 @@ describe('packages/split-button', () => { }); }); + // TODO: Consolidate tests with Menu component describe('Keyboard Interaction', () => { type CloseKeys = 'esc' | 'tab'; const closeKeys: Array> = [['esc'], ['tab']]; @@ -176,35 +210,32 @@ describe('packages/split-button', () => { describe.each(closeKeys)('%s key', key => { test('Closes menu', async () => { - const { getByTestId, menuTrigger } = renderSplitButton({}); - userEvent.click(menuTrigger); - const menu = getByTestId(menuTestId); + const { openMenu } = renderSplitButton({}); + const { menuEl } = await openMenu(); - userEventInteraction(menu, key); - await waitForElementToBeRemoved(menu); - expect(menu).not.toBeInTheDocument(); + userEventInteraction(menuEl!, key); + await waitForElementToBeRemoved(menuEl); + expect(menuEl).not.toBeInTheDocument(); }); test('Returns focus to trigger {usePortal: true}', async () => { - const { getByTestId, menuTrigger } = renderSplitButton({ + const { openMenu, menuTrigger } = renderSplitButton({ usePortal: true, }); - userEvent.click(menuTrigger); - const menu = getByTestId(menuTestId); + const { menuEl } = await openMenu(); - userEventInteraction(menu, key); - await waitForElementToBeRemoved(menu); + userEventInteraction(menuEl!, key); + await waitForElementToBeRemoved(menuEl); expect(menuTrigger).toHaveFocus(); }); test('Returns focus to trigger {usePortal: false}', async () => { - const { getByTestId, menuTrigger } = renderSplitButton({ + const { openMenu, menuTrigger } = renderSplitButton({ usePortal: false, }); - userEvent.click(menuTrigger); - const menu = getByTestId(menuTestId); + const { menuEl } = await openMenu(); - userEventInteraction(menu, key); - await waitForElementToBeRemoved(menu); + userEventInteraction(menuEl!, key); + await waitForElementToBeRemoved(menuEl); expect(menuTrigger).toHaveFocus(); }); }); @@ -214,87 +245,92 @@ describe('packages/split-button', () => { describe.each(selectionKeys)('%s key', key => { const onClick = jest.fn(); - let menu: HTMLElement; - let options: Array; - let menuTrigger: HTMLElement; - - beforeEach(() => { - const menuItems = [ - - Menu Item With Description - , - - Disabled Menu Item - , - ]; - const { getByTestId, menuTrigger: menuTriggerEl } = renderSplitButton({ - menuItems, - }); - userEvent.click(menuTriggerEl); - menu = getByTestId(menuTestId); - options = globalGetAllByRole(menu, 'menuitem'); - menuTrigger = menuTriggerEl; + const menuItems = [ + + Menu Item With Description + , + + Disabled Menu Item + , + ]; + + afterEach(() => { + onClick.mockReset(); }); - test('Fires the click handler of the highlighted item', () => { - expect(options[0]).toHaveFocus(); + test('Fires the click handler of the highlighted item', async () => { + const { openMenu } = renderSplitButton({ + menuItems, + }); + const { menuItemElements } = await openMenu(); + expect(menuItemElements[0]).toHaveFocus(); - userEvent.type(options[0], `{${key}}`); + userEvent.type(menuItemElements?.[0]!, `{${key}}`); expect(onClick).toHaveBeenCalled(); }); - /* eslint-disable jest/no-disabled-tests */ + // eslint-disable-next-line jest/no-disabled-tests test.skip('Closes the menu', async () => { // Works correctly in the browser // https://github.com/testing-library/react-testing-library/issues/269#issuecomment-1453666401 - this needs v13 of testing-library // TODO: This is not triggered so the test fails - userEvent.type(options[0], `{${key}}`); - expect(menuTrigger).toHaveFocus(); + const { openMenu, menuTrigger } = renderSplitButton({ + menuItems, + }); + const { menuEl, menuItemElements } = await openMenu(); + userEvent.type(menuItemElements?.[0]!, `{${key}}`); - await waitForElementToBeRemoved(menu); + expect(menuTrigger).toHaveFocus(); + await waitForElementToBeRemoved(menuEl); }); }); describe('Arrow keys', () => { - let menu: HTMLElement; - let options: Array; - - beforeEach(() => { - const { getByTestId, menuTrigger } = renderSplitButton(); - userEvent.click(menuTrigger); - menu = getByTestId(menuTestId); - options = globalGetAllByRole(menu, 'menuitem'); - }); - + const menuItems = [ + Item 0, + Item 1, + Item 2, + Item 3, + ]; describe('Down arrow', () => { - test('highlights the next option in the menu', () => { - userEvent.type(menu, '{arrowdown}'); - // options[1] is disabled - expect(options[2]).toHaveFocus(); + test('highlights the next option in the menu', async () => { + const { openMenu } = renderSplitButton({ menuItems }); + const { menuEl, menuItemElements } = await openMenu(); + userEvent.type(menuEl!, '{arrowdown}'); + expect(menuItemElements[1]).toHaveFocus(); }); - test('cycles highlight to the top', () => { - // programmatically set focus on last option - options[options.length - 1].focus(); - userEvent.type(menu, '{arrowdown}'); - expect(options[0]).toHaveFocus(); + test('cycles highlight to the top', async () => { + const { openMenu } = renderSplitButton({ menuItems }); + const { menuEl, menuItemElements } = await openMenu(); + + for (let i = 0; i < menuItemElements.length; i++) { + userEvent.type(menuEl!, '{arrowdown}'); + } + + expect(menuItemElements[0]).toHaveFocus(); }); }); describe('Up arrow', () => { - test('highlights the previous option in the menu', () => { - // programmatically set focus on second option - // options[1] is disabled - options[2].focus(); - userEvent.type(menu, '{arrowup}'); - expect(options[0]).toHaveFocus(); + test('highlights the previous option in the menu', async () => { + const { openMenu } = renderSplitButton({ menuItems }); + const { menuEl, menuItemElements } = await openMenu(); + + userEvent.type(menuEl!, '{arrowdown}'); + userEvent.type(menuEl!, '{arrowup}'); + expect(menuItemElements[0]).toHaveFocus(); }); - test('cycles highlight to the bottom', () => { - userEvent.type(menu, '{arrowup}'); - expect(options[options.length - 1]).toHaveFocus(); + test('cycles highlight to the bottom', async () => { + const { openMenu } = renderSplitButton({ menuItems }); + const { menuEl, menuItemElements } = await openMenu(); + + const lastOption = menuItemElements[menuItemElements.length - 1]; + userEvent.type(menuEl!, '{arrowup}'); + expect(lastOption).toHaveFocus(); }); }); });