Skip to content

Generalize #1849 menu coordination to all context menus (#1849) - #2286

Merged
callumalpass merged 2 commits into
callumalpass:mainfrom
YBKF:fix/context-menu-stacking
Sep 4, 2026
Merged

Generalize #1849 menu coordination to all context menus (#1849)#2286
callumalpass merged 2 commits into
callumalpass:mainfrom
YBKF:fix/context-menu-stacking

Conversation

@YBKF

@YBKF YBKF commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Repeatedly activating the priority indicator on a task card, or opening the task context menu repeatedly, created another menu each time instead of closing or reusing the one already open. Both of these were observed and reproduced (see "Reproduction" below); the other context menus were not observed to stack.

The reason the two differ is not that the other menus are safer — it is that no wrapper coordinated menu visibility at all, except DateContextMenu, which has had its own fix since #1849 (4.8.0). Because every other wrapper shares the exact same pattern, the fix is applied to all of them rather than patching only the two observed cases: the remaining menus are one refactor away from the same bug, and the behaviour should not depend on which component happens to be clicked.

Reproduction

  • Priority: click the priority dot on a task card repeatedly — each click opens another menu; the previous ones stay open.
  • Task: click the task options button (three dots button) repeatedly from the task card — the same stacking occurs.
  • Not observed (fixed anyway as a precaution): status, date, recurrence, reminder, ICS event and batch menus.

What's in it

  • New ContextMenuCoordinator owns menu visibility: it hides the active menu before showing a new one and treats a repeated activation of the same trigger as a toggle.
  • Applied to the priority, status, date, recurrence, reminder, task, ICS event and batch menus via showCoordinatedMenu() / showCoordinatedMenuAtElement().
  • Removed the duplicated active-menu state from DateContextMenu — that component's existing behaviour is unchanged, it now comes from the shared coordinator — and the element-anchored positioning copies shared by five wrappers.
  • Triggers are resolved from the plugin-wide control conventions (role="button" set by prepareInteractiveControl, data-tn-action, data-type), so clicking an inner icon maps to the same control.

Testing

  • New tests/unit/components/ContextMenuCoordinator.test.ts (14 cases: toggle on the same trigger, nested elements, replacement across triggers, reopening after an external hide, keyboard-event anchoring).
  • Existing DateContextMenu.issue-1849 coverage passes unchanged, confirming the refactor preserves the behaviour that shipped in 4.8.0.
  • npm test: 4159 passed, 0 failed. npm run typecheck and npm run lint clean.

Manual verification

  • Priority dot: click repeatedly — the menu toggles open/closed and never stacks
  • Task context menu: click the task options button (three dots button) repeatedly from the task card — no stacking
  • With a menu open, open a different menu — the first one closes
  • Status, date, recurrence and reminder indicators behave the same (no regressions)
  • Keyboard activation (Enter / Space on a focused indicator) anchors the menu below it
  • Repeat in a pop-out window — item colour styling still applies

Notes

  • The coordinator only relies on the onHide / hide / showAtMouseEvent / showAtPosition surface, so it works for ContextMenu instances and for plain Menu instances — the latter matters for TaskContextMenu, one of the two reproduced cases, since it can be populated onto a caller-supplied Menu.
  • Views that build their own menus directly with new Menu() are untouched and can be migrated with the same one-line call if desired.
  • Release notes updated in docs/releases/unreleased.md.

Refs #1849

YBKF and others added 2 commits September 3, 2026 17:07
…alpass#1849)


Add ContextMenuCoordinator so only one context menu is open at a time; re-activating the same trigger toggles it closed. Applied to priority, status, date, recurrence, reminder, task, ICS event and batch menus, removing duplicated per-component state. Generalizes the callumalpass#1849 fix, which previously covered date menus only.
@YBKF
YBKF marked this pull request as ready for review September 4, 2026 00:50
@callumalpass
callumalpass merged commit 77e3099 into callumalpass:main Sep 4, 2026
2 checks passed
@YBKF
YBKF deleted the fix/context-menu-stacking branch September 7, 2026 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants