Skip to content

diagram: Menu cannot be dismissed by clicking its own trigger button #805

Description

@bpowers

Problem

The reimplemented Menu component (src/diagram/components/Menu.tsx) cannot be dismissed by clicking its own trigger button while it is open. Clicking the trigger flickers the menu closed and immediately reopens it.

The outside-dismiss effect (Menu.tsx:91-112) registers a document mousedown listener that calls onClose() whenever a press lands outside the menu content (contentRef):

const onMouseDown = (event: MouseEvent): void => {
  const content = contentRef.current;
  if (content && !content.contains(event.target as Node)) {
    onClose();
  }
};

The trigger button is not inside the menu content (e.g. the account IconButton in src/app/Home.tsx:256-274), and the menu's own proxy <span> is pointer-events: none (Menu.tsx:136-146), so it cannot absorb the press either. Clicking the trigger while the menu is open therefore produces:

  1. mousedown -> document outside listener -> onClose() (menu closes)
  2. click -> trigger's handleMenu -> setAnchorEl(currentTarget) (menu reopens)

Net effect: the toolbar account menu can no longer be toggled shut by clicking its own trigger; it flickers closed and immediately reopens. Escape and clicking elsewhere still dismiss it correctly.

Why it matters

This is a behavioral regression in a shared, reused component. Toggling a menu by clicking its trigger again is a standard, expected interaction (the prior Radix-based implementation handled this). It affects every caller that uses a trigger button outside the menu content, including the account menu in the app toolbar.

Component(s) affected

  • src/diagram/components/Menu.tsx (the dismiss effect)
  • Callers with an external trigger, e.g. src/app/Home.tsx account menu

Suggested fix

Pass anchorEl into the dismiss effect and skip onClose() when the press lands on the trigger:

if (content && !content.contains(target) && !(anchorEl?.contains(target))) {
  onClose();
}

(Add anchorEl to the effect's dependency array.)

Also add a regression test in src/diagram/tests/menu.test.tsx -- the current tests only cover anchor positioning, not outside-click / trigger-toggle dismissal. Cover: (a) clicking outside closes, (b) clicking the trigger while open does not leave the menu reopened, (c) Escape closes.

How discovered

Identified during review of PR #804 (branch dep-diet-round2), which reimplements the Menu component. This may merge with #804 if not caught.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions