Skip to content

Commit eac97a2

Browse files
shipg22Shipra Guptanikkimk
authored
fix: improve touch interaction handling for submenus (#5867)
* fix: improve touch interaction handling for submenus * chore: add changeset for menu touch interaction fix * test: add comprehensive touch interaction tests for menu submenus * fix(overlay): add null checks for window.__swc.warn in test environments * fix(menu): correct PropertyValues check and improve touch event detection * 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 * 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. * refactor: resolve code review comments * test: fix nested submenu touch interaction test --------- Co-authored-by: Shipra Gupta <[email protected]> Co-authored-by: Nikki Massaro <[email protected]>
1 parent 7eb0007 commit eac97a2

File tree

5 files changed

+357
-6
lines changed

5 files changed

+357
-6
lines changed

.changeset/brave-foxes-smile.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@spectrum-web-components/menu': patch
3+
---
4+
5+
**Fixed**: Improved touch interaction handling for submenus to prevent unintended submenu closures.

1st-gen/packages/menu/src/MenuItem.ts

Lines changed: 92 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,13 @@ export class MenuItem extends LikeAnchor(
320320
*/
321321
private _closedViaPointer = false;
322322

323+
/**
324+
* Touch interaction state for submenu toggling
325+
*/
326+
private _activePointerId?: number;
327+
private _touchHandledViaPointerup = false;
328+
private _touchAbortController?: AbortController;
329+
323330
private handleClickCapture(event: Event): void | boolean {
324331
if (this.disabled) {
325332
event.preventDefault();
@@ -458,17 +465,75 @@ export class MenuItem extends LikeAnchor(
458465
}
459466

460467
private handlePointerdown(event: PointerEvent): void {
461-
if (event.target === this && this.hasSubmenu && this.open) {
468+
const path = event.composedPath();
469+
const targetIsInOverlay =
470+
this.overlayElement && path.includes(this.overlayElement);
471+
472+
if (
473+
event.pointerType === 'touch' &&
474+
this.hasSubmenu &&
475+
!targetIsInOverlay &&
476+
this._activePointerId === undefined
477+
) {
478+
this._activePointerId = event.pointerId;
479+
this._touchAbortController = new AbortController();
480+
481+
window.addEventListener(
482+
'pointerup',
483+
this.handleTouchSubmenuToggle,
484+
{ once: true, signal: this._touchAbortController.signal }
485+
);
486+
window.addEventListener('pointercancel', this.handleTouchCleanup, {
487+
once: true,
488+
signal: this._touchAbortController.signal,
489+
});
490+
}
491+
492+
if (
493+
!targetIsInOverlay &&
494+
this.hasSubmenu &&
495+
this.open &&
496+
event.pointerType !== 'touch'
497+
) {
462498
this.addEventListener('focus', this.handleSubmenuFocus, {
463499
once: true,
464500
});
465-
this.overlayElement.addEventListener(
501+
this.overlayElement?.addEventListener(
466502
'beforetoggle',
467503
this.handleBeforetoggle
468504
);
469505
}
470506
}
471507

508+
private handleTouchSubmenuToggle = (event: PointerEvent): void => {
509+
if (event.pointerId !== this._activePointerId) {
510+
return;
511+
}
512+
513+
this._touchAbortController?.abort();
514+
this._touchHandledViaPointerup = true;
515+
this._activePointerId = undefined;
516+
517+
if (this.open) {
518+
this.open = false;
519+
} else {
520+
this.openOverlay();
521+
}
522+
523+
setTimeout(() => {
524+
this._touchHandledViaPointerup = false;
525+
}, 0);
526+
};
527+
528+
private handleTouchCleanup = (event: PointerEvent): void => {
529+
if (event.pointerId !== this._activePointerId) {
530+
return;
531+
}
532+
this._touchAbortController?.abort();
533+
this._activePointerId = undefined;
534+
this._touchHandledViaPointerup = false;
535+
};
536+
472537
protected override firstUpdated(changes: PropertyValues): void {
473538
super.firstUpdated(changes);
474539
this.setAttribute('tabindex', '-1');
@@ -614,9 +679,16 @@ export class MenuItem extends LikeAnchor(
614679
}
615680

616681
protected handleSubmenuClick(event: Event): void {
682+
if (this._touchHandledViaPointerup) {
683+
event.stopPropagation();
684+
event.preventDefault();
685+
return;
686+
}
687+
617688
if (event.composedPath().includes(this.overlayElement)) {
618689
return;
619690
}
691+
620692
this.openOverlay(true);
621693
}
622694

@@ -640,7 +712,12 @@ export class MenuItem extends LikeAnchor(
640712
}
641713
};
642714

643-
protected handlePointerenter(): void {
715+
protected handlePointerenter(event: PointerEvent): void {
716+
// For touch devices, don't open on pointerenter - let click handle it
717+
if (event.pointerType === 'touch') {
718+
return;
719+
}
720+
644721
if (this.leaveTimeout) {
645722
clearTimeout(this.leaveTimeout);
646723
delete this.leaveTimeout;
@@ -654,7 +731,12 @@ export class MenuItem extends LikeAnchor(
654731
protected leaveTimeout?: ReturnType<typeof setTimeout>;
655732
protected recentlyLeftChild = false;
656733

657-
protected handlePointerleave(): void {
734+
protected handlePointerleave(event: PointerEvent): void {
735+
// For touch devices, don't close on pointerleave - let click handle it
736+
if (event.pointerType === 'touch') {
737+
return;
738+
}
739+
658740
this._closedViaPointer = true;
659741
if (this.open && !this.recentlyLeftChild) {
660742
this.leaveTimeout = setTimeout(() => {
@@ -821,6 +903,12 @@ export class MenuItem extends LikeAnchor(
821903
selectionRoot: undefined,
822904
cleanupSteps: [],
823905
};
906+
907+
// Clean up any active touch listeners
908+
this._touchAbortController?.abort();
909+
this._activePointerId = undefined;
910+
this._touchHandledViaPointerup = false;
911+
824912
super.disconnectedCallback();
825913
}
826914

0 commit comments

Comments
 (0)