Skip to content

Commit 85cb5a8

Browse files
author
Shipra Gupta
committed
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
1 parent 44942ca commit 85cb5a8

File tree

2 files changed

+178
-84
lines changed

2 files changed

+178
-84
lines changed

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,8 @@ export class MenuItem extends LikeAnchor(
459459
}
460460
}
461461

462+
private _touchListenerActive = false;
463+
462464
private handlePointerdown(event: PointerEvent): void {
463465
// Track pointer type for touch detection
464466
this._lastPointerType = event.pointerType;
@@ -468,16 +470,23 @@ export class MenuItem extends LikeAnchor(
468470
if (
469471
event.pointerType === 'touch' &&
470472
this.hasSubmenu &&
471-
event.target === this
473+
event.target === this &&
474+
!this._touchListenerActive
472475
) {
473476
event.preventDefault(); // Prevent click suppression
474477
event.stopPropagation(); // Prevent bubbling to parent menu items
478+
this._touchListenerActive = true;
475479
this.addEventListener('pointerup', this.handleTouchSubmenuToggle, {
476480
once: true,
477481
});
478482
}
479483

480-
if (event.target === this && this.hasSubmenu && this.open) {
484+
if (
485+
event.target === this &&
486+
this.hasSubmenu &&
487+
this.open &&
488+
event.pointerType !== 'touch'
489+
) {
481490
this.addEventListener('focus', this.handleSubmenuFocus, {
482491
once: true,
483492
});
@@ -492,6 +501,10 @@ export class MenuItem extends LikeAnchor(
492501
event.preventDefault();
493502
event.stopPropagation();
494503

504+
// Reset the listener flag
505+
this._touchListenerActive = false;
506+
507+
// Toggle the submenu
495508
if (this.open) {
496509
this.open = false;
497510
} else {
@@ -646,24 +659,21 @@ export class MenuItem extends LikeAnchor(
646659
protected handleSubmenuClick(event: Event): void {
647660
const pointerEvent = event as PointerEvent;
648661

649-
// Check if this is a touch event
650662
const isTouchEvent =
651663
pointerEvent.pointerType === 'touch' ||
652664
this._lastPointerType === 'touch';
653665

654-
// For touch events, we handle EVERYTHING via pointerup, so completely ignore click
666+
// For touch events, completely ignore click
655667
if (isTouchEvent) {
656668
event.stopPropagation();
657669
event.preventDefault();
658670
return;
659671
}
660672

661-
// For non-touch (mouse) events, ignore clicks inside the overlay (on child items)
662673
if (event.composedPath().includes(this.overlayElement)) {
663674
return;
664675
}
665676

666-
// For mouse: just open (close is handled by pointerleave)
667677
this.openOverlay(true);
668678
}
669679

‎1st-gen/packages/menu/test/submenu.test.ts‎

Lines changed: 162 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -930,160 +930,244 @@ describe('Submenu', () => {
930930
expect(this.rootItem.open).to.be.false;
931931
});
932932

933-
it('does not close submenu on touch pointerleave', async function () {
934-
// First open the submenu via click
933+
it('opens submenu on touch tap when closed', async function () {
935934
expect(this.rootItem.open).to.be.false;
936935

937-
const opened = oneEvent(this.rootItem, 'sp-opened');
938-
this.rootItem.click();
939-
await opened;
940-
941-
expect(this.rootItem.open).to.be.true;
942-
943-
// Simulate touch pointerleave event
936+
// Simulate touch tap: pointerdown followed by pointerup
944937
this.rootItem.dispatchEvent(
945-
new PointerEvent('pointerleave', {
938+
new PointerEvent('pointerdown', {
946939
bubbles: true,
947940
pointerType: 'touch',
948941
})
949942
);
950943

951-
// Wait to ensure submenu doesn't close
952-
await aTimeout(150);
953-
954-
expect(this.rootItem.open).to.be.true;
955-
});
956-
957-
it('opens submenu on touch click when closed', async function () {
958-
expect(this.rootItem.open).to.be.false;
959-
960-
// Track pointer type by dispatching pointerenter first
944+
const opened = oneEvent(this.rootItem, 'sp-opened');
961945
this.rootItem.dispatchEvent(
962-
new PointerEvent('pointerenter', {
946+
new PointerEvent('pointerup', {
963947
bubbles: true,
964948
pointerType: 'touch',
965949
})
966950
);
967-
968-
const opened = oneEvent(this.rootItem, 'sp-opened');
969-
this.rootItem.click();
970951
await opened;
971952

972953
expect(this.rootItem.open).to.be.true;
973954
});
974955

975-
it('closes submenu on touch click when open', async function () {
956+
it('closes submenu on touch tap when open', async function () {
976957
// First open the submenu
977958
this.rootItem.dispatchEvent(
978-
new PointerEvent('pointerenter', {
959+
new PointerEvent('pointerdown', {
979960
bubbles: true,
980961
pointerType: 'touch',
981962
})
982963
);
983964

984965
const opened = oneEvent(this.rootItem, 'sp-opened');
985-
this.rootItem.click();
966+
this.rootItem.dispatchEvent(
967+
new PointerEvent('pointerup', {
968+
bubbles: true,
969+
pointerType: 'touch',
970+
})
971+
);
986972
await opened;
987973

988974
expect(this.rootItem.open).to.be.true;
989975

990-
// Click again to close
976+
// Tap again to close
977+
this.rootItem.dispatchEvent(
978+
new PointerEvent('pointerdown', {
979+
bubbles: true,
980+
pointerType: 'touch',
981+
})
982+
);
983+
991984
const closed = oneEvent(this.rootItem, 'sp-closed');
992-
this.rootItem.click();
985+
this.rootItem.dispatchEvent(
986+
new PointerEvent('pointerup', {
987+
bubbles: true,
988+
pointerType: 'touch',
989+
})
990+
);
993991
await closed;
994992

995993
expect(this.rootItem.open).to.be.false;
996994
});
997995

998-
it('mouse pointerenter still opens submenu', async function () {
999-
expect(this.rootItem.open).to.be.false;
996+
it('does not reopen submenu after touch close due to focus event', async function () {
997+
// Open the submenu with touch
998+
this.rootItem.dispatchEvent(
999+
new PointerEvent('pointerdown', {
1000+
bubbles: true,
1001+
pointerType: 'touch',
1002+
})
1003+
);
10001004

10011005
const opened = oneEvent(this.rootItem, 'sp-opened');
10021006
this.rootItem.dispatchEvent(
1003-
new PointerEvent('pointerenter', {
1007+
new PointerEvent('pointerup', {
10041008
bubbles: true,
1005-
pointerType: 'mouse',
1009+
pointerType: 'touch',
10061010
})
10071011
);
10081012
await opened;
1009-
10101013
expect(this.rootItem.open).to.be.true;
1011-
});
10121014

1013-
it('mouse pointerleave still closes submenu', async function () {
1014-
// First open via mouse
1015-
const opened = oneEvent(this.rootItem, 'sp-opened');
1015+
// Close the submenu with touch
10161016
this.rootItem.dispatchEvent(
1017-
new PointerEvent('pointerenter', {
1017+
new PointerEvent('pointerdown', {
10181018
bubbles: true,
1019-
pointerType: 'mouse',
1019+
pointerType: 'touch',
10201020
})
10211021
);
1022-
await opened;
1023-
1024-
expect(this.rootItem.open).to.be.true;
10251022

1026-
// Leave with mouse
10271023
const closed = oneEvent(this.rootItem, 'sp-closed');
10281024
this.rootItem.dispatchEvent(
1029-
new PointerEvent('pointerleave', {
1025+
new PointerEvent('pointerup', {
10301026
bubbles: true,
1031-
pointerType: 'mouse',
1027+
pointerType: 'touch',
10321028
})
10331029
);
10341030
await closed;
1035-
10361031
expect(this.rootItem.open).to.be.false;
1032+
1033+
// Simulate focus event that might occur after touch
1034+
this.rootItem.dispatchEvent(new Event('focus', { bubbles: true }));
1035+
1036+
// Wait to ensure submenu doesn't reopen
1037+
await aTimeout(100);
1038+
1039+
expect(this.rootItem.open, 'submenu should stay closed').to.be
1040+
.false;
10371041
});
10381042

1039-
it('closes sibling submenus on touch pointerenter', async function () {
1040-
// Create a second menu item with submenu
1043+
it('handles nested touch submenus correctly', async function () {
1044+
// Create nested submenu structure
10411045
const el = await fixture<Menu>(html`
10421046
<sp-menu>
1043-
<sp-menu-item class="root-1">
1044-
First submenu
1045-
<sp-menu slot="submenu">
1046-
<sp-menu-item>Item A</sp-menu-item>
1047-
</sp-menu>
1048-
</sp-menu-item>
1049-
<sp-menu-item class="root-2">
1050-
Second submenu
1047+
<sp-menu-item class="root">
1048+
Parent
10511049
<sp-menu slot="submenu">
1052-
<sp-menu-item>Item B</sp-menu-item>
1050+
<sp-menu-item class="child">
1051+
Child with submenu
1052+
<sp-menu slot="submenu">
1053+
<sp-menu-item>Nested Item 1</sp-menu-item>
1054+
<sp-menu-item>Nested Item 2</sp-menu-item>
1055+
</sp-menu>
1056+
</sp-menu-item>
10531057
</sp-menu>
10541058
</sp-menu-item>
10551059
</sp-menu>
10561060
`);
10571061
await elementUpdated(el);
1058-
const rootItem1 = el.querySelector('.root-1') as MenuItem;
1059-
const rootItem2 = el.querySelector('.root-2') as MenuItem;
1060-
await elementUpdated(rootItem1);
1061-
await elementUpdated(rootItem2);
1062-
1063-
// Open first submenu with mouse
1064-
const opened1 = oneEvent(rootItem1, 'sp-opened');
1065-
rootItem1.dispatchEvent(
1066-
new PointerEvent('pointerenter', {
1062+
const rootItem = el.querySelector('.root') as MenuItem;
1063+
const childItem = el.querySelector('.child') as MenuItem;
1064+
await elementUpdated(rootItem);
1065+
await elementUpdated(childItem);
1066+
1067+
// Open parent submenu
1068+
rootItem.dispatchEvent(
1069+
new PointerEvent('pointerdown', {
1070+
bubbles: true,
1071+
pointerType: 'touch',
1072+
})
1073+
);
1074+
const parentOpened = oneEvent(rootItem, 'sp-opened');
1075+
rootItem.dispatchEvent(
1076+
new PointerEvent('pointerup', {
1077+
bubbles: true,
1078+
pointerType: 'touch',
1079+
})
1080+
);
1081+
await parentOpened;
1082+
expect(rootItem.open).to.be.true;
1083+
1084+
// Open child submenu
1085+
childItem.dispatchEvent(
1086+
new PointerEvent('pointerdown', {
1087+
bubbles: true,
1088+
pointerType: 'touch',
1089+
})
1090+
);
1091+
const childOpened = oneEvent(childItem, 'sp-opened');
1092+
childItem.dispatchEvent(
1093+
new PointerEvent('pointerup', {
10671094
bubbles: true,
1068-
pointerType: 'mouse',
1095+
pointerType: 'touch',
10691096
})
10701097
);
1071-
await opened1;
1098+
await childOpened;
1099+
expect(childItem.open).to.be.true;
1100+
expect(rootItem.open, 'parent should stay open').to.be.true;
10721101

1073-
expect(rootItem1.open).to.be.true;
1074-
expect(rootItem2.open).to.be.false;
1102+
// Close child submenu
1103+
childItem.dispatchEvent(
1104+
new PointerEvent('pointerdown', {
1105+
bubbles: true,
1106+
pointerType: 'touch',
1107+
})
1108+
);
1109+
const childClosed = oneEvent(childItem, 'sp-closed');
1110+
childItem.dispatchEvent(
1111+
new PointerEvent('pointerup', {
1112+
bubbles: true,
1113+
pointerType: 'touch',
1114+
})
1115+
);
1116+
await childClosed;
1117+
expect(childItem.open).to.be.false;
1118+
expect(rootItem.open, 'parent should stay open after child closes')
1119+
.to.be.true;
1120+
});
10751121

1076-
// Hover second item with mouse should close first
1077-
const closed1 = oneEvent(rootItem1, 'sp-closed');
1078-
rootItem2.dispatchEvent(
1079-
new PointerEvent('pointerenter', {
1122+
it('prevents duplicate listeners from multiple pointerdown events', async function () {
1123+
expect(this.rootItem.open).to.be.false;
1124+
1125+
// Dispatch multiple pointerdown events rapidly
1126+
this.rootItem.dispatchEvent(
1127+
new PointerEvent('pointerdown', {
10801128
bubbles: true,
1081-
pointerType: 'mouse',
1129+
pointerType: 'touch',
1130+
})
1131+
);
1132+
this.rootItem.dispatchEvent(
1133+
new PointerEvent('pointerdown', {
1134+
bubbles: true,
1135+
pointerType: 'touch',
1136+
})
1137+
);
1138+
1139+
// Only one pointerup should open it
1140+
const opened = oneEvent(this.rootItem, 'sp-opened');
1141+
this.rootItem.dispatchEvent(
1142+
new PointerEvent('pointerup', {
1143+
bubbles: true,
1144+
pointerType: 'touch',
1145+
})
1146+
);
1147+
await opened;
1148+
1149+
expect(this.rootItem.open).to.be.true;
1150+
1151+
// Now close it
1152+
this.rootItem.dispatchEvent(
1153+
new PointerEvent('pointerdown', {
1154+
bubbles: true,
1155+
pointerType: 'touch',
10821156
})
10831157
);
1084-
await closed1;
1158+
const closed = oneEvent(this.rootItem, 'sp-closed');
1159+
this.rootItem.dispatchEvent(
1160+
new PointerEvent('pointerup', {
1161+
bubbles: true,
1162+
pointerType: 'touch',
1163+
})
1164+
);
1165+
await closed;
10851166

1086-
expect(rootItem1.open).to.be.false;
1167+
// Verify it closed and didn't immediately reopen
1168+
expect(this.rootItem.open).to.be.false;
1169+
await aTimeout(100);
1170+
expect(this.rootItem.open, 'should stay closed').to.be.false;
10871171
});
10881172
});
10891173
});

0 commit comments

Comments
 (0)