Skip to content

fix(GenericMenu): restore scroll on dropdown-menu panels - #1155

Draft
claude[bot] wants to merge 2 commits into
mainfrom
fix/org-selector-popover-scroll
Draft

fix(GenericMenu): restore scroll on dropdown-menu panels#1155
claude[bot] wants to merge 2 commits into
mainfrom
fix/org-selector-popover-scroll

Conversation

@claude

@claude claude Bot commented Jul 29, 2026

Copy link
Copy Markdown

Why?

Long dropdown-menu-type GenericMenu panels stopped scrolling — e.g. the org switcher's organization list: the panel clips overflowing content instead of scrolling, so items past the visible area are inaccessible.

Before: panel has overflow: hidden with no effective scroll override → items past the available height are cut off and unreachable.
After: the panel scrolls (overflow-y: auto) once content exceeds --radix-dropdown-menu-content-available-height, same as before the regression.

Root cause: Dropdown.module.css's .dropdown-menu-content already declares overflow-y: auto specifically to override the base panel's overflow: hidden (.generic-menu-panel in src/components/GenericMenu/GenericMenu.module.css, line 5). Both rules have equal specificity (single class selectors), and everything is wrapped in one @layer clickui (see plugins/css-colocate/postcss-clickui-layers.ts), so within that layer the rule registered last wins.

The css-colocate build's insertAtTop helper (plugins/css-colocate/import-inject.ts) unconditionally injects each file's own colocated CSS import at the very top of its own compiled JS module — before that module's other imports, including the one that pulls in GenericMenuPanel from GenericMenu. That means Dropdown.css ends up registered before GenericMenu.css in the build output, inverting the intended override order: the base's overflow: hidden (declared later) beats Dropdown's overflow-y: auto (declared earlier), so the panel never scrolls.

Confirmed by building the package and inspecting dist/esm/components/Dropdown/index.jsimport "./Dropdown.css"; is literally the first line, ahead of the import that pulls in GenericMenu.

This is a regression introduced by #1097 (styled-components → CSS Modules migration for the GenericMenu cluster): previously, DropdownMenuContent = styled(GenericMenuPanel) guaranteed the override styles were registered after the base by JS module evaluation order; CSS Modules colocation doesn't carry that same guarantee.

How?

  • src/components/GenericMenu/GenericMenu.module.css: added overflow-y: auto; to .generic-menu-panel_type_dropdown-menu — the same stylesheet, after the base .generic-menu-panel rule, so the override can never lose to cross-file injection order. This also covers any consumer that renders GenericMenuPanel type="dropdown-menu" directly, without going through Dropdown.Content.
  • Left Dropdown.module.css's existing .dropdown-menu-content { overflow-y: auto; } in place (now redundant but harmless) to keep the diff minimal.
  • Added a changeset (patch).

Tickets?

  • N/A (reported via Slack in #click-ui)

Contribution checklist?

  • You've done enough research before writing
  • You have reviewed the PR
  • The commit messages are detailed
  • The build command runs locally (vite build succeeds; verified the fix in the built dist/esm output)
  • Assets or static content are linked and stored in the project
  • For documentation, guides or references, you've tested the commands

Security checklist?

  • All user inputs are validated and sanitized
  • No usage of dangerouslySetInnerHTML
  • Sensitive data has been identified and is being protected properly
  • Build output contains no secrets or API keys

Notes for reviewers

  • yarn typecheck and yarn lint:css pass; yarn vitest run src/components/GenericMenu src/components/Dropdown passes (16/16).
  • This environment couldn't run the Docker-based Playwright visual-regression suite (tests/overlays/genericmenu.spec.ts). The existing Dropdown/GenericMenu stories used by that suite only render a handful of items that fit within the available height, so overflow-y: auto vs overflow: hidden shouldn't produce any visible pixel diff there (no scrollbar renders unless content actually overflows) — but a reviewer/CI should still confirm the snapshots are unaffected.
  • Opened as draft pending a repo maintainer's confirmation that this is indeed how the org switcher renders its panel (it lives in the ClickHouse console app, which wasn't accessible from this session) — the fix is written to be correct for GenericMenuPanel type="dropdown-menu" regardless of which wrapper the consumer uses.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com


Generated by Claude Code

Long dropdown-menu-type GenericMenu panels (e.g. the org switcher's
organization list) stopped scrolling: the panel clips overflow instead
of scrolling, so items past the visible area are inaccessible.

Root cause: Dropdown.module.css's `.dropdown-menu-content` sets
`overflow-y: auto` specifically to override the base panel's
`overflow: hidden` (`.generic-menu-panel` in GenericMenu.module.css).
Both rules have equal specificity, so which one wins depends on which
stylesheet is registered last. The css-colocate build's
`insertAtTop` (plugins/css-colocate/import-inject.ts) injects each
file's own CSS import at the very top of its own JS module, so
Dropdown's stylesheet ends up registered *before* GenericMenu's -
inverting the intended override order and letting `overflow: hidden`
win.

Fix: move the scroll override onto
`.generic-menu-panel_type_dropdown-menu` in GenericMenu.module.css
itself, after the base rule in the same stylesheet, so it can never
lose to cross-file injection order. This also covers any consumer
that renders GenericMenuPanel type="dropdown-menu" directly, without
going through Dropdown.Content.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DimTHLZxKnUUizAT3kLFoR
@changeset-bot

changeset-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6f99b3d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@clickhouse/click-ui Patch

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

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@XOP
XOP requested review from DreaminDani and XOP July 30, 2026 09:55
@XOP XOP added the bug Something isn't working label Jul 30, 2026

@DreaminDani DreaminDani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice find

.generic-menu-panel_type_dropdown-menu {
max-width: var(--radix-dropdown-menu-content-available-width);
max-height: var(--radix-dropdown-menu-content-available-height);
/* Dropdown.module.css's `.dropdown-menu-content` also declares

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feels like a lot of explanation for such a small line of code...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair, trimmed it to one line.

Condense the overflow-y override comment to a single line capturing
the essential why, per @DreaminDani's review.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DimTHLZxKnUUizAT3kLFoR
@workflow-authentication-public

Copy link
Copy Markdown
Contributor

Storybook Preview Deployed

✅ Preview URL: https://click-3mmrafx34-clickhouse.vercel.app

Built from commit: 0dd16c6280ef2b57f90aaa4f2084f49a372747b0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants