diff --git a/.changeset/mosaic-menu-item-height.md b/.changeset/mosaic-menu-item-height.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-menu-item-height.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/swingset/src/stories/menu.component.mdx b/packages/swingset/src/stories/menu.component.mdx index 14edfa8b7fd..0a1e3081e37 100644 --- a/packages/swingset/src/stories/menu.component.mdx +++ b/packages/swingset/src/stories/menu.component.mdx @@ -174,6 +174,21 @@ has to be given somewhere else to land. ; ``` +### Scrolling + +A menu with more items than fit gets the shared ScrollArea treatment rather than a bare scrollbar: +the rows fade out at whichever edge still has content past it, and the thumb lifts as the pointer +enters. Nothing to opt into — `Menu.Popup` composes it. + +The popup itself only supplies the chrome and the height cap; the rows scroll in a `menu-viewport` +inside it. They cannot be one element, because the fade is a `mask-image` and a mask on the popup +would clip its own background and drop shadow along with the overflowing rows. + + + ### Controlled ```tsx @@ -194,6 +209,7 @@ const [open, setOpen] = useState(false); | `Menu.Root` | — | State provider; owns open/close, placement, and keyboard navigation. | | `Menu.Trigger` | `menu-trigger` | Opens the menu. Defaults to a square ghost `Button` with an ellipsis. | | `Menu.Popup` | `menu-positioner` / `menu-popup` | Portals, positions, and renders the popup surface. | +| ↳ viewport | `menu-viewport` | The scrolling box inside the popup. Carries the ScrollArea fade and scrollbar. | | `Menu.Item` | `menu-item` | A single action whose content is composed through children. | | `Menu.Media` | `menu-media` | Square leading column that centers an item's icon, image, or avatar. | | `Menu.Label` | `menu-label` | The item's text. Fills the row between media and trailing marks, and truncates. | diff --git a/packages/swingset/src/stories/menu.component.stories.tsx b/packages/swingset/src/stories/menu.component.stories.tsx index 172bc72dec9..a16db64af27 100644 --- a/packages/swingset/src/stories/menu.component.stories.tsx +++ b/packages/swingset/src/stories/menu.component.stories.tsx @@ -81,3 +81,97 @@ export function Accounts() { ); } + +const workspaces = [ + 'Ashgrove', + 'Bramblewood', + 'Cedar Hollow', + 'Dunmore Flats', + 'Elderbrook', + 'Fernwick', + 'Glasswater', + 'Hallowfield', + 'Ironvale', + 'Juniper Reach', + 'Kestrel Point', + 'Larkspur Mill', + 'Marrowden', + 'Northgate', + 'Oakenshire', + 'Pinecrest', + 'Quarryfield', + 'Redhollow', + 'Stonebridge', + 'Thornbury', + 'Underhill', + 'Vellamere', + 'Westmarch', + 'Yarrowdale', + 'Alderwick', + 'Blackthorn', + 'Coldspring', + 'Dovecote', + 'Eastfen', + 'Foxglove', + 'Greyholt', + 'Harrowgate', + 'Inglewood', + 'Jessamine', + 'Kirkstall', + 'Lindenfell', + 'Mossbank', + 'Nettlefold', + 'Oxbow Landing', + 'Peartree Row', + 'Quillhaven', + 'Ravensmoor', + 'Saltmarsh', + 'Tanglewood', + 'Ullswater', + 'Vinequarter', + 'Wrenfield', + 'Yewbarrow', + 'Amberlyn', + 'Brookhaven', + 'Chalkhill', + 'Dryden Cross', + 'Ellerby', + 'Fallowmere', + 'Gorsecliff', + 'Hazelmoor', + 'Ivybridge', + 'Jackdaw Lane', + 'Kilnwood', + 'Longmeadow', +]; + +/** + * More rows than the popup can show. The popup caps at the available height and the rows scroll + * inside it, so this is the story where the ScrollArea treatment is visible: the fade at whichever + * edge still has content past it, and the thumb that lifts as the pointer enters. + */ +export function Overflowing() { + return ( + + Switch workspace + + {workspaces.map(workspace => ( + + + + {workspace[0]} + + + {workspace} + + ))} + + + ); +} diff --git a/packages/ui/src/mosaic/components/menu/menu.styles.ts b/packages/ui/src/mosaic/components/menu/menu.styles.ts index 5f272d41953..a81e051855d 100644 --- a/packages/ui/src/mosaic/components/menu/menu.styles.ts +++ b/packages/ui/src/mosaic/components/menu/menu.styles.ts @@ -13,17 +13,13 @@ export const positioner = stylex.create({ export const popup = stylex.create({ base: { - padding: space['0.5'], borderRadius: radiusVars['--cl-radius-lg'], - gap: space['0.5'], outline: 'none', backgroundColor: colorVars['--cl-color-card'], boxShadow: `0 12px 12px -7px light-dark(oklch(0.2046 0 0 / 12%), transparent), 0 24px 24px -10px light-dark(oklch(0.2046 0 0 / 4%), transparent), 0 0 0 1px light-dark(oklch(0.2046 0 0 / 4%), oklch(1 0 0 / 10%))`, color: colorVars['--cl-color-card-foreground'], - display: 'flex', - flexDirection: 'column', opacity: { default: 1, ':is([data-ending-style])': 0, @@ -48,8 +44,22 @@ export const popup = stylex.create({ // reaches the width it has to truncate at. maxWidth: 'min(18rem, calc(100vw - 2rem))', minWidth: '12.5rem', - overflowX: 'visible', - overflowY: 'auto', + }, +}); + +// The popup supplies the chrome and the height cap; this scrolls inside it. They cannot be +// one element: `scrollAreaViewport()` carries a `mask-image` for the fade, and a mask clips +// the element's whole rendering — so on the popup it would eat the background and the drop +// shadow along with the overflowing rows. Same split the Dialog panel makes. +export const viewport = stylex.create({ + base: { + // The inset belongs to the scrolling box, not the popup: `Menu.Separator` bleeds through it + // with a negative margin, and from inside a viewport that clips its inline axis a bleed past + // the popup's own padding would be cut off instead. + padding: space['0.5'], + gap: space['0.5'], + display: 'flex', + flexDirection: 'column', }, }); @@ -70,6 +80,9 @@ export const item = stylex.create({ }, cursor: { default: 'pointer', ':is([data-disabled])': 'not-allowed' }, display: 'flex', + // The viewport is a height-capped flex column: without this the rows squash to fit the cap + // instead of overflowing it, and the menu silently loses both its row height and its scroll. + flexShrink: 0, fontFamily: fontFamilyVars['--cl-font-family-sans'], fontSize: typeScaleVars['--cl-text-sm-size'], fontWeight: fontWeightVars['--cl-font-medium'], @@ -83,7 +96,7 @@ export const item = stylex.create({ '@media (prefers-reduced-motion: reduce)': '0.01ms', }, transitionProperty: 'background-color', - height: space['7'], + height: space['8'], width: '100%', '::before': { insetBlock: `calc(-1 * ${space['0.5']})`, @@ -130,10 +143,11 @@ export const label = stylex.create({ export const separator = stylex.create({ base: { - // Full-bleed across the popup: cancel the popup's inline padding. + // Full-bleed across the popup: cancel the viewport's inline padding. marginBlock: space['0.5'], marginInline: `calc(-1 * ${space['0.5']})`, backgroundColor: colorVars['--cl-color-border'], blockSize: '1px', + flexShrink: 0, }, }); diff --git a/packages/ui/src/mosaic/components/menu/menu.test.tsx b/packages/ui/src/mosaic/components/menu/menu.test.tsx index b6f28631e5d..6b6e62137d4 100644 --- a/packages/ui/src/mosaic/components/menu/menu.test.tsx +++ b/packages/ui/src/mosaic/components/menu/menu.test.tsx @@ -1,10 +1,24 @@ +import * as stylex from '@stylexjs/stylex'; import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { describe, expect, it, vi } from 'vitest'; +import { scrollAreaRoot, scrollAreaViewport } from '../scroll-area'; import { Menu } from './menu'; +const scrollClasses = stylex.props(...scrollAreaViewport()).className?.split(' ') ?? []; +// The popup shares the root's atoms (both are `min-height: 0` flex columns), so only what the +// viewport adds on top of them — the mask, the overflow, the scrollbar — distinguishes the two. +const rootClasses = stylex.props(scrollAreaRoot).className?.split(' ') ?? []; +// StyleX atoms are content-hashed, so an identical declaration here resolves to the same atom the +// styles module emits. jsdom has no stylesheet, so the class is the only thing there is to assert on. +// The debug class alongside it is derived from the source filename, so only the atoms can match. +const noShrinkClass = (stylex.props(stylex.create({ base: { flexShrink: 0 } }).base).className ?? '') + .split(' ') + .filter(name => /^x[a-z0-9]+$/.test(name)); +const viewportOnlyClasses = scrollClasses.filter(name => !rootClasses.includes(name)); + function renderMenu(props?: { onSignOut?: () => void }) { return render( @@ -79,6 +93,35 @@ describe('Mosaic Menu', () => { expect(screen.getByTestId('add-icon')).toBeInTheDocument(); }); + it('scrolls the items in a viewport inside the popup, not on the popup itself', async () => { + const user = userEvent.setup(); + renderMenu(); + + await user.click(screen.getByRole('button')); + + const popup = screen.getByRole('menu').querySelector('.cl-menu-popup'); + const viewport = popup?.querySelector('.cl-menu-viewport'); + expect(viewport).toBeInTheDocument(); + expect(viewport).toHaveClass(...scrollClasses); + // The chrome must NOT carry them: `scrollAreaViewport()` masks the element it lands on, and a + // mask on the popup would clip its own background and drop shadow. + expect(viewportOnlyClasses).not.toHaveLength(0); + expect(viewportOnlyClasses.filter(name => popup?.classList.contains(name))).toEqual([]); + expect(screen.getByRole('menuitem', { name: 'Sign out' }).closest('.cl-menu-viewport')).toBe(viewport); + }); + + it('holds row height inside the capped viewport rather than letting flex squash it', async () => { + const user = userEvent.setup(); + renderMenu(); + + await user.click(screen.getByRole('button')); + + // The viewport is a height-capped flex column. With flex's default shrink the rows compress to + // fit the cap instead of overflowing it, so the menu loses both its row height and its scroll. + expect(screen.getByRole('menuitem', { name: 'Sign out' })).toHaveClass(...noShrinkClass); + expect(screen.getByRole('separator')).toHaveClass(...noShrinkClass); + }); + it('calls an item handler and closes the menu on click', async () => { const user = userEvent.setup(); const onSignOut = vi.fn(); diff --git a/packages/ui/src/mosaic/components/menu/menu.tsx b/packages/ui/src/mosaic/components/menu/menu.tsx index 5e44286a319..b54b5af77d0 100644 --- a/packages/ui/src/mosaic/components/menu/menu.tsx +++ b/packages/ui/src/mosaic/components/menu/menu.tsx @@ -17,6 +17,7 @@ import { reset } from '../../utils/reset.styles'; import { truncationStyles } from '../../utils/typography.styles'; import { Button } from '../button'; import { Icon } from '../icon'; +import { scrollAreaRoot, scrollAreaViewport } from '../scroll-area'; import * as slots from './menu.styles'; export type { MenuProps, MenuSeparatorProps }; @@ -74,10 +75,22 @@ export const MenuPopup = React.forwardRef(functi > - {children} +
+ {children} +