fix: improve touch interaction handling for submenus

.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.

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<typeof setTimeout>;
     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(

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<Menu>(html`
+                <sp-menu>
+                    <sp-menu-item>No submenu</sp-menu-item>
+                    <sp-menu-item class="root">
+                        Has submenu
+                        <sp-menu slot="submenu">
+                            <sp-menu-item class="submenu-item-1">
+                                One
+                            </sp-menu-item>
+                            <sp-menu-item class="submenu-item-2">
+                                Two
+                            </sp-menu-item>
+                            <sp-menu-item class="submenu-item-3">
+                                Three
+                            </sp-menu-item>
+                        </sp-menu>
+                    </sp-menu-item>
+                </sp-menu>
+            `);
+            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<Menu>(html`
+                <sp-menu>
+                    <sp-menu-item class="root-1">
+                        First submenu
+                        <sp-menu slot="submenu">
+                            <sp-menu-item>Item A</sp-menu-item>
+                        </sp-menu>
+                    </sp-menu-item>
+                    <sp-menu-item class="root-2">
+                        Second submenu
+                        <sp-menu slot="submenu">
+                            <sp-menu-item>Item B</sp-menu-item>
+                        </sp-menu>
+                    </sp-menu-item>
+                </sp-menu>
+            `);
+            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;
+        });
+    });
 });