fix(menu press): cancel only the lifted finger's touchend

.changeset/menu-press-touch-identifier.md

@@ -0,0 +1,7 @@
+---
+'@astryxdesign/core': patch
+---
+
+[fix] DropdownMenu, ContextMenu, Selector, TabList, TopNav, SideNav, Breadcrumbs: when a finger picks a menu row, the click guard cancels only that finger's `touchend` once the menu has named the touch, so another finger's `touchend` passes. The menu names the touch from the `touchstart` that follows its `pointerdown`. A `touchend` without `changedTouches`, or a lift whose touch the menu could not name, keeps the old rule: the next `touchend` is cancelled.
+
+@cixzhang @vjeux

packages/core/src/hooks/useMenuPress.test.tsx

@@ -130,6 +130,20 @@ function fireTouchEnd(target: Element): Event {
   return event;
 }
 
+/** A cancelable touch event at `target` whose one changed touch has `identifier`. */
+function fireTouch(
+  target: Element,
+  type: 'touchstart' | 'touchend',
+  identifier: number,
+): Event {
+  const event = new Event(type, {bubbles: true, cancelable: true});
+  Object.defineProperty(event, 'changedTouches', {
+    value: [{identifier, target}],
+  });
+  target.dispatchEvent(event);
+  return event;
+}
+
 beforeEach(() => {
   __resetMenuPressForTest();
 });
@@ -649,6 +663,26 @@ describe("useMenuPress — a finger's touchend (spec:AST-094 FR21)", () => {
     fireEvent.pointerDown(screen.getByTestId('outside'), touch(3));
     expect(fireTouchEnd(row('B')).defaultPrevented).toBe(false);
   });
