refactor: implement safe triangle pattern for submenu navigation#11183
Draft
DiegoCardoso wants to merge 8 commits intomainfrom
Draft
refactor: implement safe triangle pattern for submenu navigation#11183DiegoCardoso wants to merge 8 commits intomainfrom
DiegoCardoso wants to merge 8 commits intomainfrom
Conversation
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>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #9904
SafeTriangleControllerthat prevents premature submenu closing when the user moves the mouse diagonally from a parent menu item toward the open submenuvaadin-context-menu(items mixin) andvaadin-menu-bar(menu bar mixin)How it works
When a submenu is open, a
pointermovelistener ondocumenttracks cursor movement (throttled to 16ms). 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:
pointerType === 'touch'or'pen'Performance analysis
Chrome DevTools performance traces were captured before (commit
b0d3dd98c8) and after (commit76db29d76b) the change, using the samedev/menu-bar.htmlpage and identical interaction sequences (open Share submenu → hover nested "On social media" → hover sibling "Get link" → repeat).Page load (control — controller inactive)
Interaction (hot path — controller active during submenu hover)
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.
Test plan
yarn test --group context-menu— 210 passed, 0 failedyarn test --group menu-bar— 180 passed, 0 failed/dev/menu-bar.htmland/dev/context-menu.htmlwith diagonal mouse movements🤖 Generated with Claude Code