Skip to content

Commit 4e02eb4

Browse files
author
Shipra Gupta
committed
refactor: resolve code review comments
1 parent e8cf182 commit 4e02eb4

File tree

1 file changed

+47
-33
lines changed

1 file changed

+47
-33
lines changed

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

Lines changed: 47 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,6 @@ export class MenuItem extends LikeAnchor(
181181
return this._value || this.itemText;
182182
}
183183

184-
private _lastPointerType?: string;
185-
186184
public set value(value: string) {
187185
if (value === this._value) {
188186
return;
@@ -459,57 +457,78 @@ export class MenuItem extends LikeAnchor(
459457
}
460458
}
461459

462-
private _touchListenerActive = false;
460+
private _activePointerId?: number;
461+
private _touchHandledViaPointerup = false;
462+
private _touchAbortController?: AbortController;
463463

464464
private handlePointerdown(event: PointerEvent): void {
465-
// Track pointer type for touch detection
466-
this._lastPointerType = event.pointerType;
465+
const path = event.composedPath();
466+
const targetIsInOverlay =
467+
this.overlayElement && path.includes(this.overlayElement);
467468

468-
// For touch devices with submenus, handle on pointerup instead of click
469-
// Only if the touch is directly on this menu item (not on overlay or child elements)
470469
if (
471470
event.pointerType === 'touch' &&
472471
this.hasSubmenu &&
473-
event.target === this &&
474-
!this._touchListenerActive
472+
!targetIsInOverlay &&
473+
this._activePointerId === undefined
475474
) {
476-
event.preventDefault(); // Prevent click suppression
477-
event.stopPropagation(); // Prevent bubbling to parent menu items
478-
this._touchListenerActive = true;
479-
this.addEventListener('pointerup', this.handleTouchSubmenuToggle, {
475+
this._activePointerId = event.pointerId;
476+
this._touchAbortController = new AbortController();
477+
478+
window.addEventListener(
479+
'pointerup',
480+
this.handleTouchSubmenuToggle,
481+
{ once: true, signal: this._touchAbortController.signal }
482+
);
483+
window.addEventListener('pointercancel', this.handleTouchCleanup, {
480484
once: true,
485+
signal: this._touchAbortController.signal,
481486
});
482487
}
483488

484489
if (
485-
event.target === this &&
490+
!targetIsInOverlay &&
486491
this.hasSubmenu &&
487492
this.open &&
488493
event.pointerType !== 'touch'
489494
) {
490495
this.addEventListener('focus', this.handleSubmenuFocus, {
491496
once: true,
492497
});
493-
this.overlayElement.addEventListener(
498+
this.overlayElement?.addEventListener(
494499
'beforetoggle',
495500
this.handleBeforetoggle
496501
);
497502
}
498503
}
499504

500505
private handleTouchSubmenuToggle = (event: PointerEvent): void => {
501-
event.preventDefault();
502-
event.stopPropagation();
506+
if (event.pointerId !== this._activePointerId) {
507+
return;
508+
}
503509

504-
// Reset the listener flag
505-
this._touchListenerActive = false;
510+
this._touchAbortController?.abort();
511+
this._touchHandledViaPointerup = true;
512+
this._activePointerId = undefined;
506513

507-
// Toggle the submenu
508514
if (this.open) {
509515
this.open = false;
510516
} else {
511-
this.openOverlay(true);
517+
this.openOverlay();
512518
}
519+
520+
setTimeout(() => {
521+
this._touchHandledViaPointerup = false;
522+
}, 0);
523+
};
524+
525+
private handleTouchCleanup = (event: PointerEvent): void => {
526+
if (event.pointerId !== this._activePointerId) {
527+
return;
528+
}
529+
this._touchAbortController?.abort();
530+
this._activePointerId = undefined;
531+
this._touchHandledViaPointerup = false;
513532
};
514533

515534
protected override firstUpdated(changes: PropertyValues): void {
@@ -657,14 +676,7 @@ export class MenuItem extends LikeAnchor(
657676
}
658677

659678
protected handleSubmenuClick(event: Event): void {
660-
const pointerEvent = event as PointerEvent;
661-
662-
const isTouchEvent =
663-
pointerEvent.pointerType === 'touch' ||
664-
this._lastPointerType === 'touch';
665-
666-
// For touch events, completely ignore click
667-
if (isTouchEvent) {
679+
if (this._touchHandledViaPointerup) {
668680
event.stopPropagation();
669681
event.preventDefault();
670682
return;
@@ -698,8 +710,6 @@ export class MenuItem extends LikeAnchor(
698710
};
699711

700712
protected handlePointerenter(event: PointerEvent): void {
701-
this._lastPointerType = event.pointerType;
702-
703713
// For touch devices, don't open on pointerenter - let click handle it
704714
if (event.pointerType === 'touch') {
705715
return;
@@ -719,8 +729,6 @@ export class MenuItem extends LikeAnchor(
719729
protected recentlyLeftChild = false;
720730

721731
protected handlePointerleave(event: PointerEvent): void {
722-
this._lastPointerType = event.pointerType;
723-
724732
// For touch devices, don't close on pointerleave - let click handle it
725733
if (event.pointerType === 'touch') {
726734
return;
@@ -892,6 +900,12 @@ export class MenuItem extends LikeAnchor(
892900
selectionRoot: undefined,
893901
cleanupSteps: [],
894902
};
903+
904+
// Clean up any active touch listeners
905+
this._touchAbortController?.abort();
906+
this._activePointerId = undefined;
907+
this._touchHandledViaPointerup = false;
908+
895909
super.disconnectedCallback();
896910
}
897911

0 commit comments

Comments
 (0)