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. diff --git a/1st-gen/packages/menu/src/MenuItem.ts b/1st-gen/packages/menu/src/MenuItem.ts index 75de1e0de3d..3194f0711e8 100644 --- a/1st-gen/packages/menu/src/MenuItem.ts +++ b/1st-gen/packages/menu/src/MenuItem.ts @@ -320,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(); @@ -458,17 +465,75 @@ export class MenuItem extends LikeAnchor( } private handlePointerdown(event: PointerEvent): void { - if (event.target === this && this.hasSubmenu && this.open) { + const path = event.composedPath(); + const targetIsInOverlay = + this.overlayElement && path.includes(this.overlayElement); + + if ( + event.pointerType === 'touch' && + this.hasSubmenu && + !targetIsInOverlay && + this._activePointerId === undefined + ) { + 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 ( + !targetIsInOverlay && + this.hasSubmenu && + this.open && + event.pointerType !== 'touch' + ) { this.addEventListener('focus', this.handleSubmenuFocus, { once: true, }); - this.overlayElement.addEventListener( + this.overlayElement?.addEventListener( 'beforetoggle', this.handleBeforetoggle ); } } + private handleTouchSubmenuToggle = (event: PointerEvent): void => { + if (event.pointerId !== this._activePointerId) { + return; + } + + this._touchAbortController?.abort(); + this._touchHandledViaPointerup = true; + this._activePointerId = undefined; + + if (this.open) { + this.open = false; + } else { + 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 { super.firstUpdated(changes); this.setAttribute('tabindex', '-1'); @@ -614,9 +679,16 @@ export class MenuItem extends LikeAnchor( } protected handleSubmenuClick(event: Event): void { + if (this._touchHandledViaPointerup) { + event.stopPropagation(); + event.preventDefault(); + return; + } + if (event.composedPath().includes(this.overlayElement)) { return; } + this.openOverlay(true); } @@ -640,7 +712,12 @@ export class MenuItem extends LikeAnchor( } }; - protected handlePointerenter(): void { + protected handlePointerenter(event: PointerEvent): void { + // For touch devices, don't open on pointerenter - let click handle it + if (event.pointerType === 'touch') { + return; + } + if (this.leaveTimeout) { clearTimeout(this.leaveTimeout); delete this.leaveTimeout; @@ -654,7 +731,12 @@ export class MenuItem extends LikeAnchor( protected leaveTimeout?: ReturnType; protected recentlyLeftChild = false; - protected handlePointerleave(): void { + protected handlePointerleave(event: PointerEvent): void { + // For touch devices, don't close on pointerleave - let click handle it + if (event.pointerType === 'touch') { + return; + } + this._closedViaPointer = true; if (this.open && !this.recentlyLeftChild) { this.leaveTimeout = setTimeout(() => { @@ -821,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 e3dde7ef127..78c3e6e02cc 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 () { @@ -887,4 +918,231 @@ 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; + + // 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 () { + expect(this.rootItem.open).to.be.false; + + // Simulate touch tap: pointerdown followed by pointerup + this.rootItem.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + pointerType: 'touch', + }) + ); + + 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; + + // 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 () { + // First open the submenu + this.rootItem.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + pointerType: 'touch', + }) + ); + + 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; + + // Tap again to close + this.rootItem.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + pointerType: 'touch', + }) + ); + + const closed = oneEvent(this.rootItem, 'sp-closed'); + this.rootItem.dispatchEvent( + new PointerEvent('pointerup', { + bubbles: true, + pointerType: 'touch', + }) + ); + await closed; + + 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('pointerup', { + bubbles: true, + pointerType: 'touch', + }) + ); + await opened; + expect(this.rootItem.open).to.be.true; + + // Close the submenu with touch + this.rootItem.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + pointerType: 'touch', + }) + ); + + const closed = oneEvent(this.rootItem, 'sp-closed'); + this.rootItem.dispatchEvent( + new PointerEvent('pointerup', { + bubbles: true, + 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('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: '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', + }) + ); + const closed = oneEvent(this.rootItem, 'sp-closed'); + this.rootItem.dispatchEvent( + new PointerEvent('pointerup', { + bubbles: true, + pointerType: 'touch', + }) + ); + await closed; + + // 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; + }); + }); }); 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.