refactor(menu): drop redundant caller-level menu height caps

[RohanK6]

Problem / Motivation

Now that the shared Radix menu primitives cap content height to
max-h-[var(--radix-*-content-available-height)] overflow-y-auto overscroll-contain
and scroll (PR #7186), several dashboard callers still carry their own
per-menu max-h-[…] overflow-y-auto point patches that predate the
primitive-level fix. Those are redundant second spellings that can drift
from the primitive over time.

Why it matters

A caller cap whose only purpose was "don't let the menu overflow the
viewport, scroll instead" now duplicates behavior the primitive
guarantees. Leaving the duplicates in place invites drift: a future
change to the primitive default silently stops applying at those sites,
and a reader can't tell an intentional tighter cap from a leftover patch.

What changed

A per-site pass over the caps the issue lists, splitting them by intent:

• Removed the two caps whose sole job the primitive now does — pure
  "don't exceed the viewport" viewport-fraction caps:

  • InstanceTabBar.tsx — instance switcher menu max-h-[60vh] overflow-y-auto
  • ChatSidebar.tsx — session filter menu max-h-[70vh] overflow-y-auto

  (The max-w-[calc(100vw-1rem)] phone-fit cap on the filter menu is a
  horizontal constraint the primitive does not provide, so it stays.)

• Kept, but composed correctly — the genuinely tighter fixed-pixel
  caps that hold a growable list submenu compact. Because cn() runs
  tailwind-merge, which dedupes max-h-*, a bare max-h-[<px>] would
  override the primitive's max-h-[var(--radix-*-content-available-height)]
  and lose the never-clip viewport floor on a short viewport. So each kept
  cap is spelled max-h-[min(<px>,var(--radix-dropdown-menu-content-available-height))]
  (the codebase's existing pattern in AgentSelector.tsx) — genuinely
  tighter and still floored to the viewport. Their now-redundant
  overflow-y-auto is dropped (the primitive supplies it), and each is
  annotated with the accurate model:

  • CronJobMoveMenu.tsx folder picker (240px)
  • CronRowActions.tsx move-to-folder submenu (240px)
  • ArtifactDetailPage.tsx folder picker (300px)
  • ChatPage.tsx MCP tools submenu (340px)
  • ChatSidebar.tsx new-chat-in-folder / new-chat-on-crew submenus (300px)

• Updated the ChatSidebar folder-scroll rationale comment to reference
  the primitive cap instead of the removed caller max-h.

The plain <div className="max-h-… overflow-y-auto"> inner scroll
regions in these files are not menu-content primitives and are out of
scope — left untouched.

This matches the shape the triage comment suggested: a mechanical removal
of the caps the primitive default subsumes, plus a short annotated list
of the intentionally-tighter caps that remain. Behavior at the two
removed sites now follows the primitive (cap to available viewport
height, scroll) instead of a fixed viewport fraction; both still scroll
and never clip.

Tests

• npx tsc -b — clean.
• npx eslint on all six changed files — 0 errors, 0 new warnings.
• npx vitest run on the suites that exercise the touched menus —
  67/67 pass:
  • menuContentOverflow.test.tsx (the primitive cap/scroll test from fix(menu): cap dropdown/context menus to viewport height and scroll #7186)
  • InstanceTabBar.test.tsx
  • cronFoldersSchedulePage.test.tsx
  • ChatSidebar.createMenu.test.tsx
• Re-verified after addressing the Design/First-Principles review: the
  kept caps now compose via min() so the primitive's viewport floor is
  preserved, and the duplicate overflow-y-auto is removed.

Screenshots / video

Why no screenshot: there is no rendered delta at rest or in the
common case — the change only swaps two menus' redundant fixed
viewport-fraction caps (60vh/70vh) for the primitive's own
max-h-[var(--radix-*-content-available-height)], which Radix already
applies, so a short menu renders at content height and a full one still
scrolls exactly as before. The only reachable difference is an edge case
(a menu whose content exceeds the fraction on a tall viewport where the
primitive's available-height is larger): it may grow marginally taller
before scrolling instead of scrolling at the fixed fraction — still
capped to the viewport, still scrollable, never clipped. A screenshot at
rest would show no difference and would be misleading evidence for that
edge case; the four kept caps and all comment/annotation changes have no
visual effect at all.

Pattern harvest

Not generalizable: a one-off cleanup of duplicated CSS caps against a
primitive default; no reusable rule.

Related Issues

Fixes #7227

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 6ac9cf3f902b89399ff9f1980b32f99c6ea4e8a4 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 6ac9cf3

[github-actions]

UX Review (Fable 5, fork) — ✅ PASS

UX-level review of 6ac9cf3f902b89399ff9f1980b32f99c6ea4e8a4 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The base primitive already carries max-h-[var(--radix-dropdown-menu-content-available-height)] overflow-y-auto, so every changed call site keeps scrolling behavior while gaining the viewport never-clip floor. No user-facing strings, controls, or flows change; the fixed caps (240/300/340px) are preserved via min(), and the two menus that dropped their 60vh/70vh caps now follow the primitive's standard available-height behavior — consistent with the rest of the product, not a regression. No screenshots in the diff.

UX-Verdict: PASS

Pure short-viewport robustness fix: dropdowns now defer to the primitive's viewport cap, so no menu clips off-screen and every picker keeps its compact height.

[UX-REVIEWED] 6ac9cf3

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 6ac9cf3f902b89399ff9f1980b32f99c6ea4e8a4 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 6ac9cf3

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of 6ac9cf3f902b89399ff9f1980b32f99c6ea4e8a4 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The patch checks out against the base: the primitive (website/src/components/ui/dropdown-menu.tsx:28,133) does supply max-h-[var(--radix-dropdown-menu-content-available-height)] overflow-y-auto, the min() composition matches the existing AgentSelector.tsx:264 pattern, and every hunk is accounted for by the description.

Design-Verdict: PASS

Redundant caps removed and intentional ones correctly composed via min(), preserving the primitive's viewport floor — the right split, verified against the primitive.

Suggestions

• The same 5-line tailwind-merge rationale is now hand-copied at six call sites, and any future caller writing a bare max-h-[Npx] silently reintroduces the clipping bug this PR fixes; a maxHeightPx prop (or shared class helper) on the menu primitives would own the composition once — worth a follow-up.

[DESIGN-REVIEWED] 6ac9cf3

[github-actions]

First Principles Review (Fable 5, fork) — ⚠️ could not complete

The first-principles review did not produce a verdict for 6ac9cf3f902b89399ff9f1980b32f99c6ea4e8a4 (the review step completed but returned no verdict header). See the Fork First Principles Review job logs. Advisory — does not block merge.

[RohanK6]

Both review findings were legitimate and are fixed at root cause in 553d49622:

Design Review (CONCERNS) — the tailwind-merge override: Correct. cn() is twMerge(clsx(...)), and tailwind-merge dedupes max-h-*, so a bare caller max-h-[240px] replaced the primitive's max-h-[var(--radix-*-content-available-height)] and dropped the never-clip viewport floor on a short viewport — and my annotations codified the wrong model. Fixed by composing each kept cap with the primitive var via max-h-[min(<px>,var(--radix-dropdown-menu-content-available-height))], exactly the existing pattern at AgentSelector.tsx:264. The cap stays genuinely tighter and the viewport floor is restored. Annotations rewritten to state this accurately.

First Principles (advisory) — the residual overflow-y-auto: Correct. The kept sites still carried overflow-y-auto, the same second spelling this PR removes. Dropped from all six kept sites; the primitive supplies overflow-y-auto overscroll-contain.

Re-verified on the new head: tsc -b clean, eslint 0 errors / 0 new warnings, vitest 67/67 (incl. menuContentOverflow). PR body's "What changed" updated to match.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• This PR is OVERLAPPING with PR #7255. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7760: KEEP. A large file-split PR moves the exact line PR #7760 edits; ordering must be agreed and the loser rebased, since a clean textual merge can silently discard the fix. Files: website/src/pages/ChatPage.tsx.
• This PR is OVERLAPPING with PR #7271. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7760: KEEP. Different goals (coarse-pointer submenu rendering vs. de-duplicating caller height caps) touching the same primitive contract, with a concrete interaction on the phone inline branch that neither PR currently accounts for. Files: website/src/components/ui/dropdown-menu.tsx.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

[RohanK6]

Thanks @bolichen97 – will keep this open per your suggestion. Confirming that this is ready for review

[bolichen97]

Approving — className-only cleanup, no interaction or layout redesign.

Verified the premise directly against website/src/components/ui/dropdown-menu.tsx on this head: both DropdownMenuContent (line 28) and DropdownMenuSubContent (line 133) already carry max-h-[var(--radix-dropdown-menu-content-available-height)] overflow-y-auto overscroll-contain, and Radix publishes that same var name on Content and SubContent alike. So every caller cap this PR removes really is a second spelling of the primitive's, and every dropped overflow-y-auto really is supplied below.

Per-site check:

• InstanceTabBar.tsx and ChatSidebar.tsx filter menu: pure viewport-fraction caps (60vh / 70vh) removed, nothing else touched. The max-w-[calc(100vw-1rem)] phone-fit constraint is correctly left in place — the primitive supplies no horizontal cap.
• The five kept fixed-pixel caps are spelled max-h-[min(<px>,var(--radix-dropdown-menu-content-available-height))], which is the right composition rather than a bare max-h-[<px>]: cn()'s tailwind-merge dedupes max-h-*, so a bare pixel cap would replace the primitive's and drop the never-clip viewport floor on a short viewport. Matches the existing precedent in AgentSelector.tsx:264 (correctly using the popover var for its own primitive).
• The ChatSidebar folder-ordering comment is updated to cite the primitive instead of the deleted caller max-h — the comment no longer describes code that is gone.
• No component, handler, aria attribute, item, or ordering changed anywhere in the diff; the only removals are max-h/overflow-y-auto utilities. Consistent with <!-- no-visual-delta -->.

The one real behavioral delta is the acknowledged edge case: on a tall viewport the two uncapped menus may grow past the old fixed fraction before scrolling. Still viewport-bounded and still scrollable via the primitive, and it is exactly the primitive default this PR is aligning to, so it is in scope rather than scope creep.

No new tests, which is fine here: min() against a Radix runtime CSS var is not computable in jsdom, so the only possible assertion would be on the class string, and menuContentOverflow.test.tsx from #7186 already pins the primitive contract these sites now inherit.

All checks pass (61 pass, 6 skipped, 0 failing).