Skip to content
Draft

test PR #5832

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions packages/menu/src/Menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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[] {
Expand Down Expand Up @@ -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.
Expand Down
98 changes: 46 additions & 52 deletions packages/menu/src/MenuItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()}`;
}
Expand Down Expand Up @@ -575,6 +567,7 @@ export class MenuItem extends LikeAnchor(

return false;
}

/**
* forward key info from keydown event to parent menu
*/
Expand All @@ -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) {
Expand All @@ -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<typeof setTimeout>;
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(() => {
Expand Down Expand Up @@ -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(
Expand Down
Loading