From 8ec5c0770a6778b1a00be47938da4a04b60d64fe Mon Sep 17 00:00:00 2001 From: Minwook Shin <163576506+minwookshin@users.noreply.github.com> Date: Thu, 1 Oct 2026 02:31:39 -0400 Subject: [PATCH 1/2] fix(menu): keep submenus beside scrolled parent menus Use horizontal flipping for default submenu positioning while retaining explicit fallback placements and custom Popper options. Cover scrolled parents, viewport edges, RTL and overrides in component tests. Fixes #4180 --- cypress/component/SubmenuPlacement.spec.tsx | 125 ++++++++++++++++++ modules/react/menu/lib/Submenu.tsx | 27 +++- modules/react/menu/stories/Menu.stories.ts | 4 + .../stories/examples/ScrollableSubmenu.tsx | 57 ++++++++ 4 files changed, 211 insertions(+), 2 deletions(-) create mode 100644 cypress/component/SubmenuPlacement.spec.tsx create mode 100644 modules/react/menu/stories/examples/ScrollableSubmenu.tsx diff --git a/cypress/component/SubmenuPlacement.spec.tsx b/cypress/component/SubmenuPlacement.spec.tsx new file mode 100644 index 0000000000..442f149e72 --- /dev/null +++ b/cypress/component/SubmenuPlacement.spec.tsx @@ -0,0 +1,125 @@ +import {ScrollableSubmenu} from '../../modules/react/menu/stories/examples/ScrollableSubmenu'; + +describe('Submenu placement', () => { + (['ltr', 'rtl'] as const).forEach(dir => { + [false, true].forEach(atEnd => { + const side = (dir === 'ltr') !== atEnd ? 'right' : 'left'; + + context(`given a scrollable ${dir} menu at the ${atEnd ? 'end' : 'start'} edge`, () => { + beforeEach(() => { + cy.mount(); + cy.findByRole('button', {name: 'Open Menu'}).click(); + cy.findByRole('menuitem', {name: 'More Items'}).realHover(); + cy.findByTestId('submenu').should('be.visible'); + }); + + it('should pass axe checks', () => { + cy.checkA11y(); + }); + + it(`should open beside the parent on the ${side}`, () => { + cy.findByTestId('submenu') + .closest('[data-popper-placement]') + .should('have.attr', 'data-popper-placement', `${side}-start`); + }); + + context('when the parent menu is scrolled', () => { + beforeEach(() => { + cy.findByTestId('scrollable-menu') + .find('[data-part="list-box-container"]') + .scrollTo(0, 280); + cy.findByTestId('submenu') + .closest('[data-popper-placement]') + .should('have.attr', 'data-popper-reference-hidden'); + }); + + it('should remain beside the parent without covering it', () => { + cy.findByTestId('scrollable-menu').then($parent => { + const parent = $parent[0].getBoundingClientRect(); + cy.findByTestId('submenu').should($submenu => { + const submenu = $submenu[0].getBoundingClientRect(); + if (side === 'right') { + expect(submenu.left).to.be.at.least(parent.right); + } else { + expect(submenu.right).to.be.at.most(parent.left); + } + }); + }); + }); + }); + }); + }); + }); + + context('given an explicit submenu placement', () => { + beforeEach(() => { + cy.mount( + + ); + cy.findByRole('button', {name: 'Open Menu'}).click(); + cy.findByRole('menuitem', {name: 'More Items'}).realHover(); + cy.findByTestId('submenu').should('be.visible'); + }); + + it('should preserve the custom placement', () => { + cy.findByTestId('submenu') + .closest('[data-popper-placement]') + .should('have.attr', 'data-popper-placement', 'bottom-start'); + }); + + it('should pass axe checks', () => { + cy.checkA11y(); + }); + }); + + context('given an empty fallback list at the right edge', () => { + beforeEach(() => { + cy.mount(); + cy.findByRole('button', {name: 'Open Menu'}).click(); + cy.findByRole('menuitem', {name: 'More Items'}).realHover(); + cy.findByTestId('submenu').should('be.visible'); + }); + + it('should still allow the opposite placement', () => { + cy.findByTestId('submenu') + .closest('[data-popper-placement]') + .should('have.attr', 'data-popper-placement', 'left-start'); + }); + + it('should pass axe checks', () => { + cy.checkA11y(); + }); + }); + + context('given custom Popper options', () => { + beforeEach(() => { + cy.mount( + + ); + cy.findByRole('button', {name: 'Open Menu'}).click(); + cy.findByRole('menuitem', {name: 'More Items'}).realHover(); + cy.findByTestId('submenu').should('be.visible'); + }); + + it('should preserve the custom offset', () => { + cy.findByRole('menuitem', {name: 'More Items'}).then($target => { + const target = $target[0].getBoundingClientRect(); + cy.findByTestId('submenu') + .closest('[data-popper-placement]') + .should($popper => { + expect($popper[0].getBoundingClientRect().left).to.be.closeTo(target.right + 24, 1); + }); + }); + }); + + it('should pass axe checks', () => { + cy.checkA11y(); + }); + }); +}); diff --git a/modules/react/menu/lib/Submenu.tsx b/modules/react/menu/lib/Submenu.tsx index 4360ffe06a..2f38bb76c9 100644 --- a/modules/react/menu/lib/Submenu.tsx +++ b/modules/react/menu/lib/Submenu.tsx @@ -40,9 +40,32 @@ export interface SubmenuProps export const SubmenuPopper = createSubcomponent('div')({ modelHook: useMenuModel, elemPropsHook: useMenuPopper, -})>(({children, ...elemProps}) => { +})>(({children, fallbackPlacements, popperOptions, ...elemProps}) => { + const submenuPopperOptions = React.useMemo( + () => ({ + ...popperOptions, + modifiers: [ + // Keep the default submenu beside its parent when scrolling clips the target vertically. + // Explicit fallback placements retain the caller's existing positioning behavior. + ...(fallbackPlacements === undefined + ? [ + {name: 'fallbackModifier', enabled: false}, + {name: 'flip', options: {altAxis: false, flipVariations: false}}, + ] + : []), + ...(popperOptions?.modifiers ?? defaultMenuPopperOptions.modifiers), + ], + }), + [fallbackPlacements, popperOptions] + ); + return ( - + {children} ); diff --git a/modules/react/menu/stories/Menu.stories.ts b/modules/react/menu/stories/Menu.stories.ts index 4d6c5ab3e1..d11df6eae1 100644 --- a/modules/react/menu/stories/Menu.stories.ts +++ b/modules/react/menu/stories/Menu.stories.ts @@ -10,6 +10,7 @@ import {Icons as IconsExample} from './examples/Icons'; import {Nested as NestedExample} from './examples/Nested'; import {NestedDynamic as NestedDynamicExample} from './examples/NestedDynamic'; import {NestedSiblings as NestedSiblingsExample} from './examples/NestedSiblings'; +import {ScrollableSubmenu as ScrollableSubmenuExample} from './examples/ScrollableSubmenu'; import {SelectableMenu as SelectableMenuExample} from './examples/SelectableMenu'; export default { @@ -49,3 +50,6 @@ export const NestedDynamic: Story = { export const NestedSiblings: Story = { render: NestedSiblingsExample, }; +export const ScrollableSubmenu: StoryObj = { + render: ScrollableSubmenuExample, +}; diff --git a/modules/react/menu/stories/examples/ScrollableSubmenu.tsx b/modules/react/menu/stories/examples/ScrollableSubmenu.tsx new file mode 100644 index 0000000000..29086093d1 --- /dev/null +++ b/modules/react/menu/stories/examples/ScrollableSubmenu.tsx @@ -0,0 +1,57 @@ +import {CanvasProvider} from '@workday/canvas-kit-react/common'; +import {Menu, menuCardStencil} from '@workday/canvas-kit-react/menu'; +import {PopperProps} from '@workday/canvas-kit-react/popup'; +import {createStencil, createStyles, handleCsProp, px2rem} from '@workday/canvas-kit-styling'; + +const containerStencil = createStencil({ + base: {display: 'flex'}, + modifiers: {atEnd: {true: {justifyContent: 'flex-end'}}}, +}); + +// A fixed height makes the parent list scroll independently of the page. +const cardStyles = createStyles({[menuCardStencil.vars.maxHeight]: px2rem(280)}); + +export const ScrollableSubmenu = ({ + dir = 'ltr', + atEnd = false, + submenuPopperProps, +}: { + dir?: 'ltr' | 'rtl'; + atEnd?: boolean; + submenuPopperProps?: Pick; +}) => ( + +
+ + Open Menu + + + + First Item + Second Item + + More Items + + + + {Array.from({length: 6}, (_, index) => ( + + Additional Menu Item {index + 1} + + ))} + + + + + {Array.from({length: 15}, (_, index) => ( + + Item {index + 1} + + ))} + + + + +
+ +); From 4d1e0de8214f1a6efbafa760ff449084551b96c2 Mon Sep 17 00:00:00 2001 From: Minwook Shin <163576506+minwookshin@users.noreply.github.com> Date: Fri, 2 Oct 2026 00:35:32 -0400 Subject: [PATCH 2/2] fix(menu): account for parent scrollbar gutters in submenu placement --- cypress/component/SubmenuPlacement.spec.tsx | 19 +++++ modules/react/menu/lib/Submenu.tsx | 86 ++++++++++++++------- 2 files changed, 75 insertions(+), 30 deletions(-) diff --git a/cypress/component/SubmenuPlacement.spec.tsx b/cypress/component/SubmenuPlacement.spec.tsx index 442f149e72..96e8852dac 100644 --- a/cypress/component/SubmenuPlacement.spec.tsx +++ b/cypress/component/SubmenuPlacement.spec.tsx @@ -8,11 +8,30 @@ describe('Submenu placement', () => { context(`given a scrollable ${dir} menu at the ${atEnd ? 'end' : 'start'} edge`, () => { beforeEach(() => { cy.mount(); + cy.document().then(document => { + const style = document.createElement('style'); + style.id = 'submenu-scrollbar-test'; + style.textContent = ` + [data-testid="scrollable-menu"] [data-part="list-box-container"] { + scrollbar-gutter: stable both-edges; + } + [data-testid="scrollable-menu"] [data-part="list-box-container"]::-webkit-scrollbar { + width: 16px; + } + `; + document.head.appendChild(style); + }); cy.findByRole('button', {name: 'Open Menu'}).click(); cy.findByRole('menuitem', {name: 'More Items'}).realHover(); cy.findByTestId('submenu').should('be.visible'); }); + afterEach(() => { + cy.document().then(document => + document.getElementById('submenu-scrollbar-test')?.remove() + ); + }); + it('should pass axe checks', () => { cy.checkA11y(); }); diff --git a/modules/react/menu/lib/Submenu.tsx b/modules/react/menu/lib/Submenu.tsx index 2f38bb76c9..519657867f 100644 --- a/modules/react/menu/lib/Submenu.tsx +++ b/modules/react/menu/lib/Submenu.tsx @@ -14,7 +14,7 @@ import { subModelHook, useForkRef, } from '@workday/canvas-kit-react/common'; -import {Popper} from '@workday/canvas-kit-react/popup'; +import {Placement, Popper} from '@workday/canvas-kit-react/popup'; import {chevronRightSmallIcon} from '@workday/canvas-system-icons-web'; import {MenuCard} from './MenuCard'; @@ -40,36 +40,62 @@ export interface SubmenuProps export const SubmenuPopper = createSubcomponent('div')({ modelHook: useMenuModel, elemPropsHook: useMenuPopper, -})>(({children, fallbackPlacements, popperOptions, ...elemProps}) => { - const submenuPopperOptions = React.useMemo( - () => ({ - ...popperOptions, - modifiers: [ - // Keep the default submenu beside its parent when scrolling clips the target vertically. - // Explicit fallback placements retain the caller's existing positioning behavior. - ...(fallbackPlacements === undefined - ? [ - {name: 'fallbackModifier', enabled: false}, - {name: 'flip', options: {altAxis: false, flipVariations: false}}, - ] - : []), - ...(popperOptions?.modifiers ?? defaultMenuPopperOptions.modifiers), - ], - }), - [fallbackPlacements, popperOptions] - ); +})>( + ({children, fallbackPlacements, popperOptions, ...elemProps}, Element, model) => { + const parentContainerRef = model.UNSTABLE_parentModel?.state.containerRef; + const targetRef = model.state.targetRef; + const submenuPopperOptions = React.useMemo( + () => ({ + ...popperOptions, + modifiers: [ + // Keep the default submenu beside its parent when scrolling clips the target vertically. + // Explicit fallback placements retain the caller's existing positioning behavior. + ...(fallbackPlacements === undefined + ? [ + {name: 'fallbackModifier', enabled: false}, + {name: 'flip', options: {altAxis: false, flipVariations: false}}, + ] + : []), + ...(popperOptions?.modifiers ?? [ + { + name: 'offset', + options: { + offset: ({placement}: {placement: Placement}) => { + const [skidding, distance] = + defaultMenuPopperOptions.modifiers[0].options.offset(); + const parent = parentContainerRef?.current?.getBoundingClientRect(); + const target = targetRef.current?.getBoundingClientRect(); + // Scrollbar gutters narrow the target without narrowing the parent menu. + const gutter = + parent && target + ? placement.startsWith('right') + ? parent.right - target.right + : placement.startsWith('left') + ? target.left - parent.left + : 0 + : 0; + return [skidding, distance + Math.max(0, gutter)]; + }, + }, + }, + ]), + ], + }), + [fallbackPlacements, popperOptions, parentContainerRef, targetRef] + ); - return ( - - {children} - - ); -}); + return ( + + {children} + + ); + } +); const useIntentTimer = (fn: Function, waitMs: number = 0): {start(): void; clear(): void} => { const timer = React.useRef() as React.MutableRefObject;