base-ui icon indicating copy to clipboard operation
base-ui copied to clipboard

[docs] Opening a dialog from menu example is wrong

Open radqut opened this issue 2 months ago • 3 comments

Bug report

Current behavior

When I click on a content of an opened dialog, a menu closes under the dialog backdrop.

Expected behavior

The previous floating components should not be closed

Reproducible example

Link to wrong demo: https://base-ui.com/react/components/dialog#open-from-a-menu

Stackblitz showing the bug: https://stackblitz.com/edit/vitejs-vite-psmrztnd?file=src%2FApp.tsx,vite.config.ts,src%2Findex.css&terminal=dev

Base UI version

1.0.0-beta.4

Which browser are you using?

Chrome

Which OS are you using?

Debian 13

radqut avatar Nov 06 '25 14:11 radqut

~It seems like a bug, the sandbox is set up correctly according to this doc~

@MichailShcherbakov Actually the docs example is wrong, it needs to be set up like this:

<Menu.Root>
  <Menu.Portal>
    <Menu.Positioner>
      <Menu.Popup>
        <Menu.Item>Normal menu item</Menu.Item>

        <Dialog.Root>
          <Menu.Item render={<Dialog.Trigger />}> {/* 👈 */}
            Dialog trigger menu item
          </Menu.Item>
          <Dialog.Portal>
            {/* the rest of the dialog */}
          </Dialog.Portal>
        </Dialog.Root>    
      </Menu.Popup>
    </Menu.Positioner>
  </Menu.Portal>
</Menu.Root>

Here's a working sandbox: https://stackblitz.com/edit/vitejs-vite-ygdt5ykm?file=src%2FApp.tsx

mj12albert avatar Nov 06 '25 16:11 mj12albert

The example is correct actually. It only breaks if you want to use closeOnClick={false} on the item, but that's not strictly necessary. If you want to keep the menu item underneath, you need to render the dialog inside the menu popup. You can't do that if the menu closes, though - it must be outside. aria-haspopup="dialog" should be set on the menu item in the controlled scenario, though.


As for composing the menu item with the dialog trigger:

It's actually a bit more complicated because <Dialog.Trigger> renders a <button> while <Menu.Item> renders a <div>. This causes the nativeButton mismatch error.

Three different fixes (all here https://stackblitz.com/edit/vitejs-vite-jzxjfuzv?file=src%2FApp.tsx):

  1. <Dialog.Trigger> needs to also render a <div> so it matches <Menu.Item>
<Menu.Item
  closeOnClick={false}
  render={<Dialog.Trigger render={<div />} nativeButton={false} />}
>
  First song
</Menu.Item>
  1. Make Menu.Item a nativeButton given it renders one out. This causes a styles mismatch though.
<Menu.Item
  closeOnClick={false}
  render={<Dialog.Trigger />}
  nativeButton
>
  First song
</Menu.Item>
  1. Swap the ordering, so Dialog.Trigger is the one rendering out Menu.Item. tabIndex={0} is applied to <Dialog.Trigger> by default though, so you need to unset tabIndex
<Dialog.Trigger
  render={<Menu.Item closeOnClick={false} />}
  nativeButton={false}
  tabIndex={-1} {/* this overrides roving tabIndex, though */}
>
  First song
</Dialog.Trigger>

@mj12albert most likely solution No. 1 is the correct one here

atomiks avatar Nov 14 '25 10:11 atomiks

I see ~ I was following this experiment: https://github.com/mui/base-ui/blob/master/docs/src/app/(private)/experiments/menu/complex-nesting.tsx

mj12albert avatar Nov 14 '25 11:11 mj12albert