diff --git a/packages/menu/src/Menu.ts b/packages/menu/src/Menu.ts index 28cbfb6f74..62910d9046 100644 --- a/packages/menu/src/Menu.ts +++ b/packages/menu/src/Menu.ts @@ -103,30 +103,30 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { /** * Minimum vertical movement (in pixels) required to trigger scrolling detection. - * + * * This threshold is consistent with other components in the design system: * - Card component uses 10px for click vs. drag detection * - Menu component uses 10px for scroll vs. selection detection - * + * * The 10px threshold is carefully chosen to: * - Allow for natural finger tremor and accidental touches * - Distinguish between intentional scroll gestures and taps * - Provide consistent behavior across the platform - * + * * @see {@link packages/card/src/Card.ts} for similar threshold usage */ private scrollThreshold = 10; // pixels /** * Maximum time (in milliseconds) for a movement to be considered scrolling. - * + * * This threshold is consistent with other timing values in the design system: * - Longpress duration: 300ms (ActionButton, LongpressController) * - Scroll detection: 300ms (Menu component) - * + * * Quick movements within this timeframe are likely intentional scrolls, * while slower movements are more likely taps or selections. - * + * * @see {@link packages/action-button/src/ActionButton.ts} for longpress duration * @see {@link packages/overlay/src/LongpressController.ts} for longpress duration */ @@ -179,7 +179,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { public valueSeparator = ','; /** - * selected items values as string + * Updating comment for test */ @property({ attribute: false }) public get selected(): string[] { @@ -576,7 +576,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { * Resets the scrolling state after a short delay (100ms) to allow for * any final touch events to be processed. This delay prevents immediate * state changes that could interfere with the selection logic. - * + * * The 100ms delay is consistent with the design system's approach to * touch event handling and ensures that any final touch events or * gesture recognition can complete before the scrolling state is reset. diff --git a/packages/menu/src/MenuItem.ts b/packages/menu/src/MenuItem.ts index 75de1e0de3..4450c23ef3 100644 --- a/packages/menu/src/MenuItem.ts +++ b/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; 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(