fix(ui): keep sidebar user dropdown open

[malinskibeniamin]

What

Fix Base UI dropdown content rendering so the sidebar user menu stays open and usable. @SpicyPete

Why

The visible dropdown content was nested inside the Base UI popup element, so focus and menu behavior were attached to a wrapper instead of the rendered menu surface.

How

Render the visible dropdown surface directly through Base UI's Popup.render, preserving Base UI props, role, focus handling, and Radix-compatible state attributes on the same element.

Acceptance Criteria

• Sidebar user menu remains open after activation.
• Menu actions are clickable without needing to keep focus pinned.
• Dropdown content keeps data-state styling compatibility.

Testing Steps

• bun run type:check
• bun run lint:check (reports existing repo-wide restricted-import/unused-suppression issues; exit 0)
• bun run test

[malinskibeniamin]

@claude review

[claude]

Claude finished @malinskibeniamin's task in 3m 35s —— View job

---

Claude PR Review

• Read PR changes (dropdown-menu.tsx, dropdown-menu.test.tsx)
• Examine the dropdown menu implementation context
• Analyze correctness, accessibility, and potential regressions
• Post review feedback

---

The fix's core idea — collapsing the Popup wrapper and the visible content into a single rendered element via the function-form render prop — is the right call and resolves the reported sidebar bug. Below are concerns ordered by impact.

🟠 Close animation is silently broken

By removing the inner motion.div (with its initial/animate/exit/transition), the outer <AnimatePresence> in dropdown-menu.tsx:268-278 no longer has a motion child to delay-unmount. When isOpen flips to false, the Portal subtree is torn down immediately — the CSS data-[state=closed]:animate-out / fade-out-0 / zoom-out-95 / slide-in-* classes never get a frame to play because the element is gone.

DropdownMenuSubContent (dropdown-menu.tsx:164-180) still relies on the same CSS classes for state transitions and doesn't use AnimatePresence, so it has the same gap there — but the regression here is that the main content used to have a real exit animation and now silently doesn't.

Options:

1. Accept the trade-off and remove the now-pointless <AnimatePresence> wrapper (and the useMirroredOpen mirror it depends on, if nothing else uses it) — fewer moving parts, no behavior change.
2. Keep a motion-aware wrapper for exit, but at this point you'd be re-introducing what the fix removed.

I'd recommend option 1 unless the close animation is wanted back.

🟠 Dead transition prop + misleading type

dropdown-menu.tsx:182-198 still declares:

type DropdownMenuContentProps = React.ComponentProps<typeof DropdownMenuPrimitive.Popup> &
  HTMLMotionProps<'div'> &
  Pick<PortalContentProps, 'container' | 'onOpenAutoFocus'> & {
    transition?: Transition;
    ...
  };

…and dropdown-menu.tsx:207 destructures transition: _transition only to discard it. The HTMLMotionProps<'div'> intersection also implies the component accepts whileHover, initial, animate, exit, etc. — but those are now collected into ...props and spread onto DropdownMenuPrimitive.Popup (dropdown-menu.tsx:225), not the visible <div>. Base UI's Popup will swallow or forward them as DOM attrs.

Consumers will see them in autocomplete and assume they work. Suggest:

• Drop HTMLMotionProps<'div'> and transition?: Transition from the type
• Remove the transition: _transition = … destructure
• Either change the type to Omit<React.ComponentProps<typeof DropdownMenuPrimitive.Popup>, 'render'> & React.HTMLAttributes<HTMLDivElement> & Pick<PortalContentProps, ...> or accept that ...props are popup-level props now and not div-level props (and update where the spread lands accordingly — see next point).

🟡 {...props} is on Popup; HTML/data props won't reach the menu DOM node

dropdown-menu.tsx:224-225:

<DropdownMenuPrimitive.Popup
  {...props}
 
  render={(popupProps, state) => (
    <div {...popupProps} ... />
  )}
