From ab0ac8dc5c9088a43825f48661ad2a5cc706c087 Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Fri, 20 Feb 2026 18:21:27 +0100 Subject: [PATCH 01/13] refactor: implement safe triangle pattern for submenu navigation 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 --- .../src/vaadin-contextmenu-items-mixin.js | 27 +++ .../src/vaadin-safe-triangle-controller.d.ts | 39 ++++ .../src/vaadin-safe-triangle-controller.js | 211 ++++++++++++++++++ packages/context-menu/test/items.test.js | 102 +++++++++ .../menu-bar/src/vaadin-menu-bar-mixin.js | 25 ++- packages/menu-bar/test/sub-menu.test.js | 137 ++++++++++++ 6 files changed, 540 insertions(+), 1 deletion(-) create mode 100644 packages/context-menu/src/vaadin-safe-triangle-controller.d.ts create mode 100644 packages/context-menu/src/vaadin-safe-triangle-controller.js diff --git a/packages/context-menu/src/vaadin-contextmenu-items-mixin.js b/packages/context-menu/src/vaadin-contextmenu-items-mixin.js index 815277dff08..6a73fe58157 100644 --- a/packages/context-menu/src/vaadin-contextmenu-items-mixin.js +++ b/packages/context-menu/src/vaadin-contextmenu-items-mixin.js @@ -4,6 +4,7 @@ * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ */ import { isTouch } from '@vaadin/component-base/src/browser-utils.js'; +import { SafeTriangleController } from './vaadin-safe-triangle-controller.js'; /** * @polymerMixin @@ -163,6 +164,11 @@ export const ItemsMixin = (superClass) => }, }), ); + + // Activate safe triangle tracking for the newly opened submenu + if (this.__safeTriangle) { + this.__safeTriangle.activate(subMenuOverlay, itemElement); + } } /** @private */ @@ -263,6 +269,19 @@ export const ItemsMixin = (superClass) => return; } + // If a submenu is open and the safe triangle indicates the user is + // aiming at it, defer the switch instead of switching immediately. + if (!isTouch && this._subMenu.opened && this.__safeTriangle && this.__safeTriangle.shouldKeepOpen()) { + const item = event.composedPath().find((node) => node.localName === `${this._tagNamePrefix}-item`); + const expandedItem = this._listBox && this._listBox.querySelector('[expanded]'); + if (item && item !== expandedItem) { + this.__safeTriangle.scheduleSwitch(() => { + this.__showSubMenu(event, item); + }); + return; + } + } + this.__showSubMenu(event); }); @@ -349,6 +368,10 @@ export const ItemsMixin = (superClass) => if (expandedItem) { this.__updateExpanded(expandedItem, false); } + // Deactivate safe triangle tracking when submenu closes + if (this.__safeTriangle) { + this.__safeTriangle.deactivate(); + } } }); @@ -472,6 +495,10 @@ export const ItemsMixin = (superClass) => this._subMenu = subMenu; this.appendChild(subMenu); + if (!isTouch) { + this.__safeTriangle = new SafeTriangleController(); + } + requestAnimationFrame(() => { this.__openListenerActive = true; }); diff --git a/packages/context-menu/src/vaadin-safe-triangle-controller.d.ts b/packages/context-menu/src/vaadin-safe-triangle-controller.d.ts new file mode 100644 index 00000000000..ccf5a67654f --- /dev/null +++ b/packages/context-menu/src/vaadin-safe-triangle-controller.d.ts @@ -0,0 +1,39 @@ +/** + * @license + * Copyright (c) 2016 - 2026 Vaadin Ltd. + * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ + */ + +/** + * A controller that implements the "safe triangle" pattern for submenu navigation. + * + * When a submenu is open, moving the mouse diagonally from a parent item toward the + * submenu can cause the cursor to pass over sibling items, which would normally close + * the current submenu. This controller detects whether the cursor is aimed at the open + * submenu using atan2 angle comparison, and prevents premature submenu switching. + */ +export class SafeTriangleController { + /** + * Activate the safe triangle tracking for the given submenu overlay. + * Should be called when a submenu opens. + */ + activate(submenuOverlay: HTMLElement, parentItem: HTMLElement): void; + + /** + * Deactivate the safe triangle tracking. + * Should be called when a submenu closes. + */ + deactivate(): void; + + /** + * Check whether the submenu should be kept open based on pointer movement. + * Returns true if the user appears to be aiming at the submenu. + */ + shouldKeepOpen(): boolean; + + /** + * Schedule a deferred submenu switch. If the user moves outside the safe + * triangle before the callback fires, the callback will execute. + */ + scheduleSwitch(callback: () => void): void; +} diff --git a/packages/context-menu/src/vaadin-safe-triangle-controller.js b/packages/context-menu/src/vaadin-safe-triangle-controller.js new file mode 100644 index 00000000000..d853a800d02 --- /dev/null +++ b/packages/context-menu/src/vaadin-safe-triangle-controller.js @@ -0,0 +1,211 @@ +/** + * @license + * Copyright (c) 2016 - 2026 Vaadin Ltd. + * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ + */ + +const TOLERANCE_RAD = 15 * (Math.PI / 180); +const INVALID_THRESHOLD = 2; +const THROTTLE_MS = 16; + +/** + * A controller that implements the "safe triangle" pattern for submenu navigation. + * + * When a submenu is open, moving the mouse diagonally from a parent item toward the + * submenu can cause the cursor to pass over sibling items, which would normally close + * the current submenu. This controller detects whether the cursor is aimed at the open + * submenu using atan2 angle comparison, and prevents premature submenu switching. + * + * The approach is based on React Aria's pointer-friendly submenu implementation: + * - Computes angles from cursor position to the near corners of the submenu + * - If the cursor movement angle falls within the cone (with tolerance), the user + * is aiming at the submenu + * - Requires multiple consecutive "miss" movements before allowing a switch + * (accommodates motor impairments and tremors) + * - Only active for pointer/mouse input; ignored for touch and pen + */ +export class SafeTriangleController { + constructor() { + /** @private */ + this._lastX = 0; + /** @private */ + this._lastY = 0; + /** @private */ + this._invalidCount = 0; + /** @private */ + this._active = false; + /** @private */ + this._lastMoveTime = 0; + /** @private */ + this._submenuElement = null; + /** @private */ + this._parentItemElement = null; + /** @private */ + this._pendingTimeout = null; + /** @private */ + this._boundOnPointerMove = this._onPointerMove.bind(this); + } + + /** + * Activate the safe triangle tracking for the given submenu overlay. + * Should be called when a submenu opens. + * + * @param {HTMLElement} submenuOverlay - The submenu overlay element + * @param {HTMLElement} parentItem - The parent menu item that triggered the submenu + */ + activate(submenuOverlay, parentItem) { + this._submenuElement = submenuOverlay; + this._parentItemElement = parentItem; + this._invalidCount = 0; + this._lastMoveTime = 0; + + if (!this._active) { + this._active = true; + document.addEventListener('pointermove', this._boundOnPointerMove); + } + } + + /** + * Deactivate the safe triangle tracking. + * Should be called when a submenu closes. + */ + deactivate() { + if (this._active) { + this._active = false; + document.removeEventListener('pointermove', this._boundOnPointerMove); + } + this._submenuElement = null; + this._parentItemElement = null; + this._invalidCount = 0; + this._cancelPendingSwitch(); + } + + /** + * Check whether the submenu should be kept open based on pointer movement. + * Returns true if the user appears to be aiming at the submenu. + * + * @return {boolean} + */ + shouldKeepOpen() { + if (!this._active || !this._submenuElement) { + return false; + } + // Only block switches if we've actually tracked pointer movement. + // Without movement data, we can't determine intent. + if (this._lastMoveTime === 0) { + return false; + } + return this._invalidCount < INVALID_THRESHOLD; + } + + /** + * Schedule a deferred submenu switch. If the user moves outside the safe + * triangle before the callback fires, the callback will execute. + * + * @param {Function} callback - The function to call when the switch should happen + */ + scheduleSwitch(callback) { + this._cancelPendingSwitch(); + this._pendingSwitch = callback; + } + + /** @private */ + _cancelPendingSwitch() { + this._pendingSwitch = null; + if (this._pendingTimeout) { + clearTimeout(this._pendingTimeout); + this._pendingTimeout = null; + } + } + + /** @private */ + _executePendingSwitch() { + const callback = this._pendingSwitch; + this._pendingSwitch = null; + this._pendingTimeout = null; + if (callback) { + callback(); + } + } + + /** @private */ + _onPointerMove(event) { + // Only handle mouse pointer, not touch or pen + if (event.pointerType === 'touch' || event.pointerType === 'pen') { + return; + } + + const now = performance.now(); + if (now - this._lastMoveTime < THROTTLE_MS) { + return; + } + this._lastMoveTime = now; + + const x = event.clientX; + const y = event.clientY; + + if (this._lastX === 0 && this._lastY === 0) { + this._lastX = x; + this._lastY = y; + return; + } + + if (!this._submenuElement) { + this._lastX = x; + this._lastY = y; + return; + } + + const submenuRect = this._submenuElement.$.overlay.getBoundingClientRect(); + + // Skip if submenu is not visible + if (submenuRect.width === 0 || submenuRect.height === 0) { + this._lastX = x; + this._lastY = y; + return; + } + + // Determine submenu direction from actual position, not RTL flag + const parentRect = this._parentItemElement.getBoundingClientRect(); + const submenuIsRight = submenuRect.left >= parentRect.left; + + const dx = x - this._lastX; + + // Early exit: moving horizontally away from the submenu + if ((submenuIsRight && dx < -1) || (!submenuIsRight && dx > 1)) { + this._invalidCount += 1; + } else { + // Compute the near edge corners of the submenu + const nearX = submenuIsRight ? submenuRect.left : submenuRect.right; + const topY = submenuRect.top; + const bottomY = submenuRect.bottom; + + // Angle from previous cursor position to the two submenu corners + const thetaTop = Math.atan2(topY - this._lastY, nearX - this._lastX); + const thetaBottom = Math.atan2(bottomY - this._lastY, nearX - this._lastX); + + // Angle of cursor movement vector + const dy = y - this._lastY; + const thetaPointer = Math.atan2(dy, dx); + + // Determine the angular bounds (top and bottom may swap depending on direction) + const minAngle = Math.min(thetaTop, thetaBottom); + const maxAngle = Math.max(thetaTop, thetaBottom); + + if (thetaPointer >= minAngle - TOLERANCE_RAD && thetaPointer <= maxAngle + TOLERANCE_RAD) { + // Cursor is aimed at the submenu + this._invalidCount = 0; + } else { + this._invalidCount += 1; + } + } + + this._lastX = x; + this._lastY = y; + + // If the user has moved outside the safe triangle enough times, execute pending switch + if (this._invalidCount >= INVALID_THRESHOLD && this._pendingSwitch) { + this._executePendingSwitch(); + } + } +} diff --git a/packages/context-menu/test/items.test.js b/packages/context-menu/test/items.test.js index d24dfe6f3f9..4f000db8950 100644 --- a/packages/context-menu/test/items.test.js +++ b/packages/context-menu/test/items.test.js @@ -730,4 +730,106 @@ describe('items', () => { expect(getMenuItems(rootMenu)[1].hasAttribute('focused')).to.be.true; }); }); + + (isTouch ? describe.skip : describe)('safe triangle', () => { + function pointerMove(x, y) { + document.dispatchEvent( + new PointerEvent('pointermove', { + clientX: x, + clientY: y, + bubbles: true, + pointerType: 'mouse', + }), + ); + } + + it('should keep submenu open when pointer moves diagonally toward it', async () => { + const parentItem = getMenuItems(rootMenu)[0]; + const parentRect = parentItem.getBoundingClientRect(); + const subMenuOverlay = subMenu._overlayElement; + const subMenuRect = subMenuOverlay.getBoundingClientRect(); + + // Start from the center of the parent item + const startX = parentRect.left + parentRect.width / 2; + const startY = parentRect.top + parentRect.height / 2; + + // Simulate pointer movement toward the submenu (diagonally) + pointerMove(startX, startY); + + // Wait for throttle + await new Promise((resolve) => { + setTimeout(resolve, 20); + }); + + // Move diagonally toward the submenu + const targetX = subMenuRect.left + subMenuRect.width / 2; + const targetY = subMenuRect.top + subMenuRect.height / 2; + const midX = (startX + targetX) / 2; + const midY = (startY + targetY) / 2; + pointerMove(midX, midY); + + await new Promise((resolve) => { + setTimeout(resolve, 20); + }); + + // Now hover over a sibling item — submenu should stay open + const siblingItem = getMenuItems(rootMenu)[3]; + activateItem(siblingItem); + + expect(subMenu.opened).to.be.true; + expect(getMenuItems(subMenu)[0].textContent).to.equal('foo-0-0'); + }); + + it('should switch submenu when pointer moves away from it', async () => { + const parentItem = getMenuItems(rootMenu)[0]; + const parentRect = parentItem.getBoundingClientRect(); + + // Start from the center of the parent item + const startX = parentRect.left + parentRect.width / 2; + const startY = parentRect.top + parentRect.height / 2; + + // Simulate pointer movement away from the submenu (moving left) + pointerMove(startX, startY); + + await new Promise((resolve) => { + setTimeout(resolve, 20); + }); + + // Move away (to the left, opposite of submenu) + pointerMove(startX - 50, startY); + + await new Promise((resolve) => { + setTimeout(resolve, 20); + }); + + // Move away again to exceed the threshold + pointerMove(startX - 100, startY); + + await new Promise((resolve) => { + setTimeout(resolve, 20); + }); + + // Now hover over the other parent item — should switch + activateItem(getMenuItems(rootMenu)[3]); + expect(getMenuItems(subMenu)[0].textContent).to.equal('foo-3-0'); + }); + + it('should not interfere with keyboard navigation', async () => { + // Arrow right on an item with children should open nested submenu + const items = getMenuItems(subMenu); + const itemWithChildren = items[2]; // foo-0-2 has children + itemWithChildren.focus(); + arrowRightKeyDown(itemWithChildren); + const subMenu2 = getSubMenu(subMenu); + await nextRender(); + expect(subMenu2.opened).to.be.true; + }); + + it('should deactivate when submenu closes', () => { + const safeTriangle = rootMenu.querySelector('[slot="submenu"]').__safeTriangle; + expect(safeTriangle).to.exist; + subMenu.close(); + expect(safeTriangle.shouldKeepOpen()).to.be.false; + }); + }); }); diff --git a/packages/menu-bar/src/vaadin-menu-bar-mixin.js b/packages/menu-bar/src/vaadin-menu-bar-mixin.js index 9f38f34a427..9ae1887fc54 100644 --- a/packages/menu-bar/src/vaadin-menu-bar-mixin.js +++ b/packages/menu-bar/src/vaadin-menu-bar-mixin.js @@ -11,10 +11,12 @@ import { FocusMixin } from '@vaadin/a11y-base/src/focus-mixin.js'; import { isElementFocused, isElementHidden, isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js'; import { KeyboardDirectionMixin } from '@vaadin/a11y-base/src/keyboard-direction-mixin.js'; import { microTask } from '@vaadin/component-base/src/async.js'; +import { isTouch } from '@vaadin/component-base/src/browser-utils.js'; import { Debouncer } from '@vaadin/component-base/src/debounce.js'; import { I18nMixin } from '@vaadin/component-base/src/i18n-mixin.js'; import { ResizeMixin } from '@vaadin/component-base/src/resize-mixin.js'; import { SlotController } from '@vaadin/component-base/src/slot-controller.js'; +import { SafeTriangleController } from '@vaadin/context-menu/src/vaadin-safe-triangle-controller.js'; /** * Custom Lit directive for rendering item components @@ -314,6 +316,10 @@ export const MenuBarMixin = (superClass) => this.addEventListener('mousedown', () => this._hideTooltip(true)); this.addEventListener('mouseleave', () => this._hideTooltip()); + if (!isTouch) { + this.__safeTriangle = new SafeTriangleController(); + } + this._container = this.shadowRoot.querySelector('[part="container"]'); } @@ -907,7 +913,15 @@ export const MenuBarMixin = (superClass) => // with children, regardless of whether openOnHover is set. // If the button has no children, keep the sub-menu opened. if (button.item.children && (this.openOnHover || this._subMenu.opened)) { - this.__openSubMenu(button, false); + // If a submenu is open and the safe triangle indicates the user is + // aiming at it, defer the switch instead of switching immediately. + if (this._subMenu.opened && this.__safeTriangle && this.__safeTriangle.shouldKeepOpen()) { + this.__safeTriangle.scheduleSwitch(() => { + this.__openSubMenu(button, false); + }); + } else { + this.__openSubMenu(button, false); + } } if (button === this._overflow || (this.openOnHover && button.item.children)) { @@ -995,6 +1009,11 @@ export const MenuBarMixin = (superClass) => }), ); + // Activate safe triangle tracking for the newly opened submenu + if (this.__safeTriangle) { + this.__safeTriangle.activate(overlay, button); + } + overlay.addEventListener( 'vaadin-overlay-open', () => { @@ -1065,6 +1084,10 @@ export const MenuBarMixin = (superClass) => if (this._subMenu.opened) { this._subMenu.close(); } + // Deactivate safe triangle tracking when submenu closes + if (this.__safeTriangle) { + this.__safeTriangle.deactivate(); + } } /** diff --git a/packages/menu-bar/test/sub-menu.test.js b/packages/menu-bar/test/sub-menu.test.js index 20b3dbe1493..b489c0c0dc8 100644 --- a/packages/menu-bar/test/sub-menu.test.js +++ b/packages/menu-bar/test/sub-menu.test.js @@ -736,3 +736,140 @@ describe('touch', () => { }); }); }); + +(isTouch ? describe.skip : describe)('safe triangle', () => { + let menu, buttons, subMenu; + + function pointerMove(x, y) { + document.dispatchEvent( + new PointerEvent('pointermove', { + clientX: x, + clientY: y, + bubbles: true, + pointerType: 'mouse', + }), + ); + } + + beforeEach(async () => { + menu = fixtureSync(''); + menu.items = [ + { + text: 'Menu Item 1', + children: [{ text: 'Menu Item 1 1' }, { text: 'Menu Item 1 2' }], + }, + { text: 'Menu Item 2' }, + { + text: 'Menu Item 3', + children: [{ text: 'Menu Item 3 1' }, { text: 'Menu Item 3 2' }], + }, + ]; + menu.openOnHover = true; + await nextRender(); + subMenu = menu._subMenu; + buttons = menu._buttons; + }); + + it('should keep submenu open when pointer moves toward it', async () => { + // Open submenu for button 0 + fire(buttons[0], 'mouseover'); + await nextRender(); + expect(subMenu.opened).to.be.true; + + const btnRect = buttons[0].getBoundingClientRect(); + const overlayRect = subMenu._overlayElement.getBoundingClientRect(); + + // Start from center of expanded button + const startX = btnRect.left + btnRect.width / 2; + const startY = btnRect.top + btnRect.height / 2; + pointerMove(startX, startY); + + await new Promise((resolve) => { + setTimeout(resolve, 20); + }); + + // Move pointer toward the submenu overlay (downward toward it) + const targetX = overlayRect.left + overlayRect.width / 2; + const targetY = overlayRect.top + overlayRect.height / 2; + const midX = (startX + targetX) / 2; + const midY = (startY + targetY) / 2; + pointerMove(midX, midY); + + await new Promise((resolve) => { + setTimeout(resolve, 20); + }); + + // Hover over another button — should NOT switch due to safe triangle + fire(buttons[2], 'mouseover'); + await nextRender(); + + expect(subMenu.opened).to.be.true; + expect(subMenu.listenOn).to.equal(buttons[0]); + }); + + it('should switch submenu when pointer moves away', async () => { + // Open submenu for button 0 + fire(buttons[0], 'mouseover'); + await nextRender(); + expect(subMenu.opened).to.be.true; + + const btnRect = buttons[0].getBoundingClientRect(); + + // Start from center of expanded button + const startX = btnRect.left + btnRect.width / 2; + const startY = btnRect.top + btnRect.height / 2; + pointerMove(startX, startY); + + await new Promise((resolve) => { + setTimeout(resolve, 20); + }); + + // Move pointer upward (away from submenu which opens below) + pointerMove(startX, startY - 50); + + await new Promise((resolve) => { + setTimeout(resolve, 20); + }); + + // Move again to exceed threshold + pointerMove(startX, startY - 100); + + await new Promise((resolve) => { + setTimeout(resolve, 20); + }); + + // Hover over another button — should switch + fire(buttons[2], 'mouseover'); + await nextRender(); + + expect(subMenu.opened).to.be.true; + expect(subMenu.listenOn).to.equal(buttons[2]); + }); + + it('should deactivate safe triangle when submenu closes', async () => { + fire(buttons[0], 'mouseover'); + await nextRender(); + expect(subMenu.opened).to.be.true; + + const safeTriangle = menu.__safeTriangle; + expect(safeTriangle).to.exist; + + menu.close(); + await nextRender(); + + expect(safeTriangle.shouldKeepOpen()).to.be.false; + }); + + it('should not interfere when no pointer movement is tracked', async () => { + fire(buttons[0], 'mouseover'); + await nextRender(); + expect(subMenu.opened).to.be.true; + + // Without any pointermove events, hovering another button should switch immediately + fire(buttons[2], 'mouseover'); + await nextRender(); + + expect(subMenu.opened).to.be.true; + expect(subMenu.listenOn).to.equal(buttons[2]); + }); +}); From 6d020a4eba0a8aed3b1f2cebb6a75319ed7bd5e7 Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Fri, 20 Feb 2026 19:08:18 +0100 Subject: [PATCH 02/13] fix: safe triangle controller reset, fallback timeout, and sentinel robustness - 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 --- .../src/vaadin-contextmenu-items-mixin.js | 2 +- .../src/vaadin-safe-triangle-controller.js | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/context-menu/src/vaadin-contextmenu-items-mixin.js b/packages/context-menu/src/vaadin-contextmenu-items-mixin.js index 6a73fe58157..8fbb15f2c9a 100644 --- a/packages/context-menu/src/vaadin-contextmenu-items-mixin.js +++ b/packages/context-menu/src/vaadin-contextmenu-items-mixin.js @@ -271,7 +271,7 @@ export const ItemsMixin = (superClass) => // If a submenu is open and the safe triangle indicates the user is // aiming at it, defer the switch instead of switching immediately. - if (!isTouch && this._subMenu.opened && this.__safeTriangle && this.__safeTriangle.shouldKeepOpen()) { + if (this._subMenu.opened && this.__safeTriangle && this.__safeTriangle.shouldKeepOpen()) { const item = event.composedPath().find((node) => node.localName === `${this._tagNamePrefix}-item`); const expandedItem = this._listBox && this._listBox.querySelector('[expanded]'); if (item && item !== expandedItem) { diff --git a/packages/context-menu/src/vaadin-safe-triangle-controller.js b/packages/context-menu/src/vaadin-safe-triangle-controller.js index d853a800d02..179ca5df300 100644 --- a/packages/context-menu/src/vaadin-safe-triangle-controller.js +++ b/packages/context-menu/src/vaadin-safe-triangle-controller.js @@ -7,6 +7,7 @@ const TOLERANCE_RAD = 15 * (Math.PI / 180); const INVALID_THRESHOLD = 2; const THROTTLE_MS = 16; +const FALLBACK_TIMEOUT_MS = 400; /** * A controller that implements the "safe triangle" pattern for submenu navigation. @@ -26,6 +27,8 @@ const THROTTLE_MS = 16; */ export class SafeTriangleController { constructor() { + /** @private */ + this._hasLastPosition = false; /** @private */ this._lastX = 0; /** @private */ @@ -41,6 +44,8 @@ export class SafeTriangleController { /** @private */ this._parentItemElement = null; /** @private */ + this._pendingSwitch = null; + /** @private */ this._pendingTimeout = null; /** @private */ this._boundOnPointerMove = this._onPointerMove.bind(this); @@ -58,6 +63,9 @@ export class SafeTriangleController { this._parentItemElement = parentItem; this._invalidCount = 0; this._lastMoveTime = 0; + this._hasLastPosition = false; + this._lastX = 0; + this._lastY = 0; if (!this._active) { this._active = true; @@ -107,6 +115,11 @@ export class SafeTriangleController { scheduleSwitch(callback) { this._cancelPendingSwitch(); this._pendingSwitch = callback; + // Fallback: if the user stops moving entirely, execute the switch + // after a timeout so the submenu doesn't stay stuck indefinitely. + this._pendingTimeout = setTimeout(() => { + this._executePendingSwitch(); + }, FALLBACK_TIMEOUT_MS); } /** @private */ @@ -144,7 +157,8 @@ export class SafeTriangleController { const x = event.clientX; const y = event.clientY; - if (this._lastX === 0 && this._lastY === 0) { + if (!this._hasLastPosition) { + this._hasLastPosition = true; this._lastX = x; this._lastY = y; return; From a6a6e576b2448d8dec3c9100acdf9d98dd376a8b Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Fri, 20 Feb 2026 19:13:24 +0100 Subject: [PATCH 03/13] refactor: use JS private class members in SafeTriangleController 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 --- .../src/vaadin-safe-triangle-controller.js | 260 +++++++++--------- 1 file changed, 126 insertions(+), 134 deletions(-) diff --git a/packages/context-menu/src/vaadin-safe-triangle-controller.js b/packages/context-menu/src/vaadin-safe-triangle-controller.js index 179ca5df300..28e4bd12ee6 100644 --- a/packages/context-menu/src/vaadin-safe-triangle-controller.js +++ b/packages/context-menu/src/vaadin-safe-triangle-controller.js @@ -26,168 +26,72 @@ const FALLBACK_TIMEOUT_MS = 400; * - Only active for pointer/mouse input; ignored for touch and pen */ export class SafeTriangleController { - constructor() { - /** @private */ - this._hasLastPosition = false; - /** @private */ - this._lastX = 0; - /** @private */ - this._lastY = 0; - /** @private */ - this._invalidCount = 0; - /** @private */ - this._active = false; - /** @private */ - this._lastMoveTime = 0; - /** @private */ - this._submenuElement = null; - /** @private */ - this._parentItemElement = null; - /** @private */ - this._pendingSwitch = null; - /** @private */ - this._pendingTimeout = null; - /** @private */ - this._boundOnPointerMove = this._onPointerMove.bind(this); - } + #hasLastPosition = false; - /** - * Activate the safe triangle tracking for the given submenu overlay. - * Should be called when a submenu opens. - * - * @param {HTMLElement} submenuOverlay - The submenu overlay element - * @param {HTMLElement} parentItem - The parent menu item that triggered the submenu - */ - activate(submenuOverlay, parentItem) { - this._submenuElement = submenuOverlay; - this._parentItemElement = parentItem; - this._invalidCount = 0; - this._lastMoveTime = 0; - this._hasLastPosition = false; - this._lastX = 0; - this._lastY = 0; - - if (!this._active) { - this._active = true; - document.addEventListener('pointermove', this._boundOnPointerMove); - } - } + #lastX = 0; - /** - * Deactivate the safe triangle tracking. - * Should be called when a submenu closes. - */ - deactivate() { - if (this._active) { - this._active = false; - document.removeEventListener('pointermove', this._boundOnPointerMove); - } - this._submenuElement = null; - this._parentItemElement = null; - this._invalidCount = 0; - this._cancelPendingSwitch(); - } + #lastY = 0; - /** - * Check whether the submenu should be kept open based on pointer movement. - * Returns true if the user appears to be aiming at the submenu. - * - * @return {boolean} - */ - shouldKeepOpen() { - if (!this._active || !this._submenuElement) { - return false; - } - // Only block switches if we've actually tracked pointer movement. - // Without movement data, we can't determine intent. - if (this._lastMoveTime === 0) { - return false; - } - return this._invalidCount < INVALID_THRESHOLD; - } + #invalidCount = 0; - /** - * Schedule a deferred submenu switch. If the user moves outside the safe - * triangle before the callback fires, the callback will execute. - * - * @param {Function} callback - The function to call when the switch should happen - */ - scheduleSwitch(callback) { - this._cancelPendingSwitch(); - this._pendingSwitch = callback; - // Fallback: if the user stops moving entirely, execute the switch - // after a timeout so the submenu doesn't stay stuck indefinitely. - this._pendingTimeout = setTimeout(() => { - this._executePendingSwitch(); - }, FALLBACK_TIMEOUT_MS); - } + #active = false; - /** @private */ - _cancelPendingSwitch() { - this._pendingSwitch = null; - if (this._pendingTimeout) { - clearTimeout(this._pendingTimeout); - this._pendingTimeout = null; - } - } + #lastMoveTime = 0; - /** @private */ - _executePendingSwitch() { - const callback = this._pendingSwitch; - this._pendingSwitch = null; - this._pendingTimeout = null; - if (callback) { - callback(); - } - } + #submenuElement = null; + + #parentItemElement = null; - /** @private */ - _onPointerMove(event) { + #pendingSwitch = null; + + #pendingTimeout = null; + + #onPointerMove = (event) => { // Only handle mouse pointer, not touch or pen if (event.pointerType === 'touch' || event.pointerType === 'pen') { return; } const now = performance.now(); - if (now - this._lastMoveTime < THROTTLE_MS) { + if (now - this.#lastMoveTime < THROTTLE_MS) { return; } - this._lastMoveTime = now; + this.#lastMoveTime = now; const x = event.clientX; const y = event.clientY; - if (!this._hasLastPosition) { - this._hasLastPosition = true; - this._lastX = x; - this._lastY = y; + if (!this.#hasLastPosition) { + this.#hasLastPosition = true; + this.#lastX = x; + this.#lastY = y; return; } - if (!this._submenuElement) { - this._lastX = x; - this._lastY = y; + if (!this.#submenuElement) { + this.#lastX = x; + this.#lastY = y; return; } - const submenuRect = this._submenuElement.$.overlay.getBoundingClientRect(); + const submenuRect = this.#submenuElement.$.overlay.getBoundingClientRect(); // Skip if submenu is not visible if (submenuRect.width === 0 || submenuRect.height === 0) { - this._lastX = x; - this._lastY = y; + this.#lastX = x; + this.#lastY = y; return; } // Determine submenu direction from actual position, not RTL flag - const parentRect = this._parentItemElement.getBoundingClientRect(); + const parentRect = this.#parentItemElement.getBoundingClientRect(); const submenuIsRight = submenuRect.left >= parentRect.left; - const dx = x - this._lastX; + const dx = x - this.#lastX; // Early exit: moving horizontally away from the submenu if ((submenuIsRight && dx < -1) || (!submenuIsRight && dx > 1)) { - this._invalidCount += 1; + this.#invalidCount += 1; } else { // Compute the near edge corners of the submenu const nearX = submenuIsRight ? submenuRect.left : submenuRect.right; @@ -195,11 +99,11 @@ export class SafeTriangleController { const bottomY = submenuRect.bottom; // Angle from previous cursor position to the two submenu corners - const thetaTop = Math.atan2(topY - this._lastY, nearX - this._lastX); - const thetaBottom = Math.atan2(bottomY - this._lastY, nearX - this._lastX); + const thetaTop = Math.atan2(topY - this.#lastY, nearX - this.#lastX); + const thetaBottom = Math.atan2(bottomY - this.#lastY, nearX - this.#lastX); // Angle of cursor movement vector - const dy = y - this._lastY; + const dy = y - this.#lastY; const thetaPointer = Math.atan2(dy, dx); // Determine the angular bounds (top and bottom may swap depending on direction) @@ -208,18 +112,106 @@ export class SafeTriangleController { if (thetaPointer >= minAngle - TOLERANCE_RAD && thetaPointer <= maxAngle + TOLERANCE_RAD) { // Cursor is aimed at the submenu - this._invalidCount = 0; + this.#invalidCount = 0; } else { - this._invalidCount += 1; + this.#invalidCount += 1; } } - this._lastX = x; - this._lastY = y; + this.#lastX = x; + this.#lastY = y; // If the user has moved outside the safe triangle enough times, execute pending switch - if (this._invalidCount >= INVALID_THRESHOLD && this._pendingSwitch) { - this._executePendingSwitch(); + if (this.#invalidCount >= INVALID_THRESHOLD && this.#pendingSwitch) { + this.#executePendingSwitch(); + } + }; + + /** + * Activate the safe triangle tracking for the given submenu overlay. + * Should be called when a submenu opens. + * + * @param {HTMLElement} submenuOverlay - The submenu overlay element + * @param {HTMLElement} parentItem - The parent menu item that triggered the submenu + */ + activate(submenuOverlay, parentItem) { + this.#submenuElement = submenuOverlay; + this.#parentItemElement = parentItem; + this.#invalidCount = 0; + this.#lastMoveTime = 0; + this.#hasLastPosition = false; + this.#lastX = 0; + this.#lastY = 0; + + if (!this.#active) { + this.#active = true; + document.addEventListener('pointermove', this.#onPointerMove); + } + } + + /** + * Deactivate the safe triangle tracking. + * Should be called when a submenu closes. + */ + deactivate() { + if (this.#active) { + this.#active = false; + document.removeEventListener('pointermove', this.#onPointerMove); + } + this.#submenuElement = null; + this.#parentItemElement = null; + this.#invalidCount = 0; + this.#cancelPendingSwitch(); + } + + /** + * Check whether the submenu should be kept open based on pointer movement. + * Returns true if the user appears to be aiming at the submenu. + * + * @return {boolean} + */ + shouldKeepOpen() { + if (!this.#active || !this.#submenuElement) { + return false; + } + // Only block switches if we've actually tracked pointer movement. + // Without movement data, we can't determine intent. + if (this.#lastMoveTime === 0) { + return false; + } + return this.#invalidCount < INVALID_THRESHOLD; + } + + /** + * Schedule a deferred submenu switch. If the user moves outside the safe + * triangle before the callback fires, the callback will execute. + * + * @param {Function} callback - The function to call when the switch should happen + */ + scheduleSwitch(callback) { + this.#cancelPendingSwitch(); + this.#pendingSwitch = callback; + // Fallback: if the user stops moving entirely, execute the switch + // after a timeout so the submenu doesn't stay stuck indefinitely. + this.#pendingTimeout = setTimeout(() => { + this.#executePendingSwitch(); + }, FALLBACK_TIMEOUT_MS); + } + + #cancelPendingSwitch() { + this.#pendingSwitch = null; + if (this.#pendingTimeout) { + clearTimeout(this.#pendingTimeout); + this.#pendingTimeout = null; + } + } + + #executePendingSwitch() { + const callback = this.#pendingSwitch; + this.#pendingSwitch = null; + this.#pendingTimeout = null; + if (callback) { + callback(); } } } From 00a962f3c2f7cc655a73c0a2ad094e59ab6a0064 Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Fri, 20 Feb 2026 19:38:48 +0100 Subject: [PATCH 04/13] refactor: align context-menu safe triangle pattern with menu-bar 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 --- .../src/vaadin-contextmenu-items-mixin.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/context-menu/src/vaadin-contextmenu-items-mixin.js b/packages/context-menu/src/vaadin-contextmenu-items-mixin.js index 8fbb15f2c9a..ea1fbbb677c 100644 --- a/packages/context-menu/src/vaadin-contextmenu-items-mixin.js +++ b/packages/context-menu/src/vaadin-contextmenu-items-mixin.js @@ -269,20 +269,18 @@ export const ItemsMixin = (superClass) => return; } + // Extract item reference eagerly since composedPath() is only valid synchronously + const item = event.composedPath().find((node) => node.localName === `${this._tagNamePrefix}-item`); + // If a submenu is open and the safe triangle indicates the user is // aiming at it, defer the switch instead of switching immediately. if (this._subMenu.opened && this.__safeTriangle && this.__safeTriangle.shouldKeepOpen()) { - const item = event.composedPath().find((node) => node.localName === `${this._tagNamePrefix}-item`); - const expandedItem = this._listBox && this._listBox.querySelector('[expanded]'); - if (item && item !== expandedItem) { - this.__safeTriangle.scheduleSwitch(() => { - this.__showSubMenu(event, item); - }); - return; - } + this.__safeTriangle.scheduleSwitch(() => { + this.__showSubMenu(event, item); + }); + } else { + this.__showSubMenu(event, item); } - - this.__showSubMenu(event); }); overlay.addEventListener('keydown', (event) => { From 9ad07acf6afa7d7bdaa5dbac8f5f110f3e645840 Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Fri, 20 Feb 2026 19:48:42 +0100 Subject: [PATCH 05/13] refactor: clean up safe triangle tests 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 --- packages/context-menu/test/helpers.js | 11 ++++++++ packages/context-menu/test/items.test.js | 34 +++++------------------- packages/menu-bar/test/sub-menu.test.js | 33 +++++------------------ 3 files changed, 25 insertions(+), 53 deletions(-) diff --git a/packages/context-menu/test/helpers.js b/packages/context-menu/test/helpers.js index c3f7893e89d..91defc0a832 100644 --- a/packages/context-menu/test/helpers.js +++ b/packages/context-menu/test/helpers.js @@ -38,6 +38,17 @@ export function getSubMenu(menu) { return menu.querySelector(':scope > vaadin-context-menu[slot="submenu"]'); } +export function pointerMove(x, y) { + document.dispatchEvent( + new PointerEvent('pointermove', { + clientX: x, + clientY: y, + bubbles: true, + pointerType: 'mouse', + }), + ); +} + export async function openSubMenus(menu) { await oneEvent(menu._overlayElement, 'vaadin-overlay-open'); const itemElement = menu.querySelector(':scope > [slot="overlay"] [aria-haspopup="true"]'); diff --git a/packages/context-menu/test/items.test.js b/packages/context-menu/test/items.test.js index 4f000db8950..4408bff3701 100644 --- a/packages/context-menu/test/items.test.js +++ b/packages/context-menu/test/items.test.js @@ -4,6 +4,7 @@ import { arrowLeftKeyDown, arrowRightKeyDown, arrowUpKeyDown, + aTimeout, enterKeyDown, escKeyDown, fire, @@ -18,7 +19,7 @@ import '../src/vaadin-context-menu.js'; import '@vaadin/item/src/vaadin-item.js'; import '@vaadin/list-box/src/vaadin-list-box.js'; import { isTouch } from '@vaadin/component-base/src/browser-utils.js'; -import { activateItem, getMenuItems, getSubMenu, openMenu } from './helpers.js'; +import { activateItem, getMenuItems, getSubMenu, openMenu, pointerMove } from './helpers.js'; describe('items', () => { let rootMenu, subMenu, target, rootOverlay, subOverlay1; @@ -732,17 +733,6 @@ describe('items', () => { }); (isTouch ? describe.skip : describe)('safe triangle', () => { - function pointerMove(x, y) { - document.dispatchEvent( - new PointerEvent('pointermove', { - clientX: x, - clientY: y, - bubbles: true, - pointerType: 'mouse', - }), - ); - } - it('should keep submenu open when pointer moves diagonally toward it', async () => { const parentItem = getMenuItems(rootMenu)[0]; const parentRect = parentItem.getBoundingClientRect(); @@ -757,9 +747,7 @@ describe('items', () => { pointerMove(startX, startY); // Wait for throttle - await new Promise((resolve) => { - setTimeout(resolve, 20); - }); + await aTimeout(20); // Move diagonally toward the submenu const targetX = subMenuRect.left + subMenuRect.width / 2; @@ -768,9 +756,7 @@ describe('items', () => { const midY = (startY + targetY) / 2; pointerMove(midX, midY); - await new Promise((resolve) => { - setTimeout(resolve, 20); - }); + await aTimeout(20); // Now hover over a sibling item — submenu should stay open const siblingItem = getMenuItems(rootMenu)[3]; @@ -791,23 +777,17 @@ describe('items', () => { // Simulate pointer movement away from the submenu (moving left) pointerMove(startX, startY); - await new Promise((resolve) => { - setTimeout(resolve, 20); - }); + await aTimeout(20); // Move away (to the left, opposite of submenu) pointerMove(startX - 50, startY); - await new Promise((resolve) => { - setTimeout(resolve, 20); - }); + await aTimeout(20); // Move away again to exceed the threshold pointerMove(startX - 100, startY); - await new Promise((resolve) => { - setTimeout(resolve, 20); - }); + await aTimeout(20); // Now hover over the other parent item — should switch activateItem(getMenuItems(rootMenu)[3]); diff --git a/packages/menu-bar/test/sub-menu.test.js b/packages/menu-bar/test/sub-menu.test.js index b489c0c0dc8..bb73bad4473 100644 --- a/packages/menu-bar/test/sub-menu.test.js +++ b/packages/menu-bar/test/sub-menu.test.js @@ -3,6 +3,7 @@ import { arrowDown, arrowLeft, arrowUp, + aTimeout, click, enter, esc, @@ -19,6 +20,7 @@ import { import sinon from 'sinon'; import '../src/vaadin-menu-bar.js'; import { isTouch } from '@vaadin/component-base/src/browser-utils.js'; +import { pointerMove } from '@vaadin/context-menu/test/helpers.js'; const menuOpenEvent = isTouch ? 'click' : 'mouseover'; @@ -740,17 +742,6 @@ describe('touch', () => { (isTouch ? describe.skip : describe)('safe triangle', () => { let menu, buttons, subMenu; - function pointerMove(x, y) { - document.dispatchEvent( - new PointerEvent('pointermove', { - clientX: x, - clientY: y, - bubbles: true, - pointerType: 'mouse', - }), - ); - } - beforeEach(async () => { menu = fixtureSync(''); menu.items = [ @@ -784,9 +775,7 @@ describe('touch', () => { const startY = btnRect.top + btnRect.height / 2; pointerMove(startX, startY); - await new Promise((resolve) => { - setTimeout(resolve, 20); - }); + await aTimeout(20); // Move pointer toward the submenu overlay (downward toward it) const targetX = overlayRect.left + overlayRect.width / 2; @@ -795,9 +784,7 @@ describe('touch', () => { const midY = (startY + targetY) / 2; pointerMove(midX, midY); - await new Promise((resolve) => { - setTimeout(resolve, 20); - }); + await aTimeout(20); // Hover over another button — should NOT switch due to safe triangle fire(buttons[2], 'mouseover'); @@ -820,23 +807,17 @@ describe('touch', () => { const startY = btnRect.top + btnRect.height / 2; pointerMove(startX, startY); - await new Promise((resolve) => { - setTimeout(resolve, 20); - }); + await aTimeout(20); // Move pointer upward (away from submenu which opens below) pointerMove(startX, startY - 50); - await new Promise((resolve) => { - setTimeout(resolve, 20); - }); + await aTimeout(20); // Move again to exceed threshold pointerMove(startX, startY - 100); - await new Promise((resolve) => { - setTimeout(resolve, 20); - }); + await aTimeout(20); // Hover over another button — should switch fire(buttons[2], 'mouseover'); From 276edfa1b69ff0941489e30bdef81a980daa502c Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Mon, 23 Feb 2026 18:44:24 +0100 Subject: [PATCH 06/13] fix: cancel stale pending switch on activate, add RTL and timeout tests 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 --- .../src/vaadin-safe-triangle-controller.js | 2 + packages/context-menu/test/items.test.js | 80 +++++++++++++++ packages/menu-bar/test/sub-menu.test.js | 97 ++++++++++++++++++- 3 files changed, 178 insertions(+), 1 deletion(-) diff --git a/packages/context-menu/src/vaadin-safe-triangle-controller.js b/packages/context-menu/src/vaadin-safe-triangle-controller.js index 28e4bd12ee6..55e50d8330a 100644 --- a/packages/context-menu/src/vaadin-safe-triangle-controller.js +++ b/packages/context-menu/src/vaadin-safe-triangle-controller.js @@ -135,6 +135,7 @@ export class SafeTriangleController { * @param {HTMLElement} parentItem - The parent menu item that triggered the submenu */ activate(submenuOverlay, parentItem) { + this.#cancelPendingSwitch(); this.#submenuElement = submenuOverlay; this.#parentItemElement = parentItem; this.#invalidCount = 0; @@ -209,6 +210,7 @@ export class SafeTriangleController { #executePendingSwitch() { const callback = this.#pendingSwitch; this.#pendingSwitch = null; + clearTimeout(this.#pendingTimeout); this.#pendingTimeout = null; if (callback) { callback(); diff --git a/packages/context-menu/test/items.test.js b/packages/context-menu/test/items.test.js index 4408bff3701..0d93141d1ea 100644 --- a/packages/context-menu/test/items.test.js +++ b/packages/context-menu/test/items.test.js @@ -805,6 +805,86 @@ describe('items', () => { expect(subMenu2.opened).to.be.true; }); + it('should switch submenu after fallback timeout when pointer stops moving', async () => { + const parentItem = getMenuItems(rootMenu)[0]; + const parentRect = parentItem.getBoundingClientRect(); + const subMenuOverlay = subMenu._overlayElement; + const subMenuRect = subMenuOverlay.getBoundingClientRect(); + + // Start from the center of the parent item + const startX = parentRect.left + parentRect.width / 2; + const startY = parentRect.top + parentRect.height / 2; + pointerMove(startX, startY); + + await aTimeout(20); + + // Move diagonally toward the submenu (inside the safe triangle) + const targetX = subMenuRect.left + subMenuRect.width / 2; + const targetY = subMenuRect.top + subMenuRect.height / 2; + const midX = (startX + targetX) / 2; + const midY = (startY + targetY) / 2; + pointerMove(midX, midY); + + await aTimeout(20); + + // Hover a sibling item — deferred by safe triangle + const siblingItem = getMenuItems(rootMenu)[3]; + activateItem(siblingItem); + + // Submenu should still be open (safe triangle active) + expect(subMenu.opened).to.be.true; + expect(getMenuItems(subMenu)[0].textContent).to.equal('foo-0-0'); + + // Wait for fallback timeout (400ms) to expire + await aTimeout(450); + + // The pending switch should have executed + expect(getMenuItems(subMenu)[0].textContent).to.equal('foo-3-0'); + }); + + it('should keep submenu open when pointer moves toward it in RTL', async () => { + document.documentElement.setAttribute('dir', 'rtl'); + await nextFrame(); + + // Close and reopen to get RTL positioning + subMenu.close(); + rootMenu.close(); + await nextRender(); + await openMenu(target); + await openMenu(getMenuItems(rootMenu)[0]); + + const parentItem = getMenuItems(rootMenu)[0]; + const parentRect = parentItem.getBoundingClientRect(); + const subMenuOverlay = getSubMenu(rootMenu)._overlayElement; + const subMenuRect = subMenuOverlay.getBoundingClientRect(); + + // In RTL, submenu opens to the left + expect(subMenuRect.right).to.be.at.most(parentRect.left + 1); + + // Start from the center of the parent item + const startX = parentRect.left + parentRect.width / 2; + const startY = parentRect.top + parentRect.height / 2; + pointerMove(startX, startY); + + await aTimeout(20); + + // Move diagonally toward the submenu (leftward in RTL) + const targetX = subMenuRect.left + subMenuRect.width / 2; + const targetY = subMenuRect.top + subMenuRect.height / 2; + const midX = (startX + targetX) / 2; + const midY = (startY + targetY) / 2; + pointerMove(midX, midY); + + await aTimeout(20); + + // Now hover over a sibling item — submenu should stay open + const siblingItem = getMenuItems(rootMenu)[3]; + activateItem(siblingItem); + + expect(getSubMenu(rootMenu).opened).to.be.true; + expect(getMenuItems(getSubMenu(rootMenu))[0].textContent).to.equal('foo-0-0'); + }); + it('should deactivate when submenu closes', () => { const safeTriangle = rootMenu.querySelector('[slot="submenu"]').__safeTriangle; expect(safeTriangle).to.exist; diff --git a/packages/menu-bar/test/sub-menu.test.js b/packages/menu-bar/test/sub-menu.test.js index bb73bad4473..626706b120a 100644 --- a/packages/menu-bar/test/sub-menu.test.js +++ b/packages/menu-bar/test/sub-menu.test.js @@ -747,7 +747,7 @@ describe('touch', () => { menu.items = [ { text: 'Menu Item 1', - children: [{ text: 'Menu Item 1 1' }, { text: 'Menu Item 1 2' }], + children: [{ text: 'Menu Item 1 1' }, { text: 'Menu Item 1 2', children: [{ text: 'Menu Item 1 2 1' }] }], }, { text: 'Menu Item 2' }, { @@ -761,6 +761,10 @@ describe('touch', () => { buttons = menu._buttons; }); + afterEach(() => { + document.documentElement.setAttribute('dir', 'ltr'); + }); + it('should keep submenu open when pointer moves toward it', async () => { // Open submenu for button 0 fire(buttons[0], 'mouseover'); @@ -853,4 +857,95 @@ describe('touch', () => { expect(subMenu.opened).to.be.true; expect(subMenu.listenOn).to.equal(buttons[2]); }); + + it('should switch submenu after fallback timeout when pointer stops moving', async () => { + // Open submenu for button 0 + fire(buttons[0], 'mouseover'); + await nextRender(); + expect(subMenu.opened).to.be.true; + + const btnRect = buttons[0].getBoundingClientRect(); + const overlayRect = subMenu._overlayElement.getBoundingClientRect(); + + // Start from center of expanded button + const startX = btnRect.left + btnRect.width / 2; + const startY = btnRect.top + btnRect.height / 2; + pointerMove(startX, startY); + + await aTimeout(20); + + // Move pointer toward the submenu overlay (inside safe triangle) + const targetX = overlayRect.left + overlayRect.width / 2; + const targetY = overlayRect.top + overlayRect.height / 2; + const midX = (startX + targetX) / 2; + const midY = (startY + targetY) / 2; + pointerMove(midX, midY); + + await aTimeout(20); + + // Hover over another button — deferred by safe triangle + fire(buttons[2], 'mouseover'); + await nextRender(); + + expect(subMenu.opened).to.be.true; + expect(subMenu.listenOn).to.equal(buttons[0]); + + // Wait for fallback timeout (400ms) to expire + await aTimeout(450); + + // The pending switch should have executed + expect(subMenu.listenOn).to.equal(buttons[2]); + }); + + it('should keep nested submenu open when pointer moves toward it in RTL', async () => { + document.documentElement.setAttribute('dir', 'rtl'); + menu.style.position = 'absolute'; + menu.style.left = '0px'; + await nextRender(); + + // Open first-level submenu via button hover + fire(buttons[0], 'mouseover'); + await nextRender(); + expect(subMenu.opened).to.be.true; + + const subMenuOverlay = subMenu._overlayElement; + const items = subMenuOverlay._contentRoot.querySelectorAll('vaadin-menu-bar-item'); + const parentItem = items[1]; // 'Menu Item 1 2' has children + + // Open nested submenu + const nestedSubMenu = subMenu.querySelector('vaadin-menu-bar-submenu'); + subMenu.__openListenerActive = true; + fire(parentItem, 'mouseover'); + await oneEvent(nestedSubMenu._overlayElement, 'vaadin-overlay-open'); + expect(nestedSubMenu.opened).to.be.true; + + const parentRect = parentItem.getBoundingClientRect(); + const nestedOverlayRect = nestedSubMenu._overlayElement.getBoundingClientRect(); + + // In RTL, nested submenu opens to the left + expect(nestedOverlayRect.right).to.be.at.most(parentRect.left + 1); + + // Start from center of parent item + const startX = parentRect.left + parentRect.width / 2; + const startY = parentRect.top + parentRect.height / 2; + pointerMove(startX, startY); + + await aTimeout(20); + + // Move diagonally toward the nested submenu (leftward in RTL) + const targetX = nestedOverlayRect.left + nestedOverlayRect.width / 2; + const targetY = nestedOverlayRect.top + nestedOverlayRect.height / 2; + const midX = (startX + targetX) / 2; + const midY = (startY + targetY) / 2; + pointerMove(midX, midY); + + await aTimeout(20); + + // Hover a sibling item in the first-level submenu + const siblingItem = items[0]; // 'Menu Item 1 1' + fire(siblingItem, 'mouseover'); + + // Nested submenu should stay open (safe triangle protects it) + expect(nestedSubMenu.opened).to.be.true; + }); }); From 2f866aef8140bc30324f48063a1f2b8bab2260a0 Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Mon, 23 Feb 2026 18:55:04 +0100 Subject: [PATCH 07/13] refactor: combine LTR/RTL safe triangle tests using forEach loop Co-Authored-By: Claude Opus 4.6 --- packages/context-menu/test/items.test.js | 115 ++++++++--------------- 1 file changed, 41 insertions(+), 74 deletions(-) diff --git a/packages/context-menu/test/items.test.js b/packages/context-menu/test/items.test.js index 0d93141d1ea..4aa318e0c5b 100644 --- a/packages/context-menu/test/items.test.js +++ b/packages/context-menu/test/items.test.js @@ -733,37 +733,47 @@ describe('items', () => { }); (isTouch ? describe.skip : describe)('safe triangle', () => { - it('should keep submenu open when pointer moves diagonally toward it', async () => { - const parentItem = getMenuItems(rootMenu)[0]; - const parentRect = parentItem.getBoundingClientRect(); - const subMenuOverlay = subMenu._overlayElement; - const subMenuRect = subMenuOverlay.getBoundingClientRect(); - - // Start from the center of the parent item - const startX = parentRect.left + parentRect.width / 2; - const startY = parentRect.top + parentRect.height / 2; - - // Simulate pointer movement toward the submenu (diagonally) - pointerMove(startX, startY); - - // Wait for throttle - await aTimeout(20); - - // Move diagonally toward the submenu - const targetX = subMenuRect.left + subMenuRect.width / 2; - const targetY = subMenuRect.top + subMenuRect.height / 2; - const midX = (startX + targetX) / 2; - const midY = (startY + targetY) / 2; - pointerMove(midX, midY); - - await aTimeout(20); - - // Now hover over a sibling item — submenu should stay open - const siblingItem = getMenuItems(rootMenu)[3]; - activateItem(siblingItem); - - expect(subMenu.opened).to.be.true; - expect(getMenuItems(subMenu)[0].textContent).to.equal('foo-0-0'); + ['ltr', 'rtl'].forEach((dir) => { + describe(dir, () => { + beforeEach(async () => { + if (dir === 'rtl') { + document.documentElement.setAttribute('dir', 'rtl'); + await nextFrame(); + subMenu.close(); + rootMenu.close(); + await nextRender(); + await openMenu(target); + await openMenu(getMenuItems(rootMenu)[0]); + } + }); + + it('should keep submenu open when pointer moves toward it', async () => { + const parentItem = getMenuItems(rootMenu)[0]; + const parentRect = parentItem.getBoundingClientRect(); + const currentSubMenu = getSubMenu(rootMenu); + const subMenuOverlay = currentSubMenu._overlayElement; + const subMenuRect = subMenuOverlay.getBoundingClientRect(); + + if (dir === 'rtl') { + expect(subMenuRect.right).to.be.at.most(parentRect.left + 1); + } + + const startX = parentRect.left + parentRect.width / 2; + const startY = parentRect.top + parentRect.height / 2; + pointerMove(startX, startY); + await aTimeout(20); + + const targetX = subMenuRect.left + subMenuRect.width / 2; + const targetY = subMenuRect.top + subMenuRect.height / 2; + pointerMove((startX + targetX) / 2, (startY + targetY) / 2); + await aTimeout(20); + + activateItem(getMenuItems(rootMenu)[3]); + + expect(currentSubMenu.opened).to.be.true; + expect(getMenuItems(currentSubMenu)[0].textContent).to.equal('foo-0-0'); + }); + }); }); it('should switch submenu when pointer moves away from it', async () => { @@ -842,49 +852,6 @@ describe('items', () => { expect(getMenuItems(subMenu)[0].textContent).to.equal('foo-3-0'); }); - it('should keep submenu open when pointer moves toward it in RTL', async () => { - document.documentElement.setAttribute('dir', 'rtl'); - await nextFrame(); - - // Close and reopen to get RTL positioning - subMenu.close(); - rootMenu.close(); - await nextRender(); - await openMenu(target); - await openMenu(getMenuItems(rootMenu)[0]); - - const parentItem = getMenuItems(rootMenu)[0]; - const parentRect = parentItem.getBoundingClientRect(); - const subMenuOverlay = getSubMenu(rootMenu)._overlayElement; - const subMenuRect = subMenuOverlay.getBoundingClientRect(); - - // In RTL, submenu opens to the left - expect(subMenuRect.right).to.be.at.most(parentRect.left + 1); - - // Start from the center of the parent item - const startX = parentRect.left + parentRect.width / 2; - const startY = parentRect.top + parentRect.height / 2; - pointerMove(startX, startY); - - await aTimeout(20); - - // Move diagonally toward the submenu (leftward in RTL) - const targetX = subMenuRect.left + subMenuRect.width / 2; - const targetY = subMenuRect.top + subMenuRect.height / 2; - const midX = (startX + targetX) / 2; - const midY = (startY + targetY) / 2; - pointerMove(midX, midY); - - await aTimeout(20); - - // Now hover over a sibling item — submenu should stay open - const siblingItem = getMenuItems(rootMenu)[3]; - activateItem(siblingItem); - - expect(getSubMenu(rootMenu).opened).to.be.true; - expect(getMenuItems(getSubMenu(rootMenu))[0].textContent).to.equal('foo-0-0'); - }); - it('should deactivate when submenu closes', () => { const safeTriangle = rootMenu.querySelector('[slot="submenu"]').__safeTriangle; expect(safeTriangle).to.exist; From f1842422ad9a28cff45f4f03037625ea0a43201b Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Mon, 23 Feb 2026 19:37:27 +0100 Subject: [PATCH 08/13] fix: make safe triangle deactivation tests non-vacuous 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 --- packages/context-menu/test/items.test.js | 28 +++++++++++++----------- packages/menu-bar/test/sub-menu.test.js | 11 ++++++++++ 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/packages/context-menu/test/items.test.js b/packages/context-menu/test/items.test.js index 4aa318e0c5b..39ca28e5dbb 100644 --- a/packages/context-menu/test/items.test.js +++ b/packages/context-menu/test/items.test.js @@ -804,17 +804,6 @@ describe('items', () => { expect(getMenuItems(subMenu)[0].textContent).to.equal('foo-3-0'); }); - it('should not interfere with keyboard navigation', async () => { - // Arrow right on an item with children should open nested submenu - const items = getMenuItems(subMenu); - const itemWithChildren = items[2]; // foo-0-2 has children - itemWithChildren.focus(); - arrowRightKeyDown(itemWithChildren); - const subMenu2 = getSubMenu(subMenu); - await nextRender(); - expect(subMenu2.opened).to.be.true; - }); - it('should switch submenu after fallback timeout when pointer stops moving', async () => { const parentItem = getMenuItems(rootMenu)[0]; const parentRect = parentItem.getBoundingClientRect(); @@ -852,10 +841,23 @@ describe('items', () => { expect(getMenuItems(subMenu)[0].textContent).to.equal('foo-3-0'); }); - it('should deactivate when submenu closes', () => { - const safeTriangle = rootMenu.querySelector('[slot="submenu"]').__safeTriangle; + it('should deactivate when submenu closes', async () => { + const safeTriangle = rootMenu.__safeTriangle; expect(safeTriangle).to.exist; + + // Track pointer movement so shouldKeepOpen() would return true if still active + const parentRect = getMenuItems(rootMenu)[0].getBoundingClientRect(); + const subMenuRect = subMenu._overlayElement.getBoundingClientRect(); + pointerMove(parentRect.left + parentRect.width / 2, parentRect.top + parentRect.height / 2); + await aTimeout(20); + pointerMove(subMenuRect.left + subMenuRect.width / 2, subMenuRect.top + subMenuRect.height / 2); + await aTimeout(20); + + // Verify safe triangle is active before closing + expect(safeTriangle.shouldKeepOpen()).to.be.true; + subMenu.close(); + await nextRender(); expect(safeTriangle.shouldKeepOpen()).to.be.false; }); }); diff --git a/packages/menu-bar/test/sub-menu.test.js b/packages/menu-bar/test/sub-menu.test.js index 626706b120a..21f69cf4a80 100644 --- a/packages/menu-bar/test/sub-menu.test.js +++ b/packages/menu-bar/test/sub-menu.test.js @@ -839,6 +839,17 @@ describe('touch', () => { const safeTriangle = menu.__safeTriangle; expect(safeTriangle).to.exist; + // Track pointer movement so shouldKeepOpen() would return true if still active + const btnRect = buttons[0].getBoundingClientRect(); + const overlayRect = subMenu._overlayElement.getBoundingClientRect(); + pointerMove(btnRect.left + btnRect.width / 2, btnRect.top + btnRect.height / 2); + await aTimeout(20); + pointerMove(overlayRect.left + overlayRect.width / 2, overlayRect.top + overlayRect.height / 2); + await aTimeout(20); + + // Verify safe triangle is active before closing + expect(safeTriangle.shouldKeepOpen()).to.be.true; + menu.close(); await nextRender(); From 5dc81431e5922d4f4d145d5c5185183bf6bfba71 Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Tue, 24 Feb 2026 18:24:27 +0100 Subject: [PATCH 09/13] feat: suppress hover highlight during safe triangle navigation 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 --- packages/aura/src/components/item-overlay.css | 7 +++++++ .../src/vaadin-contextmenu-items-mixin.js | 2 +- .../src/vaadin-safe-triangle-controller.js | 14 +++++++++++++- packages/context-menu/test/items.test.js | 14 ++++++++++++++ packages/menu-bar/src/vaadin-menu-bar-mixin.js | 2 +- packages/menu-bar/test/sub-menu.test.js | 17 +++++++++++++++++ .../src/components/context-menu-list-box.css | 5 +++++ 7 files changed, 58 insertions(+), 3 deletions(-) diff --git a/packages/aura/src/components/item-overlay.css b/packages/aura/src/components/item-overlay.css index dc58d5b684b..578a58c3a58 100644 --- a/packages/aura/src/components/item-overlay.css +++ b/packages/aura/src/components/item-overlay.css @@ -67,6 +67,13 @@ vaadin-select-item:where([role]) { background: var(--_highlight-color); } + /* Suppress hover highlight during safe triangle navigation */ + @media (any-hover: hover) { + [safe-triangle-active] > &:not([aria-expanded='true']):not([disabled], [aria-disabled='true']):hover { + background: transparent; + } + } + &[aria-expanded='true']:not(:hover) { background: var(--vaadin-background-container-strong); } diff --git a/packages/context-menu/src/vaadin-contextmenu-items-mixin.js b/packages/context-menu/src/vaadin-contextmenu-items-mixin.js index ea1fbbb677c..1229a56e464 100644 --- a/packages/context-menu/src/vaadin-contextmenu-items-mixin.js +++ b/packages/context-menu/src/vaadin-contextmenu-items-mixin.js @@ -167,7 +167,7 @@ export const ItemsMixin = (superClass) => // Activate safe triangle tracking for the newly opened submenu if (this.__safeTriangle) { - this.__safeTriangle.activate(subMenuOverlay, itemElement); + this.__safeTriangle.activate(subMenuOverlay, itemElement, this._listBox); } } diff --git a/packages/context-menu/src/vaadin-safe-triangle-controller.js b/packages/context-menu/src/vaadin-safe-triangle-controller.js index 55e50d8330a..7947cb8a682 100644 --- a/packages/context-menu/src/vaadin-safe-triangle-controller.js +++ b/packages/context-menu/src/vaadin-safe-triangle-controller.js @@ -46,6 +46,8 @@ export class SafeTriangleController { #pendingTimeout = null; + #parentContainer = null; + #onPointerMove = (event) => { // Only handle mouse pointer, not touch or pen if (event.pointerType === 'touch' || event.pointerType === 'pen') { @@ -133,8 +135,9 @@ export class SafeTriangleController { * * @param {HTMLElement} submenuOverlay - The submenu overlay element * @param {HTMLElement} parentItem - The parent menu item that triggered the submenu + * @param {HTMLElement} [parentContainer] - Optional container element to set safe-triangle-active attribute on */ - activate(submenuOverlay, parentItem) { + activate(submenuOverlay, parentItem, parentContainer) { this.#cancelPendingSwitch(); this.#submenuElement = submenuOverlay; this.#parentItemElement = parentItem; @@ -144,6 +147,11 @@ export class SafeTriangleController { this.#lastX = 0; this.#lastY = 0; + if (parentContainer) { + this.#parentContainer = parentContainer; + parentContainer.setAttribute('safe-triangle-active', ''); + } + if (!this.#active) { this.#active = true; document.addEventListener('pointermove', this.#onPointerMove); @@ -155,6 +163,10 @@ export class SafeTriangleController { * Should be called when a submenu closes. */ deactivate() { + if (this.#parentContainer) { + this.#parentContainer.removeAttribute('safe-triangle-active'); + this.#parentContainer = null; + } if (this.#active) { this.#active = false; document.removeEventListener('pointermove', this.#onPointerMove); diff --git a/packages/context-menu/test/items.test.js b/packages/context-menu/test/items.test.js index 39ca28e5dbb..1b9b46e9893 100644 --- a/packages/context-menu/test/items.test.js +++ b/packages/context-menu/test/items.test.js @@ -860,5 +860,19 @@ describe('items', () => { await nextRender(); expect(safeTriangle.shouldKeepOpen()).to.be.false; }); + + it('should set safe-triangle-active attribute on list-box when active', () => { + const listBox = rootMenu._listBox; + expect(listBox.hasAttribute('safe-triangle-active')).to.be.true; + }); + + it('should remove safe-triangle-active attribute when deactivated', async () => { + const listBox = rootMenu._listBox; + expect(listBox.hasAttribute('safe-triangle-active')).to.be.true; + + subMenu.close(); + await nextRender(); + expect(listBox.hasAttribute('safe-triangle-active')).to.be.false; + }); }); }); diff --git a/packages/menu-bar/src/vaadin-menu-bar-mixin.js b/packages/menu-bar/src/vaadin-menu-bar-mixin.js index 9ae1887fc54..845deaa6393 100644 --- a/packages/menu-bar/src/vaadin-menu-bar-mixin.js +++ b/packages/menu-bar/src/vaadin-menu-bar-mixin.js @@ -1011,7 +1011,7 @@ export const MenuBarMixin = (superClass) => // Activate safe triangle tracking for the newly opened submenu if (this.__safeTriangle) { - this.__safeTriangle.activate(overlay, button); + this.__safeTriangle.activate(overlay, button, this); } overlay.addEventListener( diff --git a/packages/menu-bar/test/sub-menu.test.js b/packages/menu-bar/test/sub-menu.test.js index 21f69cf4a80..9d5d791bc94 100644 --- a/packages/menu-bar/test/sub-menu.test.js +++ b/packages/menu-bar/test/sub-menu.test.js @@ -959,4 +959,21 @@ describe('touch', () => { // Nested submenu should stay open (safe triangle protects it) expect(nestedSubMenu.opened).to.be.true; }); + + it('should set safe-triangle-active attribute on menu-bar when active', async () => { + fire(buttons[0], 'mouseover'); + await nextRender(); + expect(subMenu.opened).to.be.true; + expect(menu.hasAttribute('safe-triangle-active')).to.be.true; + }); + + it('should remove safe-triangle-active attribute when deactivated', async () => { + fire(buttons[0], 'mouseover'); + await nextRender(); + expect(menu.hasAttribute('safe-triangle-active')).to.be.true; + + menu.close(); + await nextRender(); + expect(menu.hasAttribute('safe-triangle-active')).to.be.false; + }); }); diff --git a/packages/vaadin-lumo-styles/src/components/context-menu-list-box.css b/packages/vaadin-lumo-styles/src/components/context-menu-list-box.css index 2a105e6f22d..ae44fb1e25d 100644 --- a/packages/vaadin-lumo-styles/src/components/context-menu-list-box.css +++ b/packages/vaadin-lumo-styles/src/components/context-menu-list-box.css @@ -25,6 +25,11 @@ background-color: var(--lumo-primary-color-10pct); } + /* Suppress hover highlight during safe triangle navigation */ + :host([safe-triangle-active]) [part='items'] ::slotted([role='menuitem']:hover:not([disabled]):not([expanded])) { + background-color: transparent; + } + /* RTL styles */ :host([dir='rtl']) [part='items'] ::slotted([role='menuitem']) { padding-left: calc(var(--lumo-space-l) + var(--lumo-border-radius-m) / 4); From 122e648c784d5abe014c6efc8af2802abc151d6f Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Tue, 24 Feb 2026 18:50:05 +0100 Subject: [PATCH 10/13] fix: safe triangle deactivation on Escape and stale container cleanup 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 --- packages/context-menu/src/vaadin-safe-triangle-controller.js | 3 +++ packages/menu-bar/src/vaadin-menu-bar-mixin.js | 3 +++ 2 files changed, 6 insertions(+) diff --git a/packages/context-menu/src/vaadin-safe-triangle-controller.js b/packages/context-menu/src/vaadin-safe-triangle-controller.js index 7947cb8a682..347b120abe4 100644 --- a/packages/context-menu/src/vaadin-safe-triangle-controller.js +++ b/packages/context-menu/src/vaadin-safe-triangle-controller.js @@ -147,6 +147,9 @@ export class SafeTriangleController { this.#lastX = 0; this.#lastY = 0; + if (this.#parentContainer && this.#parentContainer !== parentContainer) { + this.#parentContainer.removeAttribute('safe-triangle-active'); + } if (parentContainer) { this.#parentContainer = parentContainer; parentContainer.setAttribute('safe-triangle-active', ''); diff --git a/packages/menu-bar/src/vaadin-menu-bar-mixin.js b/packages/menu-bar/src/vaadin-menu-bar-mixin.js index 845deaa6393..230d694cee3 100644 --- a/packages/menu-bar/src/vaadin-menu-bar-mixin.js +++ b/packages/menu-bar/src/vaadin-menu-bar-mixin.js @@ -1059,6 +1059,9 @@ export const MenuBarMixin = (superClass) => /** @private */ __onEscapeClose() { this.__deactivateButton(true); + if (this.__safeTriangle) { + this.__safeTriangle.deactivate(); + } } /** @private */ From e0b7159aff7bfe2fafe0ef1be9a89c8d5d07a75f Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Fri, 20 Mar 2026 15:44:20 +0100 Subject: [PATCH 11/13] test: update snapshot references --- .../test/dom/__snapshots__/context-menu.test.snap.js | 1 + packages/menu-bar/test/dom/__snapshots__/menu-bar.test.snap.js | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/context-menu/test/dom/__snapshots__/context-menu.test.snap.js b/packages/context-menu/test/dom/__snapshots__/context-menu.test.snap.js index d0aee9606ae..9942464f714 100644 --- a/packages/context-menu/test/dom/__snapshots__/context-menu.test.snap.js +++ b/packages/context-menu/test/dom/__snapshots__/context-menu.test.snap.js @@ -77,6 +77,7 @@ snapshots["context-menu items nested"] = Date: Fri, 20 Mar 2026 16:08:07 +0100 Subject: [PATCH 12/13] refactor: simplify controller and menu-bar deactivation --- .../src/vaadin-safe-triangle-controller.d.ts | 2 +- .../src/vaadin-safe-triangle-controller.js | 36 +++++++------------ .../menu-bar/src/vaadin-menu-bar-mixin.js | 12 +++---- 3 files changed, 19 insertions(+), 31 deletions(-) diff --git a/packages/context-menu/src/vaadin-safe-triangle-controller.d.ts b/packages/context-menu/src/vaadin-safe-triangle-controller.d.ts index ccf5a67654f..78950b511a1 100644 --- a/packages/context-menu/src/vaadin-safe-triangle-controller.d.ts +++ b/packages/context-menu/src/vaadin-safe-triangle-controller.d.ts @@ -17,7 +17,7 @@ export class SafeTriangleController { * Activate the safe triangle tracking for the given submenu overlay. * Should be called when a submenu opens. */ - activate(submenuOverlay: HTMLElement, parentItem: HTMLElement): void; + activate(submenuOverlay: HTMLElement, parentItem: HTMLElement, parentContainer?: HTMLElement): void; /** * Deactivate the safe triangle tracking. diff --git a/packages/context-menu/src/vaadin-safe-triangle-controller.js b/packages/context-menu/src/vaadin-safe-triangle-controller.js index 347b120abe4..e2842f17b31 100644 --- a/packages/context-menu/src/vaadin-safe-triangle-controller.js +++ b/packages/context-menu/src/vaadin-safe-triangle-controller.js @@ -26,16 +26,12 @@ const FALLBACK_TIMEOUT_MS = 400; * - Only active for pointer/mouse input; ignored for touch and pen */ export class SafeTriangleController { - #hasLastPosition = false; - #lastX = 0; #lastY = 0; #invalidCount = 0; - #active = false; - #lastMoveTime = 0; #submenuElement = null; @@ -54,21 +50,20 @@ export class SafeTriangleController { return; } - const now = performance.now(); - if (now - this.#lastMoveTime < THROTTLE_MS) { + if (event.timeStamp - this.#lastMoveTime < THROTTLE_MS) { return; } - this.#lastMoveTime = now; const x = event.clientX; const y = event.clientY; - if (!this.#hasLastPosition) { - this.#hasLastPosition = true; + if (this.#lastMoveTime === 0) { + this.#lastMoveTime = event.timeStamp; this.#lastX = x; this.#lastY = y; return; } + this.#lastMoveTime = event.timeStamp; if (!this.#submenuElement) { this.#lastX = x; @@ -139,11 +134,11 @@ export class SafeTriangleController { */ activate(submenuOverlay, parentItem, parentContainer) { this.#cancelPendingSwitch(); + const wasActive = this.#submenuElement !== null; this.#submenuElement = submenuOverlay; this.#parentItemElement = parentItem; this.#invalidCount = 0; this.#lastMoveTime = 0; - this.#hasLastPosition = false; this.#lastX = 0; this.#lastY = 0; @@ -155,8 +150,7 @@ export class SafeTriangleController { parentContainer.setAttribute('safe-triangle-active', ''); } - if (!this.#active) { - this.#active = true; + if (!wasActive) { document.addEventListener('pointermove', this.#onPointerMove); } } @@ -170,8 +164,7 @@ export class SafeTriangleController { this.#parentContainer.removeAttribute('safe-triangle-active'); this.#parentContainer = null; } - if (this.#active) { - this.#active = false; + if (this.#submenuElement) { document.removeEventListener('pointermove', this.#onPointerMove); } this.#submenuElement = null; @@ -187,7 +180,7 @@ export class SafeTriangleController { * @return {boolean} */ shouldKeepOpen() { - if (!this.#active || !this.#submenuElement) { + if (!this.#submenuElement) { return false; } // Only block switches if we've actually tracked pointer movement. @@ -215,18 +208,15 @@ export class SafeTriangleController { } #cancelPendingSwitch() { - this.#pendingSwitch = null; - if (this.#pendingTimeout) { - clearTimeout(this.#pendingTimeout); - this.#pendingTimeout = null; - } - } - - #executePendingSwitch() { const callback = this.#pendingSwitch; this.#pendingSwitch = null; clearTimeout(this.#pendingTimeout); this.#pendingTimeout = null; + return callback; + } + + #executePendingSwitch() { + const callback = this.#cancelPendingSwitch(); if (callback) { callback(); } diff --git a/packages/menu-bar/src/vaadin-menu-bar-mixin.js b/packages/menu-bar/src/vaadin-menu-bar-mixin.js index 230d694cee3..3306f2d30b1 100644 --- a/packages/menu-bar/src/vaadin-menu-bar-mixin.js +++ b/packages/menu-bar/src/vaadin-menu-bar-mixin.js @@ -285,6 +285,11 @@ export const MenuBarMixin = (superClass) => menu.addEventListener('item-selected', this.__onItemSelected.bind(this)); menu.addEventListener('close-all-menus', this.__onEscapeClose.bind(this)); + menu.addEventListener('opened-changed', (e) => { + if (!e.detail.value && this.__safeTriangle) { + this.__safeTriangle.deactivate(); + } + }); const overlay = menu._overlayElement; overlay._contentRoot.addEventListener('keydown', this.__boundOnContextMenuKeydown); @@ -1059,9 +1064,6 @@ export const MenuBarMixin = (superClass) => /** @private */ __onEscapeClose() { this.__deactivateButton(true); - if (this.__safeTriangle) { - this.__safeTriangle.deactivate(); - } } /** @private */ @@ -1087,10 +1089,6 @@ export const MenuBarMixin = (superClass) => if (this._subMenu.opened) { this._subMenu.close(); } - // Deactivate safe triangle tracking when submenu closes - if (this.__safeTriangle) { - this.__safeTriangle.deactivate(); - } } /** From fe5e10b9697ebf238b5745b6ebf51085aa0d24a5 Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Fri, 20 Mar 2026 19:10:39 +0100 Subject: [PATCH 13/13] refactor: extract angle computation to reduce cognitive complexity 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) --- .../src/vaadin-safe-triangle-controller.js | 77 +++++++++---------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/packages/context-menu/src/vaadin-safe-triangle-controller.js b/packages/context-menu/src/vaadin-safe-triangle-controller.js index e2842f17b31..c48edb6163d 100644 --- a/packages/context-menu/src/vaadin-safe-triangle-controller.js +++ b/packages/context-menu/src/vaadin-safe-triangle-controller.js @@ -71,48 +71,13 @@ export class SafeTriangleController { return; } - const submenuRect = this.#submenuElement.$.overlay.getBoundingClientRect(); - - // Skip if submenu is not visible - if (submenuRect.width === 0 || submenuRect.height === 0) { - this.#lastX = x; - this.#lastY = y; - return; - } - - // Determine submenu direction from actual position, not RTL flag - const parentRect = this.#parentItemElement.getBoundingClientRect(); - const submenuIsRight = submenuRect.left >= parentRect.left; - const dx = x - this.#lastX; + const dy = y - this.#lastY; - // Early exit: moving horizontally away from the submenu - if ((submenuIsRight && dx < -1) || (!submenuIsRight && dx > 1)) { - this.#invalidCount += 1; + if (this.#isPointerAimedAtSubmenu(dx, dy)) { + this.#invalidCount = 0; } else { - // Compute the near edge corners of the submenu - const nearX = submenuIsRight ? submenuRect.left : submenuRect.right; - const topY = submenuRect.top; - const bottomY = submenuRect.bottom; - - // Angle from previous cursor position to the two submenu corners - const thetaTop = Math.atan2(topY - this.#lastY, nearX - this.#lastX); - const thetaBottom = Math.atan2(bottomY - this.#lastY, nearX - this.#lastX); - - // Angle of cursor movement vector - const dy = y - this.#lastY; - const thetaPointer = Math.atan2(dy, dx); - - // Determine the angular bounds (top and bottom may swap depending on direction) - const minAngle = Math.min(thetaTop, thetaBottom); - const maxAngle = Math.max(thetaTop, thetaBottom); - - if (thetaPointer >= minAngle - TOLERANCE_RAD && thetaPointer <= maxAngle + TOLERANCE_RAD) { - // Cursor is aimed at the submenu - this.#invalidCount = 0; - } else { - this.#invalidCount += 1; - } + this.#invalidCount += 1; } this.#lastX = x; @@ -207,6 +172,40 @@ export class SafeTriangleController { }, FALLBACK_TIMEOUT_MS); } + #isPointerAimedAtSubmenu(dx, dy) { + const submenuRect = this.#submenuElement.$.overlay.getBoundingClientRect(); + + // Skip if submenu is not visible + if (submenuRect.width === 0 || submenuRect.height === 0) { + return false; + } + + // Determine submenu direction from actual position, not RTL flag + const parentRect = this.#parentItemElement.getBoundingClientRect(); + const submenuIsRight = submenuRect.left >= parentRect.left; + + // Early exit: moving horizontally away from the submenu + if ((submenuIsRight && dx < -1) || (!submenuIsRight && dx > 1)) { + return false; + } + + // Compute the near edge corners of the submenu + const nearX = submenuIsRight ? submenuRect.left : submenuRect.right; + + // Angle from previous cursor position to the two submenu corners + const thetaTop = Math.atan2(submenuRect.top - this.#lastY, nearX - this.#lastX); + const thetaBottom = Math.atan2(submenuRect.bottom - this.#lastY, nearX - this.#lastX); + + // Angle of cursor movement vector + const thetaPointer = Math.atan2(dy, dx); + + // Determine the angular bounds (top and bottom may swap depending on direction) + const minAngle = Math.min(thetaTop, thetaBottom); + const maxAngle = Math.max(thetaTop, thetaBottom); + + return thetaPointer >= minAngle - TOLERANCE_RAD && thetaPointer <= maxAngle + TOLERANCE_RAD; + } + #cancelPendingSwitch() { const callback = this.#pendingSwitch; this.#pendingSwitch = null;