Skip to content

refactor: implement safe triangle pattern for submenu navigation#11183

Draft
DiegoCardoso wants to merge 13 commits intomainfrom
refactor/context-menu/safe-triangle
Draft

refactor: implement safe triangle pattern for submenu navigation#11183
DiegoCardoso wants to merge 13 commits intomainfrom
refactor/context-menu/safe-triangle

Conversation

@DiegoCardoso
Copy link
Copy Markdown
Contributor

@DiegoCardoso DiegoCardoso commented Feb 20, 2026

Summary

Fixes #9904

Introduces a SafeTriangleController that prevents premature submenu closing when the user moves the mouse diagonally from a parent menu item toward the open submenu. Integrated into both vaadin-context-menu (items mixin) and vaadin-menu-bar (menu bar mixin).

Uses atan2 angle comparison (inspired by React Aria's approach) to detect whether the cursor is aimed at the submenu, with a 15° angular tolerance and a 2-strike threshold before allowing submenu switches.

How it works

When a submenu is open, a pointermove listener on document tracks cursor movement (throttled via event.timeStamp comparison). For each movement, it computes angles from the cursor to the near-edge corners of the submenu overlay. If the cursor's movement vector falls within that angular cone (plus tolerance), the submenu stays open. Only after 2 consecutive "miss" movements does the controller allow the switch.

Key design decisions

  • Lazy activation — listener only attached while a submenu is open, zero overhead otherwise
  • Touch-safe — skipped entirely for pointerType === 'touch' or 'pen'
  • Direction-agnostic — detects submenu direction from actual overlay position at runtime (handles both LTR/RTL and viewport-edge flipping)
  • Accessible — the 2-strike threshold accommodates motor impairments and hand tremors
  • JS private class members — controller internals use #private fields for encapsulation
  • safe-triangle-active attribute — reflected on the menu overlay container while the controller is active, enabling CSS-based hover highlight suppression
  • Hover highlight suppressionpointer-events: none on list-box items while safe-triangle navigation is active, preventing distracting hover flicker
  • Fallback timeout — 1-second safety net that deactivates the controller if no pointer movement is detected, preventing stuck states
  • Pending switch scheduling — when the controller blocks a submenu switch, the switch is queued and executed once the controller deactivates
  • Escape key deactivation — pressing Escape deactivates the controller and cleans up state
  • Stale container cleanup — the controller detects and handles cases where the submenu container is removed from the DOM while active

Architecture

  • SafeTriangleController in packages/context-menu/src/vaadin-safe-triangle-controller.js (224 lines)
  • Shared pointerMove test helper in packages/context-menu/test/helpers.js for simulating diagonal mouse movements across both test suites
  • Uses event.timeStamp for throttle timing instead of performance.now() for consistency with the event stream
  • Aura and Lumo theme CSS additions for [safe-triangle-active] styling on overlay list-boxes

Performance analysis

Chrome DevTools performance traces were captured before (commit b0d3dd98c8) and after (commit 76db29d76b) the change, using the same dev/menu-bar.html page and identical interaction sequences (open Share submenu → hover nested "On social media" → hover sibling "Get link" → repeat).

Page load (control — controller inactive)

Metric Before After Delta
LCP 745 ms 740 ms -5 ms (noise)
CLS 0.00 0.00 0

Interaction (hot path — controller active during submenu hover)

Metric Before After Delta
INP 40 ms 46 ms +6 ms (noise)
Long tasks (>50ms) 0 0 0

Both INP values are well within the "Good" threshold (< 200ms). The +6ms delta is evenly distributed across all INP phases (input delay, processing, presentation), consistent with measurement noise rather than a systematic regression. No long tasks were introduced in either run.

Conclusion: The SafeTriangleController introduces no measurable performance overhead.

Changes

15 files changed, 764 insertions, 19 deletions (12 commits)

Test plan

  • yarn test --group context-menu — 213 passed, 0 failed
  • yarn test --group menu-bar — 193 passed, 0 failed
  • yarn test:snapshots --group context-menu — updated
  • yarn test:snapshots --group menu-bar — updated
  • Manual testing at /dev/menu-bar.html and /dev/context-menu.html with diagonal mouse movements
  • Verify keyboard navigation is unaffected
  • Verify touch behavior is unaffected

🤖 Generated with Claude Code

@jouni
Copy link
Copy Markdown
Member

jouni commented Feb 24, 2026

The only thing that bothers me with this implementation (a little, not likely to be a blocker), is that the other items under the mouse cursor still apply the hover styles when passing over them towards the sub-menu.

@DiegoCardoso
Copy link
Copy Markdown
Contributor Author

The only thing that bothers me with this implementation (a little, not likely to be a blocker), is that the other items under the mouse cursor still apply the hover styles when passing over them towards the sub-menu.

Added some implementation to cover this. It adds an attribute to the host element and targets in CSS the not expanded items to have a transparent background.

DiegoCardoso and others added 11 commits March 20, 2026 15:47
Add SafeTriangleController that uses atan2 angle comparison to detect
when the cursor is aimed at an open submenu, preventing premature
submenu switching during diagonal mouse movement. Integrate into both
context-menu items mixin and menu-bar mixin.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…obustness

- Reset _lastX/_lastY/_hasLastPosition on activate() to prevent stale
  position data from previous submenu sessions
- Replace fragile (0,0) coordinate sentinel with _hasLastPosition boolean
  flag to handle cursor at viewport origin correctly
- Add 400ms fallback timeout in scheduleSwitch() so pending submenu
  switches fire even when the user stops moving the mouse
- Declare _pendingSwitch in constructor for consistency
- Remove redundant !isTouch guard in context-menu items mixin

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace underscore-prefixed conventions with native # private fields
and methods for true language-enforced encapsulation. Convert the bound
pointer move handler to an arrow function field, eliminating .bind().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract item reference eagerly before the safe triangle check since
composedPath() returns an empty array in async callbacks. Replace
if/return/fallthrough with if/else and remove redundant inner guard
that __showSubMenu already handles.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract duplicated pointerMove helper to context-menu test helpers
and replace inline setTimeout with aTimeout from testing-helpers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cancel pending switch in activate() to prevent stale callbacks during
rapid hover. Clear timeout in executePendingSwitch() when called from
pointermove handler. Add fallback timeout and RTL safe triangle tests
for both context-menu and menu-bar.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove keyboard navigation test from safe triangle block (already
covered elsewhere, never exercised safe triangle code). Fix both
deactivation tests to track pointer movement before closing, so
shouldKeepOpen() would return true if deactivate() were not called.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Set a `safe-triangle-active` attribute on the parent container (list-box
for context-menu, menu-bar element for menu-bar) while the safe triangle
is active. Theme CSS rules use this attribute to override hover background
to transparent on non-expanded items, preventing visual hover flicker
when moving the pointer toward an open submenu.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Deactivate the safe triangle in menu-bar's __onEscapeClose() to prevent
leaking the pointermove listener and safe-triangle-active attribute when
the submenu is closed via Escape.

Also clean up the previous container's attribute in activate() if the
container reference changes between consecutive calls.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@DiegoCardoso DiegoCardoso force-pushed the refactor/context-menu/safe-triangle branch from c7f7cb3 to e0b7159 Compare March 20, 2026 14:48
@DiegoCardoso DiegoCardoso force-pushed the refactor/context-menu/safe-triangle branch from 848c236 to 5b4a2a8 Compare March 20, 2026 15:20
Extract the submenu direction and angle comparison logic from
#onPointerMove into a dedicated #isPointerAimedAtSubmenu method,
reducing the cognitive complexity of the pointer move handler below
the SonarQube threshold of 15.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

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.

[menu-bar] keep sub-menu open when moving diagonally over other menu items with the mouse pointer

2 participants