fix(ui): Compose the Mosaic Menu popup out of the shared ScrollArea - #9596
Conversation
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 detectedLatest commit: 2ba5a29 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: 5 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 8 reviews per hour. 📝 WalkthroughWalkthroughThe Mosaic menu now renders its content inside a dedicated scrolling viewport. Popup styles remain separate from viewport spacing and layout styles. Menu items and separators prevent flex shrinking. Menu item height increases from Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR updates the Mosaic menu to use the shared scrolling treatment and corrects row sizing and overflow behavior. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
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.
alexcarpenter
changed the title
`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.
2 participants
Description
The Mosaic
Menupopup already scrolled —overflowY: autounder amax-height: var(--cl-available-height)cap — but it did so with a bare browser scrollbar, rather than the ScrollArea treatment every other overflowing Mosaic surface gets. This composes it out of the shared utilities, plus two small style fixes that started the branch.Menu.Popupis now chrome + a scroll region, the same split the Dialog panel makes. The popup keeps the background, shadow, radius and the height cap and takesscrollAreaRoot; a newmenu-viewportinside it takesscrollAreaViewport()and does the scrolling. They can't be one element: the edge fade is amask-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.Menu.Separatorcancels that padding with a negative margin to go full-bleed; from inside a viewport that clips its inline axis, a bleed past the popup's padding would be clipped instead. Geometry is unchanged.Menu.Itemheight wasspace['7'](1.75rem). Bumped tospace['8'](2rem).Menu.ItemandMenu.Separatorsat in the height-capped flex column at flex's defaultshrink: 1, so a full menu compressed its rows to fit the cap rather than overflowing it — rows rendered at 26px instead of the 32px above, and the popup did not scroll at all until the rows bottomed out at min-content height. Both are nowflexShrink: 0.overflow-x. The popup setoverflowX: 'visible'alongsideoverflowY: 'auto'. CSS computesvisibletoautowhen the other axis isn'tvisible, so the declaration never did anything — it just read as though the popup allowed horizontal overflow.A
Menu/Overflowingswingset story renders more rows than fit, so the fade and the themed thumb are visible.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change