From fb5f00f57eddced8aa38d14c6f313b490a60a850 Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Thu, 27 Aug 2026 13:53:52 -0400 Subject: [PATCH 1/3] fix(ui): Correct Mosaic menu item height and drop no-op overflow-x MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Menu items were `space['7']` (1.75rem) tall; bump to `space['8']` (2rem). `overflowX: 'visible'` on the popup was inert — CSS computes `visible` to `auto` when the other axis is `auto`, so it only read as if it did something. --- .changeset/mosaic-menu-item-height.md | 2 ++ packages/ui/src/mosaic/components/menu/menu.styles.ts | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 .changeset/mosaic-menu-item-height.md 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/ui/src/mosaic/components/menu/menu.styles.ts b/packages/ui/src/mosaic/components/menu/menu.styles.ts index 5f272d41953..6c3dc89abdb 100644 --- a/packages/ui/src/mosaic/components/menu/menu.styles.ts +++ b/packages/ui/src/mosaic/components/menu/menu.styles.ts @@ -48,7 +48,6 @@ 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', }, }); @@ -83,7 +82,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']})`, From 79e6fe26a6219a8e86a7f7725efb2eff5a6be31d Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Thu, 27 Aug 2026 14:37:24 -0400 Subject: [PATCH 2/3] fix(ui): Compose the Mosaic Menu popup out of the shared ScrollArea MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The popup already scrolled — `overflowY: auto` under a `--cl-available-height` cap — but with a bare scrollbar instead of the ScrollArea treatment every other overflowing Mosaic surface gets. Split `Menu.Popup` the way the Dialog panel is split: the popup keeps the chrome and the height cap, and a new `menu-viewport` inside it carries `scrollAreaViewport()`. They cannot be one element, because the fade is a `mask-image` and a mask clips the element's whole rendering — on the popup it would eat the background and the drop shadow along with the overflowing rows. The inline padding moves onto the viewport so `Menu.Separator`'s negative-margin bleed still reaches a padding-box edge rather than being clipped. Adds an `Overflowing` swingset story so the scroll styling is visible. --- .../swingset/src/stories/menu.component.mdx | 16 +++++ .../src/stories/menu.component.stories.tsx | 70 +++++++++++++++++++ .../src/mosaic/components/menu/menu.styles.ts | 23 ++++-- .../src/mosaic/components/menu/menu.test.tsx | 25 +++++++ .../ui/src/mosaic/components/menu/menu.tsx | 17 ++++- 5 files changed, 143 insertions(+), 8 deletions(-) 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..f93fe63d986 100644 --- a/packages/swingset/src/stories/menu.component.stories.tsx +++ b/packages/swingset/src/stories/menu.component.stories.tsx @@ -81,3 +81,73 @@ 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', +]; + +/** + * 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 6c3dc89abdb..7478190abdd 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,7 +44,22 @@ export const popup = stylex.create({ // reaches the width it has to truncate at. maxWidth: 'min(18rem, calc(100vw - 2rem))', minWidth: '12.5rem', - 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', }, }); @@ -129,7 +140,7 @@ 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'], diff --git a/packages/ui/src/mosaic/components/menu/menu.test.tsx b/packages/ui/src/mosaic/components/menu/menu.test.tsx index b6f28631e5d..42a493b6c82 100644 --- a/packages/ui/src/mosaic/components/menu/menu.test.tsx +++ b/packages/ui/src/mosaic/components/menu/menu.test.tsx @@ -1,10 +1,18 @@ +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(' ') ?? []; +const viewportOnlyClasses = scrollClasses.filter(name => !rootClasses.includes(name)); + function renderMenu(props?: { onSignOut?: () => void }) { return render( @@ -79,6 +87,23 @@ 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('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} +
From 2ba5a29bac33c5e319cf62be9ac581d4f9c6065e Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Thu, 27 Aug 2026 15:17:41 -0400 Subject: [PATCH 3/3] fix(ui): Stop Mosaic menu rows squashing instead of scrolling `Menu.Item` and `Menu.Separator` sat in a height-capped flex column at flex's default `shrink: 1`, so a full menu compressed its rows to fit the cap rather than overflowing it. Rows rendered at 26px instead of the 32px they ask for, and the popup never scrolled at all until the rows hit their min-content height. Pin both to `flexShrink: 0`. This is what made the row-height fix in this branch observable, and what makes the ScrollArea treatment reachable. Widens the `Overflowing` swingset fixture to 60 rows so it overflows on tall displays too; at 36 it only just cleared the cap on a 1323px viewport. --- .../src/stories/menu.component.stories.tsx | 24 +++++++++++++++++++ .../src/mosaic/components/menu/menu.styles.ts | 4 ++++ .../src/mosaic/components/menu/menu.test.tsx | 18 ++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/packages/swingset/src/stories/menu.component.stories.tsx b/packages/swingset/src/stories/menu.component.stories.tsx index f93fe63d986..a16db64af27 100644 --- a/packages/swingset/src/stories/menu.component.stories.tsx +++ b/packages/swingset/src/stories/menu.component.stories.tsx @@ -119,6 +119,30 @@ const workspaces = [ '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', ]; /** diff --git a/packages/ui/src/mosaic/components/menu/menu.styles.ts b/packages/ui/src/mosaic/components/menu/menu.styles.ts index 7478190abdd..a81e051855d 100644 --- a/packages/ui/src/mosaic/components/menu/menu.styles.ts +++ b/packages/ui/src/mosaic/components/menu/menu.styles.ts @@ -80,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'], @@ -145,5 +148,6 @@ export const separator = stylex.create({ 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 42a493b6c82..6b6e62137d4 100644 --- a/packages/ui/src/mosaic/components/menu/menu.test.tsx +++ b/packages/ui/src/mosaic/components/menu/menu.test.tsx @@ -11,6 +11,12 @@ 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 }) { @@ -104,6 +110,18 @@ describe('Mosaic Menu', () => { 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();