fix(ui): Compose the Mosaic Menu popup out of the shared ScrollArea (#9596)

Author: alexcarpenter

.changeset/mosaic-menu-item-height.md

@@ -0,0 +1,2 @@
+---
+---

packages/swingset/src/stories/menu.component.mdx

@@ -174,6 +174,21 @@ has to be given somewhere else to land.
 </Menu.Root>;
 ```
 
+### 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.
+
+<Story
+  name='Overflowing'
+  storyModule={MenuStories}
+/>
+
 ### 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. |

packages/swingset/src/stories/menu.component.stories.tsx

@@ -81,3 +81,97 @@ export function Accounts() {
     </Menu.Root>
   );
 }
+
+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 (
+    <Menu.Root>
+      <Menu.Trigger>Switch workspace</Menu.Trigger>
+      <Menu.Popup>
+        {workspaces.map(workspace => (
+          <Menu.Item
+            key={workspace}
+            label={workspace}
+          >
+            <Menu.Media>
+              <Avatar.Root
+                shape='square'
+                size='fit'
+              >
+                <Avatar.Fallback>{workspace[0]}</Avatar.Fallback>
+              </Avatar.Root>
+            </Menu.Media>
+            <Menu.Label>{workspace}</Menu.Label>
+          </Menu.Item>
+        ))}
+      </Menu.Popup>
+    </Menu.Root>
+  );
+}

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,
   },
 });

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(
     <Menu.Root>
@@ -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();

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<HTMLDivElement, MenuPopupProps>(functi
       >
         <Primitive.Popup
           ref={ref}
-          {...mergeStyleProps(themeProps('menu-popup'), stylex.props(reset.base, slots.popup.base), className, style)}
+          {...mergeStyleProps(
+            themeProps('menu-popup'),
+            stylex.props(reset.base, scrollAreaRoot, slots.popup.base),
+            className,
+            style,
+          )}
           {...rest}
         >
-          {children}
+          <div
+            {...mergeStyleProps(
+              themeProps('menu-viewport'),
+              stylex.props(reset.base, ...scrollAreaViewport(), slots.viewport.base),
+            )}
+          >
+            {children}
+          </div>
         </Primitive.Popup>
       </Primitive.Positioner>
     </Primitive.Portal>