/>

If a consumer passes e.g. data-testid="user-menu" or id="..." or aria-*, those land on the Popup component, and whether they end up on the visible <div> depends on Base UI's prop-forwarding. Previously they went straight onto the rendered motion.div. Since you've inlined the render, the cleanest pattern is:

<DropdownMenuPrimitive.Popup
 
  render={(popupProps, state) => (
    <div
      {...popupProps}
      {...props}   // user props win after Base UI's
      className={cn(popupProps.className, 'data-[state=closed]:...', className)}
     
      ? 'open' : 'closed'}
      style={{ ...popupProps.style, ...props.style, willChange: 'opacity, transform' }}
    >
      ...
    </div>
  )}
/>

Also note cn(popupProps.className, ...) — currently popupProps.className (if Base UI ever sets one) is silently dropped at dropdown-menu.tsx:230-233. Low risk today, but it's the canonical merge pattern used elsewhere in the file.

🟡 Test doesn't actually exercise the bug it fixes

dropdown-menu.test.tsx opens the menu and asserts the rendered surface has the right data-slot/data-state. That's a render-shape assertion, not a regression test for "menu stays open after activation" (the PR's first acceptance criterion). A reader looking at this test in six months won't know the bug it's guarding against.

Recommend adding two assertions to the same test (no new setup needed):

// Pointer down inside the menu surface (or moving focus to the item) must not close it.
await user.hover(screen.getByRole('menuitem', { name: 'Preferences' }));
expect(screen.getByRole('menu')).toBeInTheDocument();

// Activating an item then re-opening keeps menu role intact (sanity).
await user.keyboard('{Escape}');
expect(screen.queryByRole('menu')).not.toBeInTheDocument();

The first line is the one that would have caught the original bug.

🟢 Minor

• data-slot="dropdown-menu-popup" set on Popup (line 226) is shadowed by data-slot="dropdown-menu-content" on the inner <div> (line 234) — i.e. the dropdown-menu-popup slot value is no longer queryable. Confirmed no other code in frontend/src references it, so this is safe today; just flagging for future selector-based work.
• The DropdownMenuSubContent path (line 164-180) still uses the older renderWithDataState('div') + child-content pattern, which has the same root-cause shape the main path is fixing. If sub-menus exhibit a similar focus-pinning issue, they'd need the same treatment. Out of scope for this PR but worth a follow-up issue.
• dropdown-menu.tsx:253-254 comment about <AnimatePresence> and keepMounted is now misleading — there's no motion child for AnimatePresence to gate anyway.

Summary

The fix is correct and worth landing, but consider tightening up the dead transition/HTMLMotionProps type surface and either restoring or removing the now-vestigial <AnimatePresence> wrapper. A regression test that actually pins the "menu stays open" behavior would make this much more durable.

Fix this →
• Branch: beniamin-malinski/sidebar-user-menu-dropdown

[malinskibeniamin]

@claude review

[claude]

Claude finished @malinskibeniamin's task in 2m 12s —— View job

---

Claude PR Review

• Read PR changes (dropdown-menu.tsx, dropdown-menu.test.tsx)
• Verify if previous review feedback was addressed
• Analyze correctness, accessibility, and potential regressions
• Post review feedback

---

Most of the issues from the previous review pass have been addressed (the dead transition/HTMLMotionProps type surface is gone, popupProps.className is now merged via cn, {...props} is spread on the visible <div> after {...popupProps}, and the test now exercises hover + Escape). A few new things stand out.

🟡 keepMounted on the Portal is redundant in the non-keepMounted branch

dropdown-menu.tsx:269-276:

if (!isOpen) {
  return null;
}

return (
  <DropdownMenuPrimitive.Portal container={container ?? portalContainer} keepMounted>
    {popup}
  </DropdownMenuPrimitive.Portal>
);

