Merge pull request #31 from arnauda-gh/fix/swipe-action-desync

fix(ui): synchronize swipe action gesture handlers with latest props

src/components/SwipeableRow.tsx

@@ -9,6 +9,11 @@ import {
 import { typography, type ThemePalette } from '../theme/tokens';
 import { useColors } from '../theme/colors';
 import type { SwipeAction, SwipeMode } from '../stores/settings-store';
+import {
+  shouldClaimGesture, dragOffset, resolveRelease, exitsRow,
+  COMMIT_THRESHOLD, REVEAL_WIDTH,
+  type SwipeConfig,
+} from './swipe-gesture';
 
 interface SwipeableRowProps {
   children: React.ReactNode;
@@ -26,17 +31,6 @@ interface SwipeableRowProps {
   mode?: SwipeMode;
 }
 
-// Instant mode: distance the user must drag past on release to fire the action.
-const COMMIT_THRESHOLD = 96;
-// Reveal mode: drag past this on release to snap to the open (revealed) state.
-const ACTIVATION_THRESHOLD = 32;
-// Reveal mode: width of the action band shown when revealed.
-const REVEAL_WIDTH = 88;
-
-const DIRECTION_BIAS = 1.5;       // dx must dominate dy by this factor
-const MIN_DX_TO_CLAIM = 6;
-const MAX_DRAG_OVERSHOOT_INSTANT = 240;
-const MAX_DRAG_OVERSHOOT_REVEAL = REVEAL_WIDTH * 1.4;
 // Distance the row flies off-screen by before the action callback fires.
 const EXIT_DISTANCE = 600;
 
@@ -58,12 +52,6 @@ function actionLabel(action: SwipeAction, context: { unread: boolean; starred: b
   return ACTION_META[action].defaultLabel;
 }
 
-// Actions that visibly remove the row from the list (so the row should fly off
-// instead of snapping back). Toggle-style actions stay in place.
-function exitsRow(action: SwipeAction): boolean {
-  return action === 'archive' || action === 'delete' || action === 'spam' || action === 'move';
-}
-
 export function SwipeableRow({
   children, leftAction, rightAction, context, onAction, mode = 'instant',
 }: SwipeableRowProps) {
@@ -73,19 +61,37 @@ export function SwipeableRow({
   const claimed = useRef(false);
   const widthRef = useRef(Dimensions.get('window').width);
 
+  // The PanResponder below is built once, so its handlers close over the props
+  // of the render that built it. Reading them through a ref instead keeps the
+  // gesture in step with the row it is actually attached to. Three ways that
+  // went wrong before: changing a swipe action in settings kept firing the old
+  // one; entering selection mode (which passes 'none') still fired actions;
+  // and `onAction` captured a `handleSwipeAction` from before the mailbox ids
+  // had loaded, so on a slow connection swipe did nothing even untouched.
+  // Same idiom as RichTextEditor's callback refs.
+  const latest = useRef<SwipeConfig & { onAction: (a: SwipeAction) => void }>({
+    leftAction, rightAction, mode, onAction,
+  });
+  latest.current = { leftAction, rightAction, mode, onAction };
+
   // Reveal-mode-only state: which side (if any) is currently sitting open.
   const openSideRef = useRef<'left' | 'right' | null>(null);
   const [openSide, setOpenSide] = useState<'left' | 'right' | null>(null);
 
+  const springTo = (toValue: number) => {
+    Animated.spring(dx, { toValue, useNativeDriver: true, speed: 24, bounciness: 4 }).start();
+  };
+  // Spring back to rest without changing which side is open.
+  const settle = () => springTo(0);
+
   const close = () => {
-    Animated.spring(dx, { toValue: 0, useNativeDriver: true, speed: 24, bounciness: 4 }).start();
+    springTo(0);
     openSideRef.current = null;
     setOpenSide(null);
   };
 
   const openTo = (side: 'left' | 'right') => {
-    const target = side === 'right' ? REVEAL_WIDTH : -REVEAL_WIDTH;
-    Animated.spring(dx, { toValue: target, useNativeDriver: true, speed: 24, bounciness: 4 }).start();
+    springTo(side === 'right' ? REVEAL_WIDTH : -REVEAL_WIDTH);
     openSideRef.current = side;
     setOpenSide(side);
   };
@@ -101,14 +107,14 @@ export function SwipeableRow({
         duration: 180,
         useNativeDriver: true,
       }).start(() => {
-        onAction(action);
+        latest.current.onAction(action);
         // The row is about to be removed; reset translation in case it isn't
         // (e.g. the action failed silently) so we don't leave it off-screen.
         dx.setValue(0);
       });
     } else {
-      onAction(action);
-      Animated.spring(dx, { toValue: 0, useNativeDriver: true, speed: 24, bounciness: 4 }).start();
+      latest.current.onAction(action);
+      settle();
     }
   };
 
@@ -117,77 +123,56 @@ export function SwipeableRow({
     if (action === 'none') return;
     if (exitsRow(action)) {
       // Let the row collapse a frame, then fire so the parent's list update
-      // has a clean starting point.
-      requestAnimationFrame(() => onAction(action));
+      // has a clean starting point. Deferred, so read the callback on arrival
+      // rather than capturing today's.
+      requestAnimationFrame(() => latest.current.onAction(action));
     } else {
-      onAction(action);
+      latest.current.onAction(action);
     }
   };
 
-  const responder = useRef(
-    PanResponder.create({
-      onStartShouldSetPanResponder: () => false,
-      onMoveShouldSetPanResponder: (_, g) => {
-        if (Math.abs(g.dx) < MIN_DX_TO_CLAIM) return false;
-        if (Math.abs(g.dx) < Math.abs(g.dy) * DIRECTION_BIAS) return false;
-        // Reveal mode: if a side is already open, allow the gesture so the
-        // user can drag it closed.
-        if (mode === 'reveal' && openSideRef.current === null) {
-          if (g.dx > 0 && rightAction === 'none') return false;
-          if (g.dx < 0 && leftAction === 'none') return false;
-        } else if (mode !== 'reveal') {
-          if (g.dx > 0 && rightAction === 'none') return false;
-          if (g.dx < 0 && leftAction === 'none') return false;
-        }
-        claimed.current = true;
-        return true;
-      },
-      onPanResponderMove: (_, g) => {
-        const overshoot = mode === 'reveal' ? MAX_DRAG_OVERSHOOT_REVEAL : MAX_DRAG_OVERSHOOT_INSTANT;
-        const base =
-          mode === 'reveal'
-            ? openSideRef.current === 'right' ? REVEAL_WIDTH
-              : openSideRef.current === 'left' ? -REVEAL_WIDTH
-              : 0
-            : 0;
-        const next = base + g.dx;
-        const clamped = Math.max(-overshoot, Math.min(overshoot, next));
-        dx.setValue(clamped);
-      },
-      onPanResponderRelease: (_, g) => {
-        claimed.current = false;
-        if (mode === 'reveal') {
-          const base =
-            openSideRef.current === 'right' ? REVEAL_WIDTH
-              : openSideRef.current === 'left' ? -REVEAL_WIDTH
-              : 0;
-          const finalDx = base + g.dx;
-          if (finalDx > ACTIVATION_THRESHOLD && rightAction !== 'none') {
-            openTo('right');
-          } else if (finalDx < -ACTIVATION_THRESHOLD && leftAction !== 'none') {
-            openTo('left');
-          } else {
-            close();
+  // Built once. Every handler reads `latest.current`, so a rebuild would be a
+  // no-op anyway — which is what makes useMemo (free to discard its cache)
+  // safe here.
+  const responder = React.useMemo(
+    () =>
+      PanResponder.create({
+        onStartShouldSetPanResponder: () => false,
+        onMoveShouldSetPanResponder: (_, g) => {
+          if (!shouldClaimGesture(g, latest.current, openSideRef.current)) return false;
+          claimed.current = true;
+          return true;
+        },
+        onPanResponderMove: (_, g) => {
+          dx.setValue(dragOffset(g, latest.current, openSideRef.current));
+        },
+        onPanResponderRelease: (_, g) => {
+          claimed.current = false;
+          const outcome = resolveRelease(g, latest.current, openSideRef.current);
+          switch (outcome.kind) {
+            case 'open':
+              openTo(outcome.side);
+              break;
+            case 'close':
+              close();
+              break;
+            case 'fire':
+              fly(outcome.direction * (widthRef.current || EXIT_DISTANCE), outcome.action);
+              break;
+            case 'settle':
+              settle();
+              break;
           }
-          return;
-        }
-        // Instant mode: fire on release-past-threshold, no second tap.
-        if ((g.dx >= COMMIT_THRESHOLD || (g.dx > 40 && g.vx >= 0.5)) && rightAction !== 'none') {
-          fly(widthRef.current || EXIT_DISTANCE, rightAction);
-        } else if ((g.dx <= -COMMIT_THRESHOLD || (g.dx < -40 && g.vx <= -0.5)) && leftAction !== 'none') {
-          fly(-(widthRef.current || EXIT_DISTANCE), leftAction);
-        } else {
-          Animated.spring(dx, { toValue: 0, useNativeDriver: true, speed: 24, bounciness: 4 }).start();
-        }
-      },
-      onPanResponderTerminate: () => {
-        if (mode === 'reveal') close();
-        else Animated.spring(dx, { toValue: 0, useNativeDriver: true, speed: 24, bounciness: 4 }).start();
-        claimed.current = false;
-      },
-      onPanResponderTerminationRequest: () => !claimed.current,
-    }),
-  ).current;
+        },
+        onPanResponderTerminate: () => {
+          if (latest.current.mode === 'reveal') close();
+          else settle();
+          claimed.current = false;
+        },
+        onPanResponderTerminationRequest: () => !claimed.current,
+      }),
+    [],
+  );
 
   const onLayout = (e: { nativeEvent: { layout: { width: number } } }) => {
     widthRef.current = e.nativeEvent.layout.width;

src/components/__tests__/swipe-gesture.test.ts

@@ -0,0 +1,162 @@
+import { describe, it, expect } from 'vitest';
+import {
+  shouldClaimGesture,
+  dragOffset,
+  resolveRelease,
+  restingOffset,
+  exitsRow,
+  COMMIT_THRESHOLD,
+  ACTIVATION_THRESHOLD,
+  REVEAL_WIDTH,
+  type SwipeConfig,
+} from '../swipe-gesture';
+
+const instant = (over: Partial<SwipeConfig> = {}): SwipeConfig => ({
+  mode: 'instant',
+  leftAction: 'delete',
+  rightAction: 'archive',
+  ...over,
+});
+
+const reveal = (over: Partial<SwipeConfig> = {}): SwipeConfig => instant({ mode: 'reveal', ...over });
+
+// PanResponder hands over dx/dy/vx; the rest of the gesture state is unused.
+const g = (dx: number, opts: { dy?: number; vx?: number } = {}) => ({
+  dx,
+  dy: opts.dy ?? 0,
+  vx: opts.vx ?? 0,
+});
+
+describe('shouldClaimGesture', () => {
+  it('ignores drags too short to be intentional', () => {
+    expect(shouldClaimGesture(g(4), instant(), null)).toBe(false);
+    expect(shouldClaimGesture(g(-4), instant(), null)).toBe(false);
+  });
+
+  it('leaves mostly-vertical drags to the list scroll', () => {
+    expect(shouldClaimGesture(g(20, { dy: 40 }), instant(), null)).toBe(false);
+    expect(shouldClaimGesture(g(40, { dy: 20 }), instant(), null)).toBe(true);
+  });
+
+  it("declines the direction whose action is 'none'", () => {
+    const config = instant({ rightAction: 'none' });
+    expect(shouldClaimGesture(g(50), config, null)).toBe(false);
+    expect(shouldClaimGesture(g(-50), config, null)).toBe(true);
+  });
+
+  it("declines both directions in selection mode, where both actions are 'none'", () => {
+    const config = instant({ leftAction: 'none', rightAction: 'none' });
+    expect(shouldClaimGesture(g(50), config, null)).toBe(false);
+    expect(shouldClaimGesture(g(-50), config, null)).toBe(false);
+  });
+
+  it('claims any direction while a reveal band is open, so it can be dragged shut', () => {
+    const config = reveal({ leftAction: 'none', rightAction: 'archive' });
+    expect(shouldClaimGesture(g(-50), config, 'right')).toBe(true);
+    // ...but not once it is closed again.
+    expect(shouldClaimGesture(g(-50), config, null)).toBe(false);
+  });
+});
+
+describe('dragOffset', () => {
+  it('tracks the finger in instant mode', () => {
+    expect(dragOffset(g(70), instant(), null)).toBe(70);
+    expect(dragOffset(g(-70), instant(), null)).toBe(-70);
+  });
+
+  it('clamps to the overshoot limit', () => {
+    expect(dragOffset(g(9999), instant(), null)).toBe(240);
+    expect(dragOffset(g(-9999), instant(), null)).toBe(-240);
+    // Reveal mode allows much less overshoot than instant.
+    expect(dragOffset(g(9999), reveal(), null)).toBeCloseTo(REVEAL_WIDTH * 1.4);
+  });
+
+  it('offsets from the open position in reveal mode', () => {
+    // Already open to the right; dragging back left should approach 0.
+    expect(dragOffset(g(-REVEAL_WIDTH), reveal(), 'right')).toBe(0);
+    expect(dragOffset(g(-10), reveal(), 'right')).toBe(REVEAL_WIDTH - 10);
+  });
+});
+
+describe('resolveRelease — instant mode', () => {
+  it('fires the action for the direction dragged past the threshold', () => {
+    expect(resolveRelease(g(COMMIT_THRESHOLD), instant(), null)).toEqual({
+      kind: 'fire',
+      action: 'archive',
+      direction: 1,
+    });
+    expect(resolveRelease(g(-COMMIT_THRESHOLD), instant(), null)).toEqual({
+      kind: 'fire',
+      action: 'delete',
+      direction: -1,
+    });
+  });
+
+  it('accepts a short fast flick as a commit', () => {
+    expect(resolveRelease(g(50, { vx: 0.8 }), instant(), null)).toMatchObject({ kind: 'fire' });
+    // Same distance, too slow.
+    expect(resolveRelease(g(50, { vx: 0.2 }), instant(), null)).toEqual({ kind: 'settle' });
+  });
+
+  it('springs back when the drag stops short', () => {
+    expect(resolveRelease(g(COMMIT_THRESHOLD - 1), instant(), null)).toEqual({ kind: 'settle' });
+  });
+
+  it("never fires an action configured as 'none'", () => {
+    const config = instant({ rightAction: 'none' });
+    expect(resolveRelease(g(999, { vx: 5 }), config, null)).toEqual({ kind: 'settle' });
+  });
+
+  it('fires nothing in selection mode, however hard the row is flung', () => {
+    const config = instant({ leftAction: 'none', rightAction: 'none' });
+    expect(resolveRelease(g(999, { vx: 5 }), config, null)).toEqual({ kind: 'settle' });
+    expect(resolveRelease(g(-999, { vx: -5 }), config, null)).toEqual({ kind: 'settle' });
+  });
+});
+
+describe('resolveRelease — reveal mode', () => {
+  it('opens the band rather than firing', () => {
+    expect(resolveRelease(g(ACTIVATION_THRESHOLD + 1), reveal(), null)).toEqual({
+      kind: 'open',
+      side: 'right',
+    });
+    expect(resolveRelease(g(-ACTIVATION_THRESHOLD - 1), reveal(), null)).toEqual({
+      kind: 'open',
+      side: 'left',
+    });
+  });
+
+  it('closes when the drag stops short', () => {
+    expect(resolveRelease(g(ACTIVATION_THRESHOLD - 1), reveal(), null)).toEqual({ kind: 'close' });
+  });
+
+  it('measures from the open position, so a small drag keeps it open', () => {
+    // Open to the right, nudged 5px further right: still well past the threshold.
+    expect(resolveRelease(g(5), reveal(), 'right')).toEqual({ kind: 'open', side: 'right' });
+    // Dragged back past the resting point: closes.
+    expect(resolveRelease(g(-REVEAL_WIDTH), reveal(), 'right')).toEqual({ kind: 'close' });
+  });
+
+  it("closes instead of opening a side whose action is 'none'", () => {
+    const config = reveal({ rightAction: 'none' });
+    expect(resolveRelease(g(999), config, null)).toEqual({ kind: 'close' });
+  });
+});
+
+describe('restingOffset', () => {
+  it('maps the open side to the row translation it sits at', () => {
+    expect(restingOffset('right')).toBe(REVEAL_WIDTH);
+    expect(restingOffset('left')).toBe(-REVEAL_WIDTH);
+    expect(restingOffset(null)).toBe(0);
+  });
+});
+
+describe('exitsRow', () => {
+  it('flags the actions that remove the row from the list', () => {
+    expect((['archive', 'delete', 'spam', 'move'] as const).every(exitsRow)).toBe(true);
+  });
+
+  it('keeps toggle actions in place', () => {
+    expect((['read', 'star', 'pin', 'none'] as const).some(exitsRow)).toBe(false);
+  });
+});

src/components/swipe-gesture.ts

@@ -0,0 +1,132 @@
+// Pure decision logic behind SwipeableRow's pan gesture.
+//
+// This lives outside the component for a reason beyond tidiness: the row's
+// PanResponder is created once and its handlers therefore close over whatever
+// props existed at mount. Taking the config as an argument makes staleness
+// structurally impossible here — there is nothing to capture — and leaves the
+// component with one job, keeping the current config somewhere the handlers
+// can reach.
+
+import type { SwipeAction, SwipeMode } from '../stores/settings-store';
+
+// Instant mode: distance the user must drag past on release to fire the action.
+export const COMMIT_THRESHOLD = 96;
+// Reveal mode: drag past this on release to snap to the open (revealed) state.
+export const ACTIVATION_THRESHOLD = 32;
+// Reveal mode: width of the action band shown when revealed.
+export const REVEAL_WIDTH = 88;
+
+const DIRECTION_BIAS = 1.5; // dx must dominate dy by this factor
+const MIN_DX_TO_CLAIM = 6;
+const MAX_DRAG_OVERSHOOT_INSTANT = 240;
+const MAX_DRAG_OVERSHOOT_REVEAL = REVEAL_WIDTH * 1.4;
+// A short, fast drag commits too, so the gesture doesn't demand the full
+// COMMIT_THRESHOLD when the user clearly flicks.
+const FLING_DX = 40;
+const FLING_VX = 0.5;
+
+/**
+ * The row's current swipe configuration.
+ *
+ * Note the naming: `rightAction` fires on a rightward (positive dx) drag and
+ * its band sits at the *left* edge; `leftAction` is the mirror image.
+ */
+export interface SwipeConfig {
+  leftAction: SwipeAction;
+  rightAction: SwipeAction;
+  mode: SwipeMode;
+}
+
+/** The subset of RN's PanResponder gesture state this logic needs. */
+export interface SwipeGestureState {
+  dx: number;
+  dy: number;
+  vx: number;
+}
+
+/** Which side's action band is currently held open (reveal mode only). */
+export type OpenSide = 'left' | 'right' | null;
+
+export type ReleaseOutcome =
+  /** Reveal mode: snap open so the band can be tapped. */
+  | { kind: 'open'; side: 'left' | 'right' }
+  /** Reveal mode: snap shut. */
+  | { kind: 'close' }
+  /** Instant mode: run the action; `direction` is the sign to fly the row off by. */
+  | { kind: 'fire'; action: SwipeAction; direction: 1 | -1 }
+  /** Not far enough — spring back to rest without firing. */
+  | { kind: 'settle' };
+
+/**
+ * Whether the pan should be claimed from whatever else might want it (the
+ * list's vertical scroll, mainly). Declining leaves the gesture to the parent.
+ */
+export function shouldClaimGesture(
+  g: SwipeGestureState,
+  config: SwipeConfig,
+  openSide: OpenSide,
+): boolean {
+  if (Math.abs(g.dx) < MIN_DX_TO_CLAIM) return false;
+  if (Math.abs(g.dx) < Math.abs(g.dy) * DIRECTION_BIAS) return false;
+  // An already-open row claims any horizontal drag so the user can drag it
+  // shut, even in the direction whose action is 'none'.
+  if (config.mode === 'reveal' && openSide !== null) return true;
+  if (g.dx > 0 && config.rightAction === 'none') return false;
+  if (g.dx < 0 && config.leftAction === 'none') return false;
+  return true;
+}
+
+/** Row translation for an in-progress drag, clamped to the mode's overshoot. */
+export function dragOffset(
+  g: SwipeGestureState,
+  config: SwipeConfig,
+  openSide: OpenSide,
+): number {
+  const reveal = config.mode === 'reveal';
+  const overshoot = reveal ? MAX_DRAG_OVERSHOOT_REVEAL : MAX_DRAG_OVERSHOOT_INSTANT;
+  const base = reveal ? restingOffset(openSide) : 0;
+  return Math.max(-overshoot, Math.min(overshoot, base + g.dx));
+}
+
+/** Where the row sits at rest, given which side is open. */
+export function restingOffset(openSide: OpenSide): number {
+  if (openSide === 'right') return REVEAL_WIDTH;
+  if (openSide === 'left') return -REVEAL_WIDTH;
+  return 0;
+}
+
+/** What releasing the drag here should do. */
+export function resolveRelease(
+  g: SwipeGestureState,
+  config: SwipeConfig,
+  openSide: OpenSide,
+): ReleaseOutcome {
+  if (config.mode === 'reveal') {
+    const finalDx = restingOffset(openSide) + g.dx;
+    if (finalDx > ACTIVATION_THRESHOLD && config.rightAction !== 'none') {
+      return { kind: 'open', side: 'right' };
+    }
+    if (finalDx < -ACTIVATION_THRESHOLD && config.leftAction !== 'none') {
+      return { kind: 'open', side: 'left' };
+    }
+    return { kind: 'close' };
+  }
+  // Instant mode: fire on release-past-threshold, no second tap.
+  const committedRight = g.dx >= COMMIT_THRESHOLD || (g.dx > FLING_DX && g.vx >= FLING_VX);
+  if (committedRight && config.rightAction !== 'none') {
+    return { kind: 'fire', action: config.rightAction, direction: 1 };
+  }
+  const committedLeft = g.dx <= -COMMIT_THRESHOLD || (g.dx < -FLING_DX && g.vx <= -FLING_VX);
+  if (committedLeft && config.leftAction !== 'none') {
+    return { kind: 'fire', action: config.leftAction, direction: -1 };
+  }
+  return { kind: 'settle' };
+}
+
+/**
+ * Actions that visibly remove the row from the list, so the row should fly off
+ * instead of snapping back. Toggle-style actions stay in place.
+ */
+export function exitsRow(action: SwipeAction): boolean {
+  return action === 'archive' || action === 'delete' || action === 'spam' || action === 'move';
+}