From d32f6ae6af463e7827384956d609e6b95b64caab Mon Sep 17 00:00:00 2001 From: Felix Habib Date: Tue, 3 Dec 2024 11:08:34 +1100 Subject: [PATCH] MenuRenderer: Ensure menu is visible in container with overflow hidden --- .changeset/fair-horses-sneeze.md | 10 + .changeset/four-cobras-suffer.md | 12 + .../MenuRenderer/MenuRenderer.actions.ts | 4 +- .../MenuRenderer/MenuRenderer.css.ts | 36 ++- .../MenuRenderer/MenuRenderer.screenshots.tsx | 44 +-- .../components/MenuRenderer/MenuRenderer.tsx | 147 +++++++--- .../components/MenuRenderer/testHelper.tsx | 261 ++++++++++-------- .../OverflowMenu/OverflowMenu.css.ts | 4 +- .../components/OverflowMenu/OverflowMenu.tsx | 26 +- .../src/lib/stories/all.stories.tsx | 3 +- 10 files changed, 350 insertions(+), 197 deletions(-) create mode 100644 .changeset/fair-horses-sneeze.md create mode 100644 .changeset/four-cobras-suffer.md diff --git a/.changeset/fair-horses-sneeze.md b/.changeset/fair-horses-sneeze.md new file mode 100644 index 00000000000..b37e4416f53 --- /dev/null +++ b/.changeset/fair-horses-sneeze.md @@ -0,0 +1,10 @@ +--- +'braid-design-system': patch +--- + +--- +updated: + - MenuRenderer +--- + +**MenuRenderer**: Ensure menu is visible, even when its trigger element is inside a container with overflow hidden. diff --git a/.changeset/four-cobras-suffer.md b/.changeset/four-cobras-suffer.md new file mode 100644 index 00000000000..c886718f718 --- /dev/null +++ b/.changeset/four-cobras-suffer.md @@ -0,0 +1,12 @@ +--- +'braid-design-system': patch +--- + +--- +updated: + - OverflowMenu +--- + +**OverflowMenu**: Simplify internal layout. + +Refactor the internal layout of `OverflowMenu` to improve the alignment of the menu with the button. diff --git a/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.actions.ts b/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.actions.ts index 68f33172db5..03593ef6dfd 100644 --- a/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.actions.ts +++ b/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.actions.ts @@ -16,6 +16,7 @@ export const actionTypes = { MENU_TRIGGER_TAB: 13, MENU_TRIGGER_ESCAPE: 14, BACKDROP_CLICK: 15, + WINDOW_RESIZE: 16, } as const; export type Action = @@ -49,4 +50,5 @@ export type Action = | { type: typeof actionTypes.MENU_TRIGGER_CLICK } | { type: typeof actionTypes.MENU_TRIGGER_TAB } | { type: typeof actionTypes.MENU_TRIGGER_ESCAPE } - | { type: typeof actionTypes.BACKDROP_CLICK }; + | { type: typeof actionTypes.BACKDROP_CLICK } + | { type: typeof actionTypes.WINDOW_RESIZE }; diff --git a/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.css.ts b/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.css.ts index 8bbbdd32072..b8ac8ae4d77 100644 --- a/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.css.ts +++ b/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.css.ts @@ -1,4 +1,9 @@ -import { createVar, style, styleVariants } from '@vanilla-extract/css'; +import { + createVar, + keyframes, + style, + styleVariants, +} from '@vanilla-extract/css'; import { calc } from '@vanilla-extract/css-utils'; import { vars } from '../../themes/vars.css'; @@ -7,9 +12,28 @@ export const backdrop = style({ height: '100vh', }); -export const menuIsClosed = style({ - transform: `translateY(${calc(vars.grid).negate().multiply(2)})`, - visibility: 'hidden', +export const triggerVars = { + top: createVar(), + left: createVar(), + bottom: createVar(), + right: createVar(), +}; + +// Top and bottom reversed to allow for a more natural API +export const menuPosition = style({ + top: triggerVars.bottom, + bottom: triggerVars.top, + left: triggerVars.left, + right: triggerVars.right, +}); + +export const animation = style({ + animation: `${keyframes({ + from: { + transform: `translateY(${calc(vars.grid).negate().multiply(2)})`, + opacity: 0, + }, + })} .125s ease forwards`, }); const widthVar = createVar(); @@ -23,10 +47,6 @@ export const width = styleVariants({ small, medium, large }, (w) => [ { vars: { [widthVar]: w } }, ]); -export const placementBottom = style({ - bottom: '100%', -}); - export const menuYPadding = 'xxsmall'; export const menuHeightLimit = style({ diff --git a/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.screenshots.tsx b/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.screenshots.tsx index d376dd621b0..8220212bd79 100644 --- a/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.screenshots.tsx +++ b/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.screenshots.tsx @@ -28,6 +28,15 @@ const defaultProps = { placement: 'bottom', } as const; +const triggerHeight = 44; + +const triggerPosition = { + top: triggerHeight, + left: 0, + bottom: 0, // this value is ignored when placement is top + right: 0, // this value is ignored when align is left +}; + export const screenshots: ComponentScreenshot = { screenshotWidths: [320], examples: [ @@ -39,7 +48,7 @@ export const screenshots: ComponentScreenshot = { offsetSpace="small" trigger={(triggerProps) => ( - + )} > @@ -58,7 +67,7 @@ export const screenshots: ComponentScreenshot = { offsetSpace="small" trigger={(triggerProps) => ( - + )} > @@ -76,7 +85,7 @@ export const screenshots: ComponentScreenshot = { offsetSpace="small" trigger={(triggerProps) => ( - + )} > @@ -90,7 +99,7 @@ export const screenshots: ComponentScreenshot = { Example: () => ( - + {}}>Item {}}>Item @@ -104,7 +113,7 @@ export const screenshots: ComponentScreenshot = { Example: () => ( - + {}}>Item {}}>Item @@ -120,15 +129,16 @@ export const screenshots: ComponentScreenshot = { display="flex" style={{ paddingTop: calc(vars.touchableSize).multiply(2.5).toString(), - marginBottom: calc(vars.touchableSize) - .multiply(1.5) - .negate() - .toString(), }} > - - + + {}}>Item {}}>Item @@ -143,15 +153,17 @@ export const screenshots: ComponentScreenshot = { display="flex" style={{ paddingTop: calc(vars.touchableSize).multiply(2.5).toString(), - marginBottom: calc(vars.touchableSize) - .multiply(1.5) - .negate() - .toString(), }} > - + {}}>Item {}}>Item diff --git a/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.tsx b/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.tsx index ce3874e9d5c..67d2a72b51c 100644 --- a/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.tsx +++ b/packages/braid-design-system/src/lib/components/MenuRenderer/MenuRenderer.tsx @@ -4,7 +4,6 @@ import React, { type MouseEvent, type ReactNode, type Ref, - type ReactChild, Children, useRef, useReducer, @@ -24,6 +23,8 @@ import buildDataAttributes, { type DataAttributeMap, } from '../private/buildDataAttributes'; import * as styles from './MenuRenderer.css'; +import { BraidPortal } from '../BraidPortal/BraidPortal'; +import { assignInlineVars } from '@vanilla-extract/dynamic'; interface TriggerProps { 'aria-haspopup': boolean; @@ -75,12 +76,32 @@ const { MENU_TRIGGER_TAB, MENU_TRIGGER_ESCAPE, BACKDROP_CLICK, + WINDOW_RESIZE, } = actionTypes; +type Position = { top: number; bottom: number; left: number; right: number }; + +const getPosition = (element: HTMLElement | null): Position | undefined => { + if (!element) { + return undefined; + } + + const { top, bottom, left, right } = element.getBoundingClientRect(); + const { scrollX, scrollY, innerWidth, innerHeight } = window; + + return { + top: innerHeight - top - scrollY, + bottom: bottom + scrollY, + left: left + scrollX, + right: innerWidth - right - scrollX, + }; +}; + interface State { open: boolean; highlightIndex: number; closeReason: CloseReason; + triggerPosition?: Position; } const CLOSED_INDEX = -1; @@ -89,6 +110,7 @@ const initialState: State = { open: false, highlightIndex: CLOSED_INDEX, closeReason: CLOSE_REASON_EXIT, + triggerPosition: undefined, }; export const MenuRenderer = ({ @@ -104,6 +126,7 @@ export const MenuRenderer = ({ data, ...restProps }: MenuRendererProps) => { + const menuContainerRef = useRef(null); const buttonRef = useRef(null); const lastOpen = useRef(false); const items = flattenChildren(children); @@ -120,8 +143,8 @@ export const MenuRenderer = ({ 'All child nodes within a menu component must be a MenuItem, MenuItemLink, MenuItemCheckbox or MenuItemDivider: https://seek-oss.github.io/braid-design-system/components/MenuRenderer', ); - const [{ open, highlightIndex, closeReason }, dispatch] = useReducer( - (state: State, action: Action): State => { + const [{ open, highlightIndex, closeReason, triggerPosition }, dispatch] = + useReducer((state: State, action: Action): State => { switch (action.type) { case MENU_TRIGGER_UP: case MENU_ITEM_UP: { @@ -130,6 +153,7 @@ export const MenuRenderer = ({ open: true, closeReason: CLOSE_REASON_EXIT, highlightIndex: getNextIndex(-1, state.highlightIndex, itemCount), + triggerPosition: getPosition(menuContainerRef.current), }; } case MENU_TRIGGER_DOWN: @@ -139,6 +163,7 @@ export const MenuRenderer = ({ open: true, closeReason: CLOSE_REASON_EXIT, highlightIndex: getNextIndex(1, state.highlightIndex, itemCount), + triggerPosition: getPosition(menuContainerRef.current), }; } case BACKDROP_CLICK: @@ -183,22 +208,29 @@ export const MenuRenderer = ({ open: nextOpen, closeReason: CLOSE_REASON_EXIT, highlightIndex: nextOpen ? 0 : CLOSED_INDEX, + triggerPosition: getPosition(menuContainerRef.current), }; } case MENU_TRIGGER_CLICK: { const nextOpen = !state.open; + return { ...state, open: nextOpen, closeReason: CLOSE_REASON_EXIT, + triggerPosition: getPosition(menuContainerRef.current), + }; + } + case WINDOW_RESIZE: { + return { + ...state, + triggerPosition: getPosition(menuContainerRef.current), }; } default: return state; } - }, - initialState, - ); + }, initialState); useEffect(() => { if (lastOpen.current === open) { @@ -220,6 +252,20 @@ export const MenuRenderer = ({ } }; + useEffect(() => { + const handleResize = () => { + dispatch({ type: WINDOW_RESIZE }); + }; + + if (open) { + window.addEventListener('resize', handleResize); + } + + return () => { + window.removeEventListener('resize', handleResize); + }; + }, [open]); + const onTriggerKeyUp = (event: KeyboardEvent) => { const targetKey = normalizeKey(event); @@ -286,38 +332,42 @@ export const MenuRenderer = ({ }; return ( - - - {trigger(triggerProps, { open })} - - - {items} - - + + {trigger(triggerProps, { open })} {open ? ( - { - event.stopPropagation(); - event.preventDefault(); - dispatch({ type: BACKDROP_CLICK }); - }} - position="fixed" - zIndex="dropdownBackdrop" - top={0} - left={0} - className={styles.backdrop} - /> + <> + + + {items} + + + { + event.stopPropagation(); + event.preventDefault(); + dispatch({ type: BACKDROP_CLICK }); + }} + position="fixed" + zIndex="dropdownBackdrop" + top={0} + left={0} + className={styles.backdrop} + /> + ) : null} ); @@ -340,25 +390,33 @@ interface MenuProps { dispatch: (action: Action) => void; focusTrigger: () => void; highlightIndex: number; - open: boolean; - children: ReactChild[]; - position?: 'absolute' | 'relative'; + children: ReactNode[]; + triggerPosition?: Position; + position?: 'absolute' | 'relative'; // 'relative' is used for screenshot testing } + export function Menu({ offsetSpace, align, width, placement, children, - open, dispatch, focusTrigger, highlightIndex, reserveIconSpace, + triggerPosition, position = 'absolute', }: MenuProps) { let dividerCount = 0; + const inlineVars = + triggerPosition && + assignInlineVars({ + [styles.triggerVars[placement]]: `${triggerPosition[placement]}px`, + [styles.triggerVars[align]]: `${triggerPosition[align]}px`, + }); + return ( @@ -386,9 +443,7 @@ export function Menu({ dividerCount++; return item; } - const menuItemIndex = i - dividerCount; - return ( { const defaultOpen = jest.fn(); const defaultClose = jest.fn(); - const { getAllByRole, rerender } = render( + const { queryByRole, getByRole, getAllByRole, rerender } = render( , ); return { + queryByRole, + getByRole, getAllByRole, openHandler: defaultOpen, closeHandler: defaultClose, @@ -89,19 +91,12 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }; } - function getElements({ + function getMenuItems({ getAllByRole, }: { getAllByRole: RenderResult['getAllByRole']; }) { - return { - menuButton: getAllByRole('button')[0], - menu: getAllByRole('menu', { hidden: true })[0], - menuItems: [ - ...getAllByRole('menuitem', { hidden: true }), - ...getAllByRole('menuitemcheckbox', { hidden: true }), - ], - }; + return [...getAllByRole('menuitem'), ...getAllByRole('menuitemcheckbox')]; } describe(`Menu: ${name}`, () => { @@ -109,13 +104,14 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { afterEach(cleanup); it('should open menu when clicked', async () => { - const { getAllByRole, openHandler, closeHandler } = renderMenu(); - - const { menu, menuButton } = getElements({ getAllByRole }); + const { queryByRole, getByRole, openHandler, closeHandler } = + renderMenu(); - expect(menu).not.toBeVisible(); + const menuButton = getByRole('button'); + expect(queryByRole('menu')).not.toBeInTheDocument(); await userEvent.click(menuButton); + const menu = getByRole('menu'); expect(menu).toBeVisible(); expect(menuButton).toHaveFocus(); @@ -124,29 +120,30 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }); it('should toggle the menu when clicked again', async () => { - const { getAllByRole, closeHandler } = renderMenu(); + const { queryByRole, getByRole, closeHandler } = renderMenu(); - const { menu, menuButton } = getElements({ getAllByRole }); + const menuButton = getByRole('button'); await userEvent.click(menuButton); await userEvent.click(menuButton); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); expect(menuButton).toHaveFocus(); expect(closeHandler).toHaveBeenNthCalledWith(1, { reason: 'exit' }); }); it('should set the focused menu item on mouse over', async () => { - const { getAllByRole } = renderMenu(); + const { getByRole, getAllByRole } = renderMenu(); - const { menuButton, menuItems: initialMenuItems } = getElements({ - getAllByRole, - }); + const menuButton = getByRole('button'); await userEvent.click(menuButton); + + const initialMenuItems = getMenuItems({ getAllByRole }); + fireEvent.mouseOver(initialMenuItems[2]); - const { menuItems: mouseOverMenuItems } = getElements({ + const mouseOverMenuItems = getMenuItems({ getAllByRole, }); @@ -154,25 +151,27 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }); it('should not affect the focus on mouse out', async () => { - const { getAllByRole } = renderMenu(); + const { getByRole, getAllByRole } = renderMenu(); - const { menu, menuButton, menuItems } = getElements({ - getAllByRole, - }); + const menuButton = getByRole('button'); await userEvent.click(menuButton); + + const menu = getByRole('menu'); + + const menuItems = getMenuItems({ getAllByRole }); + fireEvent.mouseOver(menuItems[1]); fireEvent.mouseOut(menu); - const { menuItems: mouseOutMenuItems } = getElements({ - getAllByRole, - }); + const mouseOutMenuItems = getMenuItems({ getAllByRole }); expect(mouseOutMenuItems[1]).toHaveFocus(); }); it('should trigger the click handler on a MenuItem', async () => { const { + getByRole, getAllByRole, openHandler, closeHandler, @@ -180,9 +179,13 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { parentHandler, } = renderMenu(); - const { menu, menuButton, menuItems } = getElements({ getAllByRole }); + const menuButton = getByRole('button'); await userEvent.click(menuButton); + + const menu = getByRole('menu'); + const menuItems = getMenuItems({ getAllByRole }); + openHandler.mockClear(); // Clear initial open invocation, to allow later negative assertion expect(menu).toBeVisible(); @@ -206,12 +209,21 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { // Skipping for now due to not behaving correctly in test environment it.skip('should toggle the state on a MenuItemCheckbox', async () => { - const { getAllByRole, openHandler, closeHandler, menuItemHandler } = - renderMenu(); + const { + getByRole, + getAllByRole, + openHandler, + closeHandler, + menuItemHandler, + } = renderMenu(); - const { menu, menuButton, menuItems } = getElements({ getAllByRole }); + const menuButton = getByRole('button'); await userEvent.click(menuButton); + + const menu = getByRole('menu'); + const menuItems = getMenuItems({ getAllByRole }); + openHandler.mockClear(); // Clear initial open invocation, to allow later negative assertion expect(menu).toBeVisible(); @@ -221,8 +233,9 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { await userEvent.click(menuItemCheckbox); - const updatedElements = getElements({ getAllByRole }); - const updatedMenuItem = updatedElements.menuItems[2]; + const updatedMenuItems = getMenuItems({ getAllByRole }); + + const updatedMenuItem = updatedMenuItems[2]; expect(updatedMenuItem.getAttribute('aria-checked')).toBe('true'); @@ -238,14 +251,20 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { afterEach(cleanup); it('should open the menu with enter key', async () => { - const { getAllByRole, openHandler, closeHandler } = renderMenu(); + const { + queryByRole, + getByRole, + getAllByRole, + openHandler, + closeHandler, + } = renderMenu(); - const { menu } = getElements({ getAllByRole }); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); await userEvent.tab(); await userEvent.keyboard('{enter}'); - const { menuItems } = getElements({ getAllByRole }); + const menu = getByRole('menu'); + const menuItems = getMenuItems({ getAllByRole }); expect(menu).toBeVisible(); expect(menuItems[0]).toHaveFocus(); @@ -254,14 +273,21 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }); it('should open the menu with space key', async () => { - const { getAllByRole, openHandler, closeHandler } = renderMenu(); + const { + queryByRole, + getByRole, + getAllByRole, + openHandler, + closeHandler, + } = renderMenu(); - const { menu } = getElements({ getAllByRole }); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); await userEvent.tab(); await userEvent.keyboard(' '); - const { menuItems } = getElements({ getAllByRole }); + + const menu = getByRole('menu'); + const menuItems = getMenuItems({ getAllByRole }); expect(menu).toBeVisible(); expect(menuItems[0]).toHaveFocus(); @@ -270,14 +296,20 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }); it('should open the menu with down arrow key', async () => { - const { getAllByRole, openHandler, closeHandler } = renderMenu(); + const { + queryByRole, + getByRole, + getAllByRole, + openHandler, + closeHandler, + } = renderMenu(); - const { menu } = getElements({ getAllByRole }); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); await userEvent.tab(); await userEvent.keyboard('{arrowdown}'); - const { menuItems } = getElements({ getAllByRole }); + const menu = getByRole('menu'); + const menuItems = getMenuItems({ getAllByRole }); expect(menu).toBeVisible(); expect(menuItems[0]).toHaveFocus(); @@ -286,14 +318,20 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }); it('should open the menu with up arrow key', async () => { - const { getAllByRole, openHandler, closeHandler } = renderMenu(); + const { + queryByRole, + getByRole, + getAllByRole, + openHandler, + closeHandler, + } = renderMenu(); - const { menu } = getElements({ getAllByRole }); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); await userEvent.tab(); await userEvent.keyboard('{arrowup}'); - const { menuItems } = getElements({ getAllByRole }); + const menu = getByRole('menu'); + const menuItems = getMenuItems({ getAllByRole }); expect(menu).toBeVisible(); expect(menuItems[2]).toHaveFocus(); @@ -302,101 +340,102 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }); it('should close the menu with escape key', async () => { - const { getAllByRole, openHandler, closeHandler } = renderMenu(); + const { queryByRole, getByRole, openHandler, closeHandler } = + renderMenu(); - const { menu, menuButton } = getElements({ - getAllByRole, - }); + const menuButton = getByRole('button'); await userEvent.click(menuButton); openHandler.mockClear(); // Clear initial open invocation, to allow later negative assertion await userEvent.keyboard('{Escape}'); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); expect(menuButton).toHaveFocus(); expect(openHandler).not.toHaveBeenCalled(); expect(closeHandler).toHaveBeenNthCalledWith(1, { reason: 'exit' }); }); it('should close the menu with tab key', async () => { - const { getAllByRole, openHandler, closeHandler } = renderMenu(); + const { queryByRole, getByRole, openHandler, closeHandler } = + renderMenu(); - const { menu, menuButton } = getElements({ - getAllByRole, - }); + const menuButton = getByRole('button'); await userEvent.click(menuButton); openHandler.mockClear(); // Clear initial open invocation, to allow later negative assertion await userEvent.tab(); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); expect(document.body).toHaveFocus(); expect(openHandler).not.toHaveBeenCalled(); expect(closeHandler).toHaveBeenNthCalledWith(1, { reason: 'exit' }); }); it('should be able to navigate down the list and back to the start', async () => { - const { getAllByRole } = renderMenu(); + const { queryByRole, getAllByRole } = renderMenu(); - const { menu } = getElements({ getAllByRole }); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); await userEvent.tab(); await userEvent.keyboard('{arrowdown}'); - const firstDown = getElements({ getAllByRole }); - const firstMenuItem = firstDown.menuItems[0]; + const firstDownMenuItems = getMenuItems({ getAllByRole }); + + const firstMenuItem = firstDownMenuItems[0]; expect(firstMenuItem).toHaveFocus(); await userEvent.keyboard('{arrowdown}'); - const secondDown = getElements({ getAllByRole }); - const secondMenuItem = secondDown.menuItems[1]; + const secondDownMenuItems = getMenuItems({ getAllByRole }); + + const secondMenuItem = secondDownMenuItems[1]; expect(secondMenuItem).toHaveFocus(); await userEvent.keyboard('{arrowdown}'); - const thirdDown = getElements({ getAllByRole }); - const thirdMenuItem = thirdDown.menuItems[2]; + const thirdDownMenuItems = getMenuItems({ getAllByRole }); + + const thirdMenuItem = thirdDownMenuItems[2]; expect(thirdMenuItem).toHaveFocus(); await userEvent.keyboard('{arrowdown}'); - const forthDown = getElements({ getAllByRole }); - const firstMenuItemAgain = forthDown.menuItems[0]; + const forthDownMenuItems = getMenuItems({ getAllByRole }); + const firstMenuItemAgain = forthDownMenuItems[0]; expect(firstMenuItemAgain).toHaveFocus(); }); it('should be able to navigate up the list and back to the end', async () => { - const { getAllByRole } = renderMenu(); + const { queryByRole, getAllByRole } = renderMenu(); - const { menu } = getElements({ getAllByRole }); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); await userEvent.tab(); await userEvent.keyboard('{arrowup}'); - const firstUp = getElements({ getAllByRole }); - const thirdMenuItem = firstUp.menuItems[2]; + const firstUpMenuItems = getMenuItems({ getAllByRole }); + + const thirdMenuItem = firstUpMenuItems[2]; expect(thirdMenuItem).toHaveFocus(); await userEvent.keyboard('{arrowup}'); - const secondUp = getElements({ getAllByRole }); - const secondMenuItem = secondUp.menuItems[1]; + const secondUpMenuItems = getMenuItems({ getAllByRole }); + const secondMenuItem = secondUpMenuItems[1]; expect(secondMenuItem).toHaveFocus(); await userEvent.keyboard('{arrowup}'); - const thirdUp = getElements({ getAllByRole }); - const firstMenuItem = thirdUp.menuItems[0]; + const thirdUpMenuItems = getMenuItems({ getAllByRole }); + const firstMenuItem = thirdUpMenuItems[0]; expect(firstMenuItem).toHaveFocus(); await userEvent.keyboard('{arrowup}'); - const forthUp = getElements({ getAllByRole }); - const lastMenuItemAgain = forthUp.menuItems[2]; + const forthUpMenuItems = getMenuItems({ getAllByRole }); + const lastMenuItemAgain = forthUpMenuItems[2]; expect(lastMenuItemAgain).toHaveFocus(); }); it('should trigger the click handler on MenuItem when selecting it with enter', async () => { - const { getAllByRole, closeHandler, menuItemHandler } = renderMenu(); + const { queryByRole, getByRole, closeHandler, menuItemHandler } = + renderMenu(); - const { menu, menuButton } = getElements({ getAllByRole }); + const menuButton = getByRole('button'); // Open menu await userEvent.tab(); @@ -405,7 +444,7 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { // Action the item await userEvent.keyboard('{enter}'); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); expect(closeHandler).toHaveBeenNthCalledWith(1, { reason: 'selection', index: 0, @@ -416,9 +455,10 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }); it('should trigger the click handler on MenuItem when selecting it with space', async () => { - const { getAllByRole, closeHandler, menuItemHandler } = renderMenu(); + const { queryByRole, getByRole, closeHandler, menuItemHandler } = + renderMenu(); - const { menu, menuButton } = getElements({ getAllByRole }); + const menuButton = getByRole('button'); // Open menu await userEvent.tab(); @@ -427,7 +467,7 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { // Action the item await userEvent.keyboard(' '); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); expect(closeHandler).toHaveBeenNthCalledWith(1, { reason: 'selection', index: 0, @@ -438,9 +478,10 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }); it('should trigger the click handler on MenuItemLink when selecting it with enter', async () => { - const { getAllByRole, closeHandler, menuItemHandler } = renderMenu(); + const { queryByRole, getByRole, closeHandler, menuItemHandler } = + renderMenu(); - const { menu, menuButton } = getElements({ getAllByRole }); + const menuButton = getByRole('button'); // Open menu await userEvent.tab(); @@ -448,7 +489,7 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { await userEvent.keyboard('{arrowdown}'); await userEvent.keyboard('{enter}'); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); expect(closeHandler).toHaveBeenNthCalledWith(1, { reason: 'selection', index: 1, @@ -459,9 +500,10 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }); it('should trigger the click handler on MenuItemLink when selecting it with space', async () => { - const { getAllByRole, closeHandler, menuItemHandler } = renderMenu(); + const { queryByRole, getByRole, closeHandler, menuItemHandler } = + renderMenu(); - const { menu, menuButton } = getElements({ getAllByRole }); + const menuButton = getByRole('button'); // Open menu await userEvent.tab(); @@ -469,7 +511,7 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { await userEvent.keyboard('{arrowdown}'); await userEvent.keyboard(' '); - expect(menu).not.toBeVisible(); + expect(queryByRole('menu')).not.toBeInTheDocument(); expect(closeHandler).toHaveBeenNthCalledWith(1, { reason: 'selection', index: 1, @@ -480,9 +522,8 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }); it('should toggle the state on MenuItemCheckbox when selecting it with enter', async () => { - const { getAllByRole, closeHandler, menuItemHandler } = renderMenu(); - - const { menu } = getElements({ getAllByRole }); + const { getByRole, getAllByRole, closeHandler, menuItemHandler } = + renderMenu(); // Open menu await userEvent.tab(); @@ -490,8 +531,9 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { await userEvent.keyboard('{arrowdown}'); await userEvent.keyboard('{arrowdown}'); - const thirdDown = getElements({ getAllByRole }); - const thirdMenuItem = thirdDown.menuItems[2]; + const menu = getByRole('menu'); + const menuItems = getMenuItems({ getAllByRole }); + const thirdMenuItem = menuItems[2]; expect(thirdMenuItem.getAttribute('aria-checked')).toBe('false'); @@ -506,9 +548,8 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }); it('should toggle the state on MenuItemCheckbox when selecting it with space', async () => { - const { getAllByRole, closeHandler, menuItemHandler } = renderMenu(); - - const { menu } = getElements({ getAllByRole }); + const { getByRole, getAllByRole, closeHandler, menuItemHandler } = + renderMenu(); // Open menu await userEvent.tab(); @@ -516,8 +557,10 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { await userEvent.keyboard('{arrowdown}'); await userEvent.keyboard('{arrowdown}'); - const thirdDown = getElements({ getAllByRole }); - const thirdMenuItem = thirdDown.menuItems[2]; + const menu = getByRole('menu'); + const menuItems = getMenuItems({ getAllByRole }); + + const thirdMenuItem = menuItems[2]; expect(thirdMenuItem.getAttribute('aria-checked')).toBe('false'); @@ -534,9 +577,9 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { describe('Open/Close handlers', () => { it('should not fire the open handler when its changed', async () => { - const { getAllByRole, openHandler, rerender } = renderMenu(); + const { getByRole, openHandler, rerender } = renderMenu(); - const { menuButton } = getElements({ getAllByRole }); + const menuButton = getByRole('button'); const newOpen = jest.fn(); await userEvent.click(menuButton); @@ -546,9 +589,9 @@ export const menuTestSuite = ({ name, Component }: MenuTestSuiteParams) => { }); it('should not fire the close handler when its changed', async () => { - const { getAllByRole, closeHandler, rerender } = renderMenu(); + const { getByRole, closeHandler, rerender } = renderMenu(); - const { menuButton } = getElements({ getAllByRole }); + const menuButton = getByRole('button'); const newClose = jest.fn(); await userEvent.click(menuButton); diff --git a/packages/braid-design-system/src/lib/components/OverflowMenu/OverflowMenu.css.ts b/packages/braid-design-system/src/lib/components/OverflowMenu/OverflowMenu.css.ts index 6ce01df47a5..ccc9423d9dd 100644 --- a/packages/braid-design-system/src/lib/components/OverflowMenu/OverflowMenu.css.ts +++ b/packages/braid-design-system/src/lib/components/OverflowMenu/OverflowMenu.css.ts @@ -1,5 +1,7 @@ import { style } from '@vanilla-extract/css'; -export const triggerOffset = style({ +export const wrapperPositioning = style({ margin: '-1px -6px', + width: 'fit-content', + justifySelf: 'flex-end', }); diff --git a/packages/braid-design-system/src/lib/components/OverflowMenu/OverflowMenu.tsx b/packages/braid-design-system/src/lib/components/OverflowMenu/OverflowMenu.tsx index fda1a2fba7d..982468b9831 100644 --- a/packages/braid-design-system/src/lib/components/OverflowMenu/OverflowMenu.tsx +++ b/packages/braid-design-system/src/lib/components/OverflowMenu/OverflowMenu.tsx @@ -20,13 +20,9 @@ export const OverflowMenu = ({ id, ...menuProps }: OverflowMenuProps) => ( - ( - + + ( - - )} - align="right" - offsetSpace="small" - {...menuProps} - > - {children} - + )} + align="right" + offsetSpace="small" + {...menuProps} + > + {children} + + ); diff --git a/packages/braid-design-system/src/lib/stories/all.stories.tsx b/packages/braid-design-system/src/lib/stories/all.stories.tsx index c42bc523fb6..31b0d6b52c2 100644 --- a/packages/braid-design-system/src/lib/stories/all.stories.tsx +++ b/packages/braid-design-system/src/lib/stories/all.stories.tsx @@ -151,8 +151,9 @@ Object.keys(allStories)