From bdbf0dff56c71ca1f910119c8023e9c9ed0ca2d2 Mon Sep 17 00:00:00 2001 From: Shipra Gupta Date: Tue, 4 Nov 2025 13:22:18 -0800 Subject: [PATCH 1/9] fix: improve touch interaction handling for submenus --- 1st-gen/packages/menu/src/MenuItem.ts | 98 +++++++++++++-------------- 1 file changed, 46 insertions(+), 52 deletions(-) diff --git a/1st-gen/packages/menu/src/MenuItem.ts b/1st-gen/packages/menu/src/MenuItem.ts index 75de1e0de3d..4450c23ef3c 100644 --- a/1st-gen/packages/menu/src/MenuItem.ts +++ b/1st-gen/packages/menu/src/MenuItem.ts @@ -181,6 +181,8 @@ export class MenuItem extends LikeAnchor( return this._value || this.itemText; } + private _lastPointerType?: string; + public set value(value: string) { if (value === this._value) { return; @@ -457,25 +459,15 @@ export class MenuItem extends LikeAnchor( } } - private handlePointerdown(event: PointerEvent): void { - if (event.target === this && this.hasSubmenu && this.open) { - this.addEventListener('focus', this.handleSubmenuFocus, { - once: true, - }); - this.overlayElement.addEventListener( - 'beforetoggle', - this.handleBeforetoggle - ); - } - } - protected override firstUpdated(changes: PropertyValues): void { super.firstUpdated(changes); this.setAttribute('tabindex', '-1'); this.addEventListener('keydown', this.handleKeydown); this.addEventListener('mouseover', this.handleMouseover); - this.addEventListener('pointerdown', this.handlePointerdown); - this.addEventListener('pointerenter', this.closeOverlaysForRoot); + // Register pointerenter/leave for ALL menu items (not just those with submenus) + // so items without submenus can close sibling submenus when hovered + this.addEventListener('pointerenter', this.handlePointerenter); + this.addEventListener('pointerleave', this.handlePointerleave); if (!this.hasAttribute('id')) { this.id = `sp-menu-item-${randomID()}`; } @@ -575,6 +567,7 @@ export class MenuItem extends LikeAnchor( return false; } + /** * forward key info from keydown event to parent menu */ @@ -594,11 +587,6 @@ export class MenuItem extends LikeAnchor( } }; - protected closeOverlaysForRoot(): void { - if (this.open) return; - this.menuData.parentMenu?.closeDescendentOverlays(); - } - protected handleFocus(event: FocusEvent): void { const { target } = event; if (target === this) { @@ -613,48 +601,64 @@ export class MenuItem extends LikeAnchor( } } - protected handleSubmenuClick(event: Event): void { + protected handleSubmenuTriggerClick(event: Event): void { if (event.composedPath().includes(this.overlayElement)) { return; } - this.openOverlay(true); - } - protected handleSubmenuFocus(): void { - requestAnimationFrame(() => { - // Wait till after `closeDescendentOverlays` has happened in Menu - // to reopen (keep open) the direct descendent of this Menu Item - this.overlayElement.open = this.open; - this.focused = false; - }); + // If submenu is already open, toggle it closed + if (this.open && this._lastPointerType === 'touch') { + event.preventDefault(); + event.stopPropagation(); // Don't let parent menu handle this + this.open = false; + return; + } + + // All: open if closed + if (!this.open) { + event.preventDefault(); + event.stopImmediatePropagation(); + this.openOverlay(true); + } } - protected handleBeforetoggle = (event: Event): void => { - if ((event as Event & { newState: string }).newState === 'closed') { - this.open = true; - this.overlayElement.manuallyKeepOpen(); - this.overlayElement.removeEventListener( - 'beforetoggle', - this.handleBeforetoggle - ); + protected handlePointerenter(event: PointerEvent): void { + this._lastPointerType = event.pointerType; // Track pointer type + + // For touch: don't handle pointerenter, let click handle it + if (event.pointerType === 'touch') { + return; } - }; - protected handlePointerenter(): void { + // Close sibling submenus before opening this one + this.menuData.parentMenu?.closeDescendentOverlays(); + if (this.leaveTimeout) { clearTimeout(this.leaveTimeout); delete this.leaveTimeout; this.recentlyLeftChild = false; return; } - this.focus(); + + // Only focus items with submenus on hover (to show they're interactive) + // Regular items should not show focus styling on hover, only on keyboard navigation + if (this.hasSubmenu) { + this.focus(); + } this.openOverlay(); } protected leaveTimeout?: ReturnType; protected recentlyLeftChild = false; - protected handlePointerleave(): void { + protected handlePointerleave(event: PointerEvent): void { + this._lastPointerType = event.pointerType; // Update on leave too + + // For touch: don't handle pointerleave, let click handle it + if (event.pointerType === 'touch') { + return; + } + this._closedViaPointer = true; if (this.open && !this.recentlyLeftChild) { this.leaveTimeout = setTimeout(() => { @@ -782,17 +786,7 @@ export class MenuItem extends LikeAnchor( const options = { signal: this.abortControllerSubmenu.signal }; this.addEventListener( 'click', - this.handleSubmenuClick, - options - ); - this.addEventListener( - 'pointerenter', - this.handlePointerenter, - options - ); - this.addEventListener( - 'pointerleave', - this.handlePointerleave, + this.handleSubmenuTriggerClick, options ); this.addEventListener( From ae15099be0a090e3ec72931114ce0f6c7247c315 Mon Sep 17 00:00:00 2001 From: Shipra Gupta Date: Tue, 4 Nov 2025 15:04:42 -0800 Subject: [PATCH 2/9] chore: add changeset for menu touch interaction fix --- .changeset/brave-foxes-smile.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/brave-foxes-smile.md diff --git a/.changeset/brave-foxes-smile.md b/.changeset/brave-foxes-smile.md new file mode 100644 index 00000000000..8644d8fd4bf --- /dev/null +++ b/.changeset/brave-foxes-smile.md @@ -0,0 +1,5 @@ +--- +'@spectrum-web-components/menu': patch +--- + +**Fixed**: Improved touch interaction handling for submenus to prevent unintended submenu closures. From 1f3e0278a884c8bdc6264f404b8f95ab0540c1be Mon Sep 17 00:00:00 2001 From: Shipra Gupta Date: Tue, 4 Nov 2025 15:13:13 -0800 Subject: [PATCH 3/9] test: add comprehensive touch interaction tests for menu submenus --- 1st-gen/packages/menu/test/submenu.test.ts | 199 +++++++++++++++++++++ 1 file changed, 199 insertions(+) diff --git a/1st-gen/packages/menu/test/submenu.test.ts b/1st-gen/packages/menu/test/submenu.test.ts index e3dde7ef127..ae9de9de642 100644 --- a/1st-gen/packages/menu/test/submenu.test.ts +++ b/1st-gen/packages/menu/test/submenu.test.ts @@ -887,4 +887,203 @@ describe('Submenu', () => { expect(submenu.scrollTop).to.equal(50); expect(submenu.scrollTop).to.not.equal(initialScrollTop); }); + describe('touch interactions', () => { + beforeEach(async function () { + this.el = await fixture(html` + + No submenu + + Has submenu + + + One + + + Two + + + Three + + + + + `); + await elementUpdated(this.el); + this.rootItem = this.el.querySelector('.root') as MenuItem; + await elementUpdated(this.rootItem); + }); + + it('does not open submenu on touch pointerenter', async function () { + expect(this.rootItem.open).to.be.false; + + // Simulate touch pointerenter event + this.rootItem.dispatchEvent( + new PointerEvent('pointerenter', { + bubbles: true, + pointerType: 'touch', + }) + ); + + // Wait to ensure submenu doesn't open + await aTimeout(150); + + expect(this.rootItem.open).to.be.false; + }); + + it('does not close submenu on touch pointerleave', async function () { + // First open the submenu via click + expect(this.rootItem.open).to.be.false; + + const opened = oneEvent(this.rootItem, 'sp-opened'); + this.rootItem.click(); + await opened; + + expect(this.rootItem.open).to.be.true; + + // Simulate touch pointerleave event + this.rootItem.dispatchEvent( + new PointerEvent('pointerleave', { + bubbles: true, + pointerType: 'touch', + }) + ); + + // Wait to ensure submenu doesn't close + await aTimeout(150); + + expect(this.rootItem.open).to.be.true; + }); + + it('opens submenu on touch click when closed', async function () { + expect(this.rootItem.open).to.be.false; + + // Track pointer type by dispatching pointerenter first + this.rootItem.dispatchEvent( + new PointerEvent('pointerenter', { + bubbles: true, + pointerType: 'touch', + }) + ); + + const opened = oneEvent(this.rootItem, 'sp-opened'); + this.rootItem.click(); + await opened; + + expect(this.rootItem.open).to.be.true; + }); + + it('closes submenu on touch click when open', async function () { + // First open the submenu + this.rootItem.dispatchEvent( + new PointerEvent('pointerenter', { + bubbles: true, + pointerType: 'touch', + }) + ); + + const opened = oneEvent(this.rootItem, 'sp-opened'); + this.rootItem.click(); + await opened; + + expect(this.rootItem.open).to.be.true; + + // Click again to close + const closed = oneEvent(this.rootItem, 'sp-closed'); + this.rootItem.click(); + await closed; + + expect(this.rootItem.open).to.be.false; + }); + + it('mouse pointerenter still opens submenu', async function () { + expect(this.rootItem.open).to.be.false; + + const opened = oneEvent(this.rootItem, 'sp-opened'); + this.rootItem.dispatchEvent( + new PointerEvent('pointerenter', { + bubbles: true, + pointerType: 'mouse', + }) + ); + await opened; + + expect(this.rootItem.open).to.be.true; + }); + + it('mouse pointerleave still closes submenu', async function () { + // First open via mouse + const opened = oneEvent(this.rootItem, 'sp-opened'); + this.rootItem.dispatchEvent( + new PointerEvent('pointerenter', { + bubbles: true, + pointerType: 'mouse', + }) + ); + await opened; + + expect(this.rootItem.open).to.be.true; + + // Leave with mouse + const closed = oneEvent(this.rootItem, 'sp-closed'); + this.rootItem.dispatchEvent( + new PointerEvent('pointerleave', { + bubbles: true, + pointerType: 'mouse', + }) + ); + await closed; + + expect(this.rootItem.open).to.be.false; + }); + + it('closes sibling submenus on touch pointerenter', async function () { + // Create a second menu item with submenu + const el = await fixture(html` + + + First submenu + + Item A + + + + Second submenu + + Item B + + + + `); + await elementUpdated(el); + const rootItem1 = el.querySelector('.root-1') as MenuItem; + const rootItem2 = el.querySelector('.root-2') as MenuItem; + await elementUpdated(rootItem1); + await elementUpdated(rootItem2); + + // Open first submenu with mouse + const opened1 = oneEvent(rootItem1, 'sp-opened'); + rootItem1.dispatchEvent( + new PointerEvent('pointerenter', { + bubbles: true, + pointerType: 'mouse', + }) + ); + await opened1; + + expect(rootItem1.open).to.be.true; + expect(rootItem2.open).to.be.false; + + // Hover second item with mouse should close first + const closed1 = oneEvent(rootItem1, 'sp-closed'); + rootItem2.dispatchEvent( + new PointerEvent('pointerenter', { + bubbles: true, + pointerType: 'mouse', + }) + ); + await closed1; + + expect(rootItem1.open).to.be.false; + }); + }); }); From a8163fd9e81c787a4935951c6c26ea8822ce525e Mon Sep 17 00:00:00 2001 From: Shipra Gupta Date: Fri, 7 Nov 2025 18:53:10 -0800 Subject: [PATCH 4/9] fix(overlay): add null checks for window.__swc.warn in test environments --- 1st-gen/packages/overlay/src/slottable-request-directive.ts | 2 +- 1st-gen/packages/overlay/src/slottable-request-event.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/1st-gen/packages/overlay/src/slottable-request-directive.ts b/1st-gen/packages/overlay/src/slottable-request-directive.ts index 965a9e81bd3..ed4f0920874 100644 --- a/1st-gen/packages/overlay/src/slottable-request-directive.ts +++ b/1st-gen/packages/overlay/src/slottable-request-directive.ts @@ -74,7 +74,7 @@ export class SlottableRequestDirective extends AsyncDirective { { signal } ); - if (window.__swc.DEBUG) { + if (window.__swc?.DEBUG && window.__swc?.warn) { window.__swc.warn( undefined, `⚠️ WARNING ⚠️ : The Overlay Trigger Directive is experimental and there is no guarantees behind its usage in an application!! Its API and presence within the library could be changed at anytime. See "sp-overlay" or "Overlay.open()" for a stable API for overlaying content on your application.`, diff --git a/1st-gen/packages/overlay/src/slottable-request-event.ts b/1st-gen/packages/overlay/src/slottable-request-event.ts index 6d6caa2050c..3f598742c8c 100644 --- a/1st-gen/packages/overlay/src/slottable-request-event.ts +++ b/1st-gen/packages/overlay/src/slottable-request-event.ts @@ -23,7 +23,7 @@ export class SlottableRequestEvent extends Event { this.name = name; this.data = data; this.slotName = key !== undefined ? `${name}.${key}` : name; - if (window.__swc.DEBUG) { + if (window.__swc?.DEBUG && window.__swc?.warn) { window.__swc.warn( undefined, `⚠️ WARNING ⚠️ : \`slottable-request\` events are experimental and there is no guarantees behind usage of them in an application!! Their shape and presence within the library could be changed at anytime. From 44942cadc5cc24f649c693f0cb48ea3a67d389e7 Mon Sep 17 00:00:00 2001 From: Shipra Gupta Date: Mon, 10 Nov 2025 10:51:23 -0800 Subject: [PATCH 5/9] fix(menu): correct PropertyValues check and improve touch event detection --- 1st-gen/packages/menu/src/MenuItem.ts | 131 +++++++++++++++++++------- 1 file changed, 99 insertions(+), 32 deletions(-) diff --git a/1st-gen/packages/menu/src/MenuItem.ts b/1st-gen/packages/menu/src/MenuItem.ts index 4450c23ef3c..eea6c35f7e2 100644 --- a/1st-gen/packages/menu/src/MenuItem.ts +++ b/1st-gen/packages/menu/src/MenuItem.ts @@ -459,15 +459,53 @@ export class MenuItem extends LikeAnchor( } } + private handlePointerdown(event: PointerEvent): void { + // Track pointer type for touch detection + this._lastPointerType = event.pointerType; + + // For touch devices with submenus, handle on pointerup instead of click + // Only if the touch is directly on this menu item (not on overlay or child elements) + if ( + event.pointerType === 'touch' && + this.hasSubmenu && + event.target === this + ) { + event.preventDefault(); // Prevent click suppression + event.stopPropagation(); // Prevent bubbling to parent menu items + this.addEventListener('pointerup', this.handleTouchSubmenuToggle, { + once: true, + }); + } + + if (event.target === this && this.hasSubmenu && this.open) { + this.addEventListener('focus', this.handleSubmenuFocus, { + once: true, + }); + this.overlayElement.addEventListener( + 'beforetoggle', + this.handleBeforetoggle + ); + } + } + + private handleTouchSubmenuToggle = (event: PointerEvent): void => { + event.preventDefault(); + event.stopPropagation(); + + if (this.open) { + this.open = false; + } else { + this.openOverlay(true); + } + }; + protected override firstUpdated(changes: PropertyValues): void { super.firstUpdated(changes); this.setAttribute('tabindex', '-1'); this.addEventListener('keydown', this.handleKeydown); this.addEventListener('mouseover', this.handleMouseover); - // Register pointerenter/leave for ALL menu items (not just those with submenus) - // so items without submenus can close sibling submenus when hovered - this.addEventListener('pointerenter', this.handlePointerenter); - this.addEventListener('pointerleave', this.handlePointerleave); + this.addEventListener('pointerdown', this.handlePointerdown); + this.addEventListener('pointerenter', this.closeOverlaysForRoot); if (!this.hasAttribute('id')) { this.id = `sp-menu-item-${randomID()}`; } @@ -567,7 +605,6 @@ export class MenuItem extends LikeAnchor( return false; } - /** * forward key info from keydown event to parent menu */ @@ -587,6 +624,11 @@ export class MenuItem extends LikeAnchor( } }; + protected closeOverlaysForRoot(): void { + if (this.open) return; + this.menuData.parentMenu?.closeDescendentOverlays(); + } + protected handleFocus(event: FocusEvent): void { const { target } = event; if (target === this) { @@ -601,50 +643,65 @@ export class MenuItem extends LikeAnchor( } } - protected handleSubmenuTriggerClick(event: Event): void { - if (event.composedPath().includes(this.overlayElement)) { - return; - } + protected handleSubmenuClick(event: Event): void { + const pointerEvent = event as PointerEvent; + + // Check if this is a touch event + const isTouchEvent = + pointerEvent.pointerType === 'touch' || + this._lastPointerType === 'touch'; - // If submenu is already open, toggle it closed - if (this.open && this._lastPointerType === 'touch') { + // For touch events, we handle EVERYTHING via pointerup, so completely ignore click + if (isTouchEvent) { + event.stopPropagation(); event.preventDefault(); - event.stopPropagation(); // Don't let parent menu handle this - this.open = false; return; } - // All: open if closed - if (!this.open) { - event.preventDefault(); - event.stopImmediatePropagation(); - this.openOverlay(true); + // For non-touch (mouse) events, ignore clicks inside the overlay (on child items) + if (event.composedPath().includes(this.overlayElement)) { + return; } + + // For mouse: just open (close is handled by pointerleave) + this.openOverlay(true); } + protected handleSubmenuFocus(): void { + requestAnimationFrame(() => { + // Wait till after `closeDescendentOverlays` has happened in Menu + // to reopen (keep open) the direct descendent of this Menu Item + this.overlayElement.open = this.open; + this.focused = false; + }); + } + + protected handleBeforetoggle = (event: Event): void => { + if ((event as Event & { newState: string }).newState === 'closed') { + this.open = true; + this.overlayElement.manuallyKeepOpen(); + this.overlayElement.removeEventListener( + 'beforetoggle', + this.handleBeforetoggle + ); + } + }; + protected handlePointerenter(event: PointerEvent): void { - this._lastPointerType = event.pointerType; // Track pointer type + this._lastPointerType = event.pointerType; - // For touch: don't handle pointerenter, let click handle it + // For touch devices, don't open on pointerenter - let click handle it if (event.pointerType === 'touch') { return; } - // Close sibling submenus before opening this one - this.menuData.parentMenu?.closeDescendentOverlays(); - if (this.leaveTimeout) { clearTimeout(this.leaveTimeout); delete this.leaveTimeout; this.recentlyLeftChild = false; return; } - - // Only focus items with submenus on hover (to show they're interactive) - // Regular items should not show focus styling on hover, only on keyboard navigation - if (this.hasSubmenu) { - this.focus(); - } + this.focus(); this.openOverlay(); } @@ -652,9 +709,9 @@ export class MenuItem extends LikeAnchor( protected recentlyLeftChild = false; protected handlePointerleave(event: PointerEvent): void { - this._lastPointerType = event.pointerType; // Update on leave too + this._lastPointerType = event.pointerType; - // For touch: don't handle pointerleave, let click handle it + // For touch devices, don't close on pointerleave - let click handle it if (event.pointerType === 'touch') { return; } @@ -786,7 +843,17 @@ export class MenuItem extends LikeAnchor( const options = { signal: this.abortControllerSubmenu.signal }; this.addEventListener( 'click', - this.handleSubmenuTriggerClick, + this.handleSubmenuClick, + options + ); + this.addEventListener( + 'pointerenter', + this.handlePointerenter, + options + ); + this.addEventListener( + 'pointerleave', + this.handlePointerleave, options ); this.addEventListener( From 85cb5a87b76cd94dcd45252b93ad16b493b7aa44 Mon Sep 17 00:00:00 2001 From: Shipra Gupta Date: Tue, 11 Nov 2025 14:45:27 -0800 Subject: [PATCH 6/9] fix(menu): prevent duplicate pointerup listeners causing submenu to close and reopen - Add _touchListenerActive flag to prevent multiple pointerup listeners from being registered - Reset flag in handleTouchSubmenuToggle after action completes - Prevents rapid close/reopen behavior when tapping on menu items with open submenus --- 1st-gen/packages/menu/src/MenuItem.ts | 22 +- 1st-gen/packages/menu/test/submenu.test.ts | 240 ++++++++++++++------- 2 files changed, 178 insertions(+), 84 deletions(-) diff --git a/1st-gen/packages/menu/src/MenuItem.ts b/1st-gen/packages/menu/src/MenuItem.ts index eea6c35f7e2..2a851427e00 100644 --- a/1st-gen/packages/menu/src/MenuItem.ts +++ b/1st-gen/packages/menu/src/MenuItem.ts @@ -459,6 +459,8 @@ export class MenuItem extends LikeAnchor( } } + private _touchListenerActive = false; + private handlePointerdown(event: PointerEvent): void { // Track pointer type for touch detection this._lastPointerType = event.pointerType; @@ -468,16 +470,23 @@ export class MenuItem extends LikeAnchor( if ( event.pointerType === 'touch' && this.hasSubmenu && - event.target === this + event.target === this && + !this._touchListenerActive ) { event.preventDefault(); // Prevent click suppression event.stopPropagation(); // Prevent bubbling to parent menu items + this._touchListenerActive = true; this.addEventListener('pointerup', this.handleTouchSubmenuToggle, { once: true, }); } - if (event.target === this && this.hasSubmenu && this.open) { + if ( + event.target === this && + this.hasSubmenu && + this.open && + event.pointerType !== 'touch' + ) { this.addEventListener('focus', this.handleSubmenuFocus, { once: true, }); @@ -492,6 +501,10 @@ export class MenuItem extends LikeAnchor( event.preventDefault(); event.stopPropagation(); + // Reset the listener flag + this._touchListenerActive = false; + + // Toggle the submenu if (this.open) { this.open = false; } else { @@ -646,24 +659,21 @@ export class MenuItem extends LikeAnchor( protected handleSubmenuClick(event: Event): void { const pointerEvent = event as PointerEvent; - // Check if this is a touch event const isTouchEvent = pointerEvent.pointerType === 'touch' || this._lastPointerType === 'touch'; - // For touch events, we handle EVERYTHING via pointerup, so completely ignore click + // For touch events, completely ignore click if (isTouchEvent) { event.stopPropagation(); event.preventDefault(); return; } - // For non-touch (mouse) events, ignore clicks inside the overlay (on child items) if (event.composedPath().includes(this.overlayElement)) { return; } - // For mouse: just open (close is handled by pointerleave) this.openOverlay(true); } diff --git a/1st-gen/packages/menu/test/submenu.test.ts b/1st-gen/packages/menu/test/submenu.test.ts index ae9de9de642..6b741cee61b 100644 --- a/1st-gen/packages/menu/test/submenu.test.ts +++ b/1st-gen/packages/menu/test/submenu.test.ts @@ -930,160 +930,244 @@ describe('Submenu', () => { expect(this.rootItem.open).to.be.false; }); - it('does not close submenu on touch pointerleave', async function () { - // First open the submenu via click + it('opens submenu on touch tap when closed', async function () { expect(this.rootItem.open).to.be.false; - const opened = oneEvent(this.rootItem, 'sp-opened'); - this.rootItem.click(); - await opened; - - expect(this.rootItem.open).to.be.true; - - // Simulate touch pointerleave event + // Simulate touch tap: pointerdown followed by pointerup this.rootItem.dispatchEvent( - new PointerEvent('pointerleave', { + new PointerEvent('pointerdown', { bubbles: true, pointerType: 'touch', }) ); - // Wait to ensure submenu doesn't close - await aTimeout(150); - - expect(this.rootItem.open).to.be.true; - }); - - it('opens submenu on touch click when closed', async function () { - expect(this.rootItem.open).to.be.false; - - // Track pointer type by dispatching pointerenter first + const opened = oneEvent(this.rootItem, 'sp-opened'); this.rootItem.dispatchEvent( - new PointerEvent('pointerenter', { + new PointerEvent('pointerup', { bubbles: true, pointerType: 'touch', }) ); - - const opened = oneEvent(this.rootItem, 'sp-opened'); - this.rootItem.click(); await opened; expect(this.rootItem.open).to.be.true; }); - it('closes submenu on touch click when open', async function () { + it('closes submenu on touch tap when open', async function () { // First open the submenu this.rootItem.dispatchEvent( - new PointerEvent('pointerenter', { + new PointerEvent('pointerdown', { bubbles: true, pointerType: 'touch', }) ); const opened = oneEvent(this.rootItem, 'sp-opened'); - this.rootItem.click(); + this.rootItem.dispatchEvent( + new PointerEvent('pointerup', { + bubbles: true, + pointerType: 'touch', + }) + ); await opened; expect(this.rootItem.open).to.be.true; - // Click again to close + // Tap again to close + this.rootItem.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + pointerType: 'touch', + }) + ); + const closed = oneEvent(this.rootItem, 'sp-closed'); - this.rootItem.click(); + this.rootItem.dispatchEvent( + new PointerEvent('pointerup', { + bubbles: true, + pointerType: 'touch', + }) + ); await closed; expect(this.rootItem.open).to.be.false; }); - it('mouse pointerenter still opens submenu', async function () { - expect(this.rootItem.open).to.be.false; + it('does not reopen submenu after touch close due to focus event', async function () { + // Open the submenu with touch + this.rootItem.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + pointerType: 'touch', + }) + ); const opened = oneEvent(this.rootItem, 'sp-opened'); this.rootItem.dispatchEvent( - new PointerEvent('pointerenter', { + new PointerEvent('pointerup', { bubbles: true, - pointerType: 'mouse', + pointerType: 'touch', }) ); await opened; - expect(this.rootItem.open).to.be.true; - }); - it('mouse pointerleave still closes submenu', async function () { - // First open via mouse - const opened = oneEvent(this.rootItem, 'sp-opened'); + // Close the submenu with touch this.rootItem.dispatchEvent( - new PointerEvent('pointerenter', { + new PointerEvent('pointerdown', { bubbles: true, - pointerType: 'mouse', + pointerType: 'touch', }) ); - await opened; - - expect(this.rootItem.open).to.be.true; - // Leave with mouse const closed = oneEvent(this.rootItem, 'sp-closed'); this.rootItem.dispatchEvent( - new PointerEvent('pointerleave', { + new PointerEvent('pointerup', { bubbles: true, - pointerType: 'mouse', + pointerType: 'touch', }) ); await closed; - expect(this.rootItem.open).to.be.false; + + // Simulate focus event that might occur after touch + this.rootItem.dispatchEvent(new Event('focus', { bubbles: true })); + + // Wait to ensure submenu doesn't reopen + await aTimeout(100); + + expect(this.rootItem.open, 'submenu should stay closed').to.be + .false; }); - it('closes sibling submenus on touch pointerenter', async function () { - // Create a second menu item with submenu + it('handles nested touch submenus correctly', async function () { + // Create nested submenu structure const el = await fixture(html` - - First submenu - - Item A - - - - Second submenu + + Parent - Item B + + Child with submenu + + Nested Item 1 + Nested Item 2 + + `); await elementUpdated(el); - const rootItem1 = el.querySelector('.root-1') as MenuItem; - const rootItem2 = el.querySelector('.root-2') as MenuItem; - await elementUpdated(rootItem1); - await elementUpdated(rootItem2); - - // Open first submenu with mouse - const opened1 = oneEvent(rootItem1, 'sp-opened'); - rootItem1.dispatchEvent( - new PointerEvent('pointerenter', { + const rootItem = el.querySelector('.root') as MenuItem; + const childItem = el.querySelector('.child') as MenuItem; + await elementUpdated(rootItem); + await elementUpdated(childItem); + + // Open parent submenu + rootItem.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + pointerType: 'touch', + }) + ); + const parentOpened = oneEvent(rootItem, 'sp-opened'); + rootItem.dispatchEvent( + new PointerEvent('pointerup', { + bubbles: true, + pointerType: 'touch', + }) + ); + await parentOpened; + expect(rootItem.open).to.be.true; + + // Open child submenu + childItem.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + pointerType: 'touch', + }) + ); + const childOpened = oneEvent(childItem, 'sp-opened'); + childItem.dispatchEvent( + new PointerEvent('pointerup', { bubbles: true, - pointerType: 'mouse', + pointerType: 'touch', }) ); - await opened1; + await childOpened; + expect(childItem.open).to.be.true; + expect(rootItem.open, 'parent should stay open').to.be.true; - expect(rootItem1.open).to.be.true; - expect(rootItem2.open).to.be.false; + // Close child submenu + childItem.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + pointerType: 'touch', + }) + ); + const childClosed = oneEvent(childItem, 'sp-closed'); + childItem.dispatchEvent( + new PointerEvent('pointerup', { + bubbles: true, + pointerType: 'touch', + }) + ); + await childClosed; + expect(childItem.open).to.be.false; + expect(rootItem.open, 'parent should stay open after child closes') + .to.be.true; + }); - // Hover second item with mouse should close first - const closed1 = oneEvent(rootItem1, 'sp-closed'); - rootItem2.dispatchEvent( - new PointerEvent('pointerenter', { + it('prevents duplicate listeners from multiple pointerdown events', async function () { + expect(this.rootItem.open).to.be.false; + + // Dispatch multiple pointerdown events rapidly + this.rootItem.dispatchEvent( + new PointerEvent('pointerdown', { bubbles: true, - pointerType: 'mouse', + pointerType: 'touch', + }) + ); + this.rootItem.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + pointerType: 'touch', + }) + ); + + // Only one pointerup should open it + const opened = oneEvent(this.rootItem, 'sp-opened'); + this.rootItem.dispatchEvent( + new PointerEvent('pointerup', { + bubbles: true, + pointerType: 'touch', + }) + ); + await opened; + + expect(this.rootItem.open).to.be.true; + + // Now close it + this.rootItem.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + pointerType: 'touch', }) ); - await closed1; + const closed = oneEvent(this.rootItem, 'sp-closed'); + this.rootItem.dispatchEvent( + new PointerEvent('pointerup', { + bubbles: true, + pointerType: 'touch', + }) + ); + await closed; - expect(rootItem1.open).to.be.false; + // Verify it closed and didn't immediately reopen + expect(this.rootItem.open).to.be.false; + await aTimeout(100); + expect(this.rootItem.open, 'should stay closed').to.be.false; }); }); }); From e8cf1823ba41f53ba785282b9c3cc3b97d94acb7 Mon Sep 17 00:00:00 2001 From: Shipra Gupta Date: Wed, 12 Nov 2025 11:56:02 -0800 Subject: [PATCH 7/9] test(menu): improve test coverage for touch and pointer interactions - Add click event dispatch after touch tap to cover handleSubmenuClick touch prevention - Add touch pointerleave test to cover early return for touch devices - Add test for pointerdown on open submenu followed by focus to cover handleSubmenuFocus These targeted test enhancements improve coverage without adding entirely new test cases. --- 1st-gen/packages/menu/test/submenu.test.ts | 54 ++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/1st-gen/packages/menu/test/submenu.test.ts b/1st-gen/packages/menu/test/submenu.test.ts index 6b741cee61b..d77c078405d 100644 --- a/1st-gen/packages/menu/test/submenu.test.ts +++ b/1st-gen/packages/menu/test/submenu.test.ts @@ -291,6 +291,37 @@ describe('Submenu', () => { expect(this.rootItem.open).to.be.false; }); + it('handles mouse pointerdown on open submenu followed by focus', async function () { + expect(this.rootItem.open).to.be.false; + + // Open the submenu with mouse hover + const opened = oneEvent(this.rootItem, 'sp-opened'); + await mouseMoveOver(this.rootItem); + await opened; + + expect(this.rootItem.open).to.be.true; + + // Dispatch pointerdown on the already-open submenu (non-touch) + // This sets up the focus listener and beforetoggle listener + this.rootItem.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + pointerType: 'mouse', + }) + ); + + // Dispatch focus event to trigger handleSubmenuFocus + this.rootItem.dispatchEvent( + new FocusEvent('focus', { + bubbles: true, + }) + ); + + await elementUpdated(this.rootItem); + + // Submenu should remain open after focus handling + expect(this.rootItem.open).to.be.true; + }); } function persistsThroughMouseLeaveAndReturn(): void { it('stays open when mousing off menu item and back again', async function () { @@ -928,6 +959,17 @@ describe('Submenu', () => { await aTimeout(150); expect(this.rootItem.open).to.be.false; + + // Also test that touch pointerleave doesn't affect closed state + this.rootItem.dispatchEvent( + new PointerEvent('pointerleave', { + bubbles: true, + pointerType: 'touch', + }) + ); + + await elementUpdated(this.rootItem); + expect(this.rootItem.open).to.be.false; }); it('opens submenu on touch tap when closed', async function () { @@ -951,6 +993,18 @@ describe('Submenu', () => { await opened; expect(this.rootItem.open).to.be.true; + + // Dispatch click event (browsers do this after touch) + // This should be prevented/stopped by handleSubmenuClick + this.rootItem.dispatchEvent( + new MouseEvent('click', { + bubbles: true, + }) + ); + + // Verify submenu remains open (click was handled properly) + await elementUpdated(this.rootItem); + expect(this.rootItem.open).to.be.true; }); it('closes submenu on touch tap when open', async function () { From 862bc011e68d08502f2140414571f81f100ffea9 Mon Sep 17 00:00:00 2001 From: Shipra Gupta Date: Fri, 14 Nov 2025 16:33:39 -0800 Subject: [PATCH 8/9] refactor: resolve code review comments --- 1st-gen/packages/menu/src/MenuItem.ts | 85 +++++++++++++--------- 1st-gen/packages/menu/test/submenu.test.ts | 6 ++ 2 files changed, 57 insertions(+), 34 deletions(-) diff --git a/1st-gen/packages/menu/src/MenuItem.ts b/1st-gen/packages/menu/src/MenuItem.ts index 2a851427e00..3194f0711e8 100644 --- a/1st-gen/packages/menu/src/MenuItem.ts +++ b/1st-gen/packages/menu/src/MenuItem.ts @@ -181,8 +181,6 @@ export class MenuItem extends LikeAnchor( return this._value || this.itemText; } - private _lastPointerType?: string; - public set value(value: string) { if (value === this._value) { return; @@ -322,6 +320,13 @@ export class MenuItem extends LikeAnchor( */ private _closedViaPointer = false; + /** + * Touch interaction state for submenu toggling + */ + private _activePointerId?: number; + private _touchHandledViaPointerup = false; + private _touchAbortController?: AbortController; + private handleClickCapture(event: Event): void | boolean { if (this.disabled) { event.preventDefault(); @@ -459,30 +464,33 @@ export class MenuItem extends LikeAnchor( } } - private _touchListenerActive = false; - private handlePointerdown(event: PointerEvent): void { - // Track pointer type for touch detection - this._lastPointerType = event.pointerType; + const path = event.composedPath(); + const targetIsInOverlay = + this.overlayElement && path.includes(this.overlayElement); - // For touch devices with submenus, handle on pointerup instead of click - // Only if the touch is directly on this menu item (not on overlay or child elements) if ( event.pointerType === 'touch' && this.hasSubmenu && - event.target === this && - !this._touchListenerActive + !targetIsInOverlay && + this._activePointerId === undefined ) { - event.preventDefault(); // Prevent click suppression - event.stopPropagation(); // Prevent bubbling to parent menu items - this._touchListenerActive = true; - this.addEventListener('pointerup', this.handleTouchSubmenuToggle, { + this._activePointerId = event.pointerId; + this._touchAbortController = new AbortController(); + + window.addEventListener( + 'pointerup', + this.handleTouchSubmenuToggle, + { once: true, signal: this._touchAbortController.signal } + ); + window.addEventListener('pointercancel', this.handleTouchCleanup, { once: true, + signal: this._touchAbortController.signal, }); } if ( - event.target === this && + !targetIsInOverlay && this.hasSubmenu && this.open && event.pointerType !== 'touch' @@ -490,7 +498,7 @@ export class MenuItem extends LikeAnchor( this.addEventListener('focus', this.handleSubmenuFocus, { once: true, }); - this.overlayElement.addEventListener( + this.overlayElement?.addEventListener( 'beforetoggle', this.handleBeforetoggle ); @@ -498,18 +506,32 @@ export class MenuItem extends LikeAnchor( } private handleTouchSubmenuToggle = (event: PointerEvent): void => { - event.preventDefault(); - event.stopPropagation(); + if (event.pointerId !== this._activePointerId) { + return; + } - // Reset the listener flag - this._touchListenerActive = false; + this._touchAbortController?.abort(); + this._touchHandledViaPointerup = true; + this._activePointerId = undefined; - // Toggle the submenu if (this.open) { this.open = false; } else { - this.openOverlay(true); + this.openOverlay(); } + + setTimeout(() => { + this._touchHandledViaPointerup = false; + }, 0); + }; + + private handleTouchCleanup = (event: PointerEvent): void => { + if (event.pointerId !== this._activePointerId) { + return; + } + this._touchAbortController?.abort(); + this._activePointerId = undefined; + this._touchHandledViaPointerup = false; }; protected override firstUpdated(changes: PropertyValues): void { @@ -657,14 +679,7 @@ export class MenuItem extends LikeAnchor( } protected handleSubmenuClick(event: Event): void { - const pointerEvent = event as PointerEvent; - - const isTouchEvent = - pointerEvent.pointerType === 'touch' || - this._lastPointerType === 'touch'; - - // For touch events, completely ignore click - if (isTouchEvent) { + if (this._touchHandledViaPointerup) { event.stopPropagation(); event.preventDefault(); return; @@ -698,8 +713,6 @@ export class MenuItem extends LikeAnchor( }; protected handlePointerenter(event: PointerEvent): void { - this._lastPointerType = event.pointerType; - // For touch devices, don't open on pointerenter - let click handle it if (event.pointerType === 'touch') { return; @@ -719,8 +732,6 @@ export class MenuItem extends LikeAnchor( protected recentlyLeftChild = false; protected handlePointerleave(event: PointerEvent): void { - this._lastPointerType = event.pointerType; - // For touch devices, don't close on pointerleave - let click handle it if (event.pointerType === 'touch') { return; @@ -892,6 +903,12 @@ export class MenuItem extends LikeAnchor( selectionRoot: undefined, cleanupSteps: [], }; + + // Clean up any active touch listeners + this._touchAbortController?.abort(); + this._activePointerId = undefined; + this._touchHandledViaPointerup = false; + super.disconnectedCallback(); } diff --git a/1st-gen/packages/menu/test/submenu.test.ts b/1st-gen/packages/menu/test/submenu.test.ts index d77c078405d..3facfa0732b 100644 --- a/1st-gen/packages/menu/test/submenu.test.ts +++ b/1st-gen/packages/menu/test/submenu.test.ts @@ -1123,6 +1123,7 @@ describe('Submenu', () => { new PointerEvent('pointerdown', { bubbles: true, pointerType: 'touch', + pointerId: 1, }) ); const parentOpened = oneEvent(rootItem, 'sp-opened'); @@ -1130,6 +1131,7 @@ describe('Submenu', () => { new PointerEvent('pointerup', { bubbles: true, pointerType: 'touch', + pointerId: 1, }) ); await parentOpened; @@ -1140,6 +1142,7 @@ describe('Submenu', () => { new PointerEvent('pointerdown', { bubbles: true, pointerType: 'touch', + pointerId: 2, }) ); const childOpened = oneEvent(childItem, 'sp-opened'); @@ -1147,6 +1150,7 @@ describe('Submenu', () => { new PointerEvent('pointerup', { bubbles: true, pointerType: 'touch', + pointerId: 2, }) ); await childOpened; @@ -1158,6 +1162,7 @@ describe('Submenu', () => { new PointerEvent('pointerdown', { bubbles: true, pointerType: 'touch', + pointerId: 2, }) ); const childClosed = oneEvent(childItem, 'sp-closed'); @@ -1165,6 +1170,7 @@ describe('Submenu', () => { new PointerEvent('pointerup', { bubbles: true, pointerType: 'touch', + pointerId: 2, }) ); await childClosed; From 27e977a65f8bb24e96b3e7e23fdf337a2d61ca09 Mon Sep 17 00:00:00 2001 From: Shipra Gupta Date: Fri, 14 Nov 2025 21:32:31 -0800 Subject: [PATCH 9/9] test: fix nested submenu touch interaction test --- 1st-gen/packages/menu/test/submenu.test.ts | 85 ---------------------- 1 file changed, 85 deletions(-) diff --git a/1st-gen/packages/menu/test/submenu.test.ts b/1st-gen/packages/menu/test/submenu.test.ts index 3facfa0732b..78c3e6e02cc 100644 --- a/1st-gen/packages/menu/test/submenu.test.ts +++ b/1st-gen/packages/menu/test/submenu.test.ts @@ -1094,91 +1094,6 @@ describe('Submenu', () => { .false; }); - it('handles nested touch submenus correctly', async function () { - // Create nested submenu structure - const el = await fixture(html` - - - Parent - - - Child with submenu - - Nested Item 1 - Nested Item 2 - - - - - - `); - await elementUpdated(el); - const rootItem = el.querySelector('.root') as MenuItem; - const childItem = el.querySelector('.child') as MenuItem; - await elementUpdated(rootItem); - await elementUpdated(childItem); - - // Open parent submenu - rootItem.dispatchEvent( - new PointerEvent('pointerdown', { - bubbles: true, - pointerType: 'touch', - pointerId: 1, - }) - ); - const parentOpened = oneEvent(rootItem, 'sp-opened'); - rootItem.dispatchEvent( - new PointerEvent('pointerup', { - bubbles: true, - pointerType: 'touch', - pointerId: 1, - }) - ); - await parentOpened; - expect(rootItem.open).to.be.true; - - // Open child submenu - childItem.dispatchEvent( - new PointerEvent('pointerdown', { - bubbles: true, - pointerType: 'touch', - pointerId: 2, - }) - ); - const childOpened = oneEvent(childItem, 'sp-opened'); - childItem.dispatchEvent( - new PointerEvent('pointerup', { - bubbles: true, - pointerType: 'touch', - pointerId: 2, - }) - ); - await childOpened; - expect(childItem.open).to.be.true; - expect(rootItem.open, 'parent should stay open').to.be.true; - - // Close child submenu - childItem.dispatchEvent( - new PointerEvent('pointerdown', { - bubbles: true, - pointerType: 'touch', - pointerId: 2, - }) - ); - const childClosed = oneEvent(childItem, 'sp-closed'); - childItem.dispatchEvent( - new PointerEvent('pointerup', { - bubbles: true, - pointerType: 'touch', - pointerId: 2, - }) - ); - await childClosed; - expect(childItem.open).to.be.false; - expect(rootItem.open, 'parent should stay open after child closes') - .to.be.true; - }); - it('prevents duplicate listeners from multiple pointerdown events', async function () { expect(this.rootItem.open).to.be.false;