+
+  it("leaves another finger's touchend alone and cancels the lifted touch's own", () => {
+    const onSelect = vi.fn();
+    render(<Harness onSelect={onSelect} />);
+    fireEvent.pointerDown(row('A'), touch(7));
+    fireTouch(row('A'), 'touchstart', 7);
+    fireEvent.pointerUp(row('A'), touch(7));
+    expect(fireTouch(row('A'), 'touchend', 99).defaultPrevented).toBe(false);
+    expect(fireTouch(row('A'), 'touchend', 7).defaultPrevented).toBe(true);
+    expect(onSelect.mock.calls).toEqual([['A']]);
+  });
+
+  it('names the touch by its place in the touchstart when its identifier is not the pointer id', () => {
+    render(<Harness />);
+    fireEvent.pointerDown(row('A'), touch(2));
+    fireTouch(row('A'), 'touchstart', 0);
+    fireEvent.pointerUp(row('A'), touch(2));
+    expect(fireTouch(row('A'), 'touchend', 1).defaultPrevented).toBe(false);
+    expect(fireTouch(row('A'), 'touchend', 0).defaultPrevented).toBe(true);
+  });
 });
 
 describe('useMenuPress — release outside and cancellation', () => {

packages/core/src/hooks/useMenuPress.ts

@@ -44,6 +44,7 @@ import {
   type LongPressTolerance,
 } from './useLongPress';
 import {edgeAutoscroll} from '../utils/edgeAutoscroll';
+import {subscribeInput} from '../utils/inputStream';
 import {focusForLastInput} from '../utils/focusReturn';
 import {useInteractionModalityTrackingFor} from '../utils/interactionModality';
 import {LONG_PRESS_DURATION_MS} from '../motion/uikit/longPress';
@@ -191,6 +192,26 @@ function normalizePointerType(type: string): MenuPressPointerType | null {
   return type === 'mouse' || type === 'touch' || type === 'pen' ? type : null;
 }
 
+/**
+ * The identifier of the touch that a pointer's touchstart started. WebKit
+ * gives the touch the pointer's id. Chromium gives it its own id, so a lone
+ * changed touch is the pointer's. Several changed touches, none with the
+ * pointer's id, name no touch.
+ */
+function touchOfPointer(
+  changed: TouchList | undefined,
+  pointerId: number,
+): number | null {
+  if (changed == null) {
+    return null;
+  }
+  const touches = Array.from(changed);
+  if (touches.some(touch => touch.identifier === pointerId)) {
+    return pointerId;
+  }
+  return touches.length === 1 ? touches[0].identifier : null;
+}
+
 function isRowFocusable(row: HTMLElement): boolean {
   return row.matches(FOCUSABLE_ROW_SELECTOR);
 }
@@ -342,6 +363,7 @@ export function useDropdownMenuPress(
 
   const gestureRef = useRef<MenuPressState<HTMLElement>>(IDLE_MENU_PRESS);
   const pointerIdRef = useRef<number | null>(null);
+  const touchIdentifierRef = useRef<number | null>(null);
   const detachRef = useRef<(() => void) | null>(null);
   // `useLongPress` detects the held finger, as in ContextMenu, so the hold
   // time and the movement tolerance have one home. A ref holds the handle,
@@ -598,6 +620,7 @@ export function useDropdownMenuPress(
           armClickGuard(doc, {
             staysInstalled: true,
             cancelsTouchEnd: native?.pointerType === 'touch',
+            touchIdentifier: touchIdentifierRef.current ?? undefined,
           });
           if (native != null) {
             activate(effect.row, native);
@@ -610,6 +633,7 @@ export function useDropdownMenuPress(
             armClickGuard(doc, {
               staysInstalled: true,
               cancelsTouchEnd: native?.pointerType === 'touch',
+              touchIdentifier: touchIdentifierRef.current ?? undefined,
             });
           }
           if (effect.dismiss) {
@@ -705,7 +729,21 @@ export function useDropdownMenuPress(
       const doc = getDocument();
       const view = doc.defaultView;
       pointerIdRef.current = pointerId;
+      touchIdentifierRef.current = null;
 
+      // The touchstart after this pointerdown names the touch, so the guard
+      // at the lift cancels only that touch's touchend.
+      let stopTouchStart: (() => void) | null = null;
+      if (view != null) {
+        stopTouchStart = subscribeInput(view, 'touchstart', event => {
+          stopTouchStart?.();
+          stopTouchStart = null;
+          touchIdentifierRef.current = touchOfPointer(
+            event.changedTouches,
+            pointerId,
+          );
+        });
+      }
       const onMove = (event: PointerEvent) => {
         if (event.pointerId !== pointerIdRef.current) {
           return;
@@ -784,6 +822,7 @@ export function useDropdownMenuPress(
       doc.addEventListener('pointerdown', onOtherPointerDown, true);
       view?.addEventListener('blur', onBlur);
       detachRef.current = () => {
+        stopTouchStart?.();
         doc.removeEventListener('pointermove', onMove, true);
         doc.removeEventListener('pointerup', onUp, true);
         doc.removeEventListener('pointercancel', onCancel, true);

packages/core/src/utils/clickGuard.test.ts

@@ -77,6 +77,16 @@ function touchEnd(target: EventTarget, cancelable = true): Event {
   return event;
 }
 
+/** A cancelable `touchend` whose one changed touch has `identifier`. */
+function touchEndOf(target: EventTarget, identifier: number): Event {
+  const event = new Event('touchend', {bubbles: true, cancelable: true});
+  Object.defineProperty(event, 'changedTouches', {
+    value: [{identifier, target}],
+  });
+  target.dispatchEvent(event);
+  return event;
+}
+
 function button(): HTMLButtonElement {
   const element = document.createElement('button');
   document.body.append(element);
@@ -287,6 +297,22 @@ describe('clickGuard', () => {
     expect(click(target).defaultPrevented).toBe(false);
   });
 
+  it("armed with the lifted touch's identifier, leaves another touch's touchend alone and stays armed for its own", () => {
+    const target = button();
+    armClickGuard(document, {cancelsTouchEnd: true, touchIdentifier: 7});
+    expect(touchEndOf(target, 99).defaultPrevented).toBe(false);
+    expect(isClickGuardArmed(document)).toBe(true);
+    expect(touchEndOf(target, 7).defaultPrevented).toBe(true);
+    expect(isClickGuardArmed(document)).toBe(false);
+  });
+
+  it('armed with an identifier, takes a touchend without changedTouches as that touch’s own', () => {
+    const target = button();
+    armClickGuard(document, {cancelsTouchEnd: true, touchIdentifier: 7});
+    expect(touchEnd(target).defaultPrevented).toBe(true);
+    expect(isClickGuardArmed(document)).toBe(false);
+  });
+
   it('leaves an uncancelable touchend alone, and stays armed for a click that may follow it', () => {
     const target = button();
     armClickGuard(document, {cancelsTouchEnd: true});

packages/core/src/utils/clickGuard.ts

@@ -31,7 +31,9 @@
 // The menu press model's arm at a finger's release also cancels that touch's
 // `touchend`, so the browser creates no click and no compatibility mouse
 // events. A busy main thread can otherwise hold that click past the window,
-// when a drill-in has put another row under the finger.
+// when a drill-in has put another row under the finger. When the menu names
+// the touch by its identifier, another finger's `touchend` passes; a touch it
+// could not name keeps the old rule, and the next `touchend` is cancelled.
 //
 // Some clicks always pass:
 // - a click with `detail` 0: a keyboard's, a screen reader's, or one that the
@@ -71,6 +73,13 @@ export interface ClickGuardOptions {
    * @default false
    */
   readonly cancelsTouchEnd?: boolean;
+  /**
+   * With `cancelsTouchEnd`: the `Touch.identifier` of the touch that lifted.
+   * The guard cancels only a `touchend` that ends this touch. Another finger's
+   * `touchend` passes, and the guard stays armed for this touch. Without it,
+   * the next `touchend` counts as this touch's own.
+   */
+  readonly touchIdentifier?: number;
 }
 
 interface Guard {
@@ -115,6 +124,7 @@ export function armClickGuard(
     staysInstalled = false,
     takesLateMouse = false,
     cancelsTouchEnd = false,
+    touchIdentifier,
   }: ClickGuardOptions = {},
 ): void {
   const view = doc.defaultView;
@@ -143,12 +153,17 @@ export function armClickGuard(
     }
   }
   if (cancelsTouchEnd) {
-    // A touch sends its pointerup before its touchend, so the next touchend is
-    // that touch's own. The next pointerdown disarms the guard first.
+    // A touch sends its pointerup before its touchend, so without an
+    // identifier the next touchend counts as that touch's own. The next
+    // pointerdown disarms the guard first.
     const stopTouchEnd = subscribeInput(
       view,
       'touchend',
       event => {
+        if (touchIdentifier != null && !endsTouch(event, touchIdentifier)) {
+          // Another finger lifted: leave its touchend.
+          return;
+        }
         stopTouchEnd();
         // A touchend that the browser took for a scroll is not cancelable.
         // The guard stays armed for a click that may follow it.
@@ -165,6 +180,18 @@ export function armClickGuard(
   guard.timer = setTimeout(() => disarmClickGuard(doc), CLICK_GUARD_WINDOW_MS);
 }
 
+/**
+ * Whether `event` ends the touch `identifier`. A `touchend` without
+ * `changedTouches` counts as that touch's own.
+ */
+function endsTouch(event: Event, identifier: number): boolean {
+  const changed = (event as Partial<TouchEvent>).changedTouches;
+  if (changed == null) {
+    return true;
+  }
+  return Array.from(changed).some(touch => touch.identifier === identifier);
+}
+
 /** Stop swallowing on `doc`; idempotent. */
 export function disarmClickGuard(doc: Document): void {
   const guard = guards.get(doc);