The outer if (!isOpen) return null already unmounts the entire portal subtree on close, so keepMounted on the Portal here does nothing — Base UI never gets the chance to "keep" anything because React tears down the parent first. Either drop the keepMounted attribute here, or remove the !isOpen gate (and rely on Base UI's mounting). I'd lean toward dropping the attribute, since removing the gate would change behavior.

🟡 Close-animation classes are dead in the default (keepMounted={false}) path

dropdown-menu.tsx:233 declares data-[state=closed]:animate-out, data-[state=closed]:fade-out-0, data-[state=closed]:zoom-out-95, etc. But because the parent returns null as soon as isOpen flips false, the element is unmounted before its data-state can transition to closed. So those classes never trigger in the default branch.

They will fire when consumers pass keepMounted={true} (the branch at lines 257-267 doesn't gate on isOpen), so the classes aren't entirely useless — but the comment at lines 255-256 frames this as deliberate, while the default branch silently has no close animation. Worth either:

• Documenting in the JSDoc that close animation only plays when keepMounted is set, or
• Removing the isOpen gate so Base UI controls the lifecycle and the closed-state classes can play.

🟡 onOpenAutoFocus is part of the public type but discarded

dropdown-menu.tsx:184 keeps onOpenAutoFocus in the Pick<PortalContentProps, ...> intersection, but line 208 destructures it as _onOpenAutoFocus and never uses it. Consumers will see it in autocomplete and assume it's wired up. Either:

• Pass it through to DropdownMenuPrimitive.Popup (Base UI's API for focus management), or
• Drop onOpenAutoFocus from the Pick<...> so the type matches reality.

🟡 Popup-specific props from Omit<…Popup, 'className' | 'render'> now land on the <div>

The type at line 182 intersects Omit<React.ComponentProps<typeof DropdownMenuPrimitive.Popup>, 'className' | 'render'> with React.HTMLAttributes<HTMLDivElement>. Anything in the Popup prop surface that isn't explicitly destructured (sideOffset/align/alignOffset/side/container/finalFocus/onOpenAutoFocus/keepMounted/style) flows into ...props and gets spread onto the visible <div> at line 230, not the Popup.

In practice this is unlikely to bite anyone today, since the documented props are all handled. But the type implies Popup-level options work, and they won't. Either:

• Spread ...props on the Popup as well (after {...popupProps} inside render to preserve precedence), or
• Tighten the type to just React.HTMLAttributes<HTMLDivElement> & Pick<…Popup, …explicit list> so unsupported Popup props are rejected at compile time.

🟢 Minor

• data-slot="dropdown-menu-popup" at line 225 is still overridden by data-slot="dropdown-menu-content" at line 236 — same observation as the prior review, low risk, only a follow-up if anything ever queries the popup slot.
• DropdownMenuSubContent (lines 164-180) still uses the older renderWithDataState('div') pattern. If sub-menus exhibit the same focus-pinning issue this PR fixes for the main menu, they'd need the same treatment. Out of scope for this PR — worth a follow-up issue.
• The new test asserts the post-Escape state via queryByRole('menu'). Because of the !isOpen gate this works synchronously, but if someone later restores a close animation (or switches to keepMounted) the element will linger and this assertion would need await screen.findBy…. Not a current bug — flagging in case the lifecycle changes.

Summary

Solid follow-up to the prior review. The remaining items are mostly cleanup of stale prop surface (onOpenAutoFocus, Popup-intersection type) and clarifying the keepMounted vs. !isOpen gating — which would be visible to anyone passing keepMounted={true} and expecting a close animation, or anyone reading the type definition. None of these block landing the fix.

Fix this →

---

• Branch: beniamin-malinski/sidebar-user-menu-dropdown

[malinskibeniamin]

This is a big change, maybe there is an easier way to get this resolved, basically when you open a user menu dropdown in the sidebar of console, it will sometimes not allow you to interact with the component because it keeps closing as soon as it loses focus