Skip to content

Set minimum pinch distance to 44#159

Open
elliottkember wants to merge 1 commit intomasterfrom
elliott/minimum-touch-distance-44
Open

Set minimum pinch distance to 44#159
elliottkember wants to merge 1 commit intomasterfrom
elliott/minimum-touch-distance-44

Conversation

@elliottkember
Copy link
Copy Markdown
Collaborator

@elliottkember elliottkember commented Jan 5, 2026

Offer sufficiently sized controls. Controls that are too small are hard for many people to interact with and select. Strive to meet the recommended minimum control size for each platform to ensure controls and menus are comfortable for all when tapping and clicking.

Below a certain number of pixels, the math for pinching stops being accurate and the scale starts to do strange things. This PR introduces a minimum distance of 44pt between the fingers for a pinch gesture.


Note

  • Pinch-to-zoom stability: Enforces a minimum two-finger distance (~44pt) before applying zoom math to avoid erratic scaling at very small distances.
  • Codebase modernization (CJS output): Switches to react/jsx-runtime, adopts class fields and optional chaining, and tidies event-callback wiring without changing public APIs.
  • Components/animations cleanup: Updates AnimatedTouchFeedback, StaticPin, and animation helpers to the new runtime and minor refactors; no functional UI changes aside from the pinch threshold.

Written by Cursor Bugbot for commit 0272ef6. Configure here.

@elliottkember elliottkember mentioned this pull request Jan 5, 2026
@thomasttvo
Copy link
Copy Markdown
Collaborator

@claude please review this PR

@thomasttvo
Copy link
Copy Markdown
Collaborator

@claude review

Comment on lines 4 to 14

export { calcNewScaledOffsetForZoomCentering } from './calcNewScaledOffsetForZoomCentering';

// Minimum distance between two touch points to be considered a valid pinch gesture
// Uses Apple's Human Interface Guidelines recommended minimum touch target size
// https://developer.apple.com/design/human-interface-guidelines/accessibility#Mobility
const MIN_PINCH_DISTANCE = 44;

/**
* Calculates the gesture center point relative to the page coordinate system
*
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The Math.max(MIN_PINCH_DISTANCE, actualDistance) clamping introduces a zoom-out artifact: on the frame when fingers first cross below 44px, the ratio clampedDistance / previousUnclampedDistance produces an incorrect zoom jump, then all subsequent frames freeze at ratio 1.0 (since lastGestureTouchDistance is set to 44 and every subsequent call also returns 44). The correct fix is to return null when actualDistance < MIN_PINCH_DISTANCE so the existing if (!distance) return guard in _handlePinching cleanly freezes zoom without any artifact.

Extended reasoning...

What the bug is and how it manifests

This PR introduces Math.max(MIN_PINCH_DISTANCE, actualDistance) clamping inside calcGestureTouchDistance. While the intent is to ignore tiny finger separations (below 44px), the implementation causes a visible zoom-out artifact followed by a complete zoom freeze whenever fingers travel from above 44px to below 44px during a pinch.

The specific code path that triggers it

In _handlePinching, the zoom growth ratio is computed as:

zoomGrowthFromLastGestureState = distance / this.lastGestureTouchDistance

where distance comes from calcGestureTouchDistance and lastGestureTouchDistance is the previous frame's distance. Consider this sequence:

  • Frame N: fingers are 80px apart → calcGestureTouchDistance returns 80, lastGestureTouchDistance = 80
  • Frame N+1: fingers move to 30px apart → calcGestureTouchDistance returns Math.max(44, 30) = 44
    • ratio = 44 / 80 = 0.55 — a large unexpected zoom-out jump
    • lastGestureTouchDistance is then set to 44
  • Frame N+2: fingers stay at 30px → returns 44 again
    • ratio = 44 / 44 = 1.0 — zoom frozen
  • All further frames below 44px: ratio = 1.0, zoom frozen

Why existing code does not prevent it

The existing if (!distance) return guard in _handlePinching correctly handles a null return by freezing zoom cleanly. However, Math.max(...) never returns null — it always returns a positive number (minimum 44). This bypasses the null-guard entirely, forcing the incorrect ratio calculation on every frame.

Impact

Users performing a slow pinch-in gesture will see a sudden zoom-out jump at the threshold crossing (frame N+1), followed by the zoom being completely locked until fingers separate past 44px again. This is the opposite of the intended behavior — instead of gracefully ignoring small separations, the PR actively worsens the UX at the boundary.

How to fix it

Return null instead of clamping:

const actualDistance = Math.sqrt(dx * dx + dy * dy);
if (actualDistance < MIN_PINCH_DISTANCE) return null;
return actualDistance;

This causes _handlePinching to hit the if (!distance) return guard, preserving the current zoom level exactly — no jump, no artifact, clean freeze until fingers re-separate beyond the threshold.

Step-by-step proof

  1. User begins pinch with fingers 100px apart. calcGestureTouchDistance returns 100. lastGestureTouchDistance = 100. Zoom ratio = 1.0 (steady).
  2. User moves fingers to 60px apart. Returns 60. Ratio = 60/100 = 0.6 (zoom out). Expected and correct.
  3. User moves fingers to 30px apart (below MIN_PINCH_DISTANCE=44). With the PR's clamping: returns 44. Ratio = 44/60 ≈ 0.733. This applies an unintended zoom-out corresponding to fingers moving from 60px to 44px — but the user's fingers are actually at 30px. This is wrong.
  4. Next frame, fingers still at 30px. Returns 44. lastGestureTouchDistance is now 44. Ratio = 44/44 = 1.0. Zoom freezes.
  5. The user sees: unexpected zoom-out artifact at step 3, then complete zoom lock at step 4 onward.

With the null fix at step 3: returns null, _handlePinching exits early, zoom stays at the value from step 2. Steps 4+ behave the same (zoom frozen). No artifact at step 3.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Returning null when actualDistance < MIN_PINCH_DISTANCE so the existing if (!distance) return guard in _handlePinching cleanly freezes zoom without any artifact.

Comment on lines +359 to +366
/**
* Calculates pinch distance
*
* @param e
* @param gestureState
* @private
*/
_handlePanResponderGrant = (e, gestureState) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The JSDoc comment on _handlePanResponderGrant incorrectly reads "Calculates pinch distance" — this method actually handles the pan responder grant event (sets up long-press timeout, stops animations, sets gestureStarted = true, and calls raisePin()). The misleading comment is a pre-existing issue in src/ReactNativeZoomableView.tsx line 473 that was carried into the compiled outputs regenerated by this PR.

Extended reasoning...

What the bug is: The JSDoc block immediately above _handlePanResponderGrant in the compiled outputs (lib/module/ReactNativeZoomableView.js and lib/commonjs/ReactNativeZoomableView.js) reads * Calculates pinch distance, which is factually incorrect. The method has nothing to do with calculating pinch distance.

What the method actually does: Reading the method body makes the true purpose clear — it (1) sets up a setTimeout long-press callback if onLongPress prop is provided, (2) calls this.props.onPanResponderGrant?.(...)" to notify the consumer that a touch grant started, (3) stops the pan and zoom animations via panAnim.stopAnimation()/zoomAnim.stopAnimation(), (4) sets this.gestureStarted = true, and (5) calls this.raisePin()`. This is the standard gesture-grant event handler — not a distance-calculation routine.

Where pinch distance is actually calculated: Pinch distance calculation is performed by calcGestureTouchDistance in src/helper/index.ts. The new MIN_PINCH_DISTANCE = 44 constant introduced by this PR is also in that helper file. The comment on _handlePanResponderGrant appears to be a copy-paste error from development history that was never corrected.

Why this PR carries it forward: The source of truth is src/ReactNativeZoomableView.tsx at line 473, which already contains the wrong JSDoc. When the compiled outputs in lib/module/ and lib/commonjs/ were regenerated for this PR, they faithfully reproduced the incorrect comment. Since the PR diff modifies these compiled files, it is an appropriate place to flag the pre-existing documentation error.

Impact: There is no functional impact — JSDoc comments do not affect runtime behavior. However, the misleading comment can confuse contributors trying to understand the gesture-handling flow; someone reading the comment might expect distance math inside _handlePanResponderGrant and look in the wrong place.

Concrete proof: Starting from the method signature in lib/module/ReactNativeZoomableView.js: the JSDoc says Calculates pinch distance, but examining the entire method body shows no arithmetic or distance-related logic — variables like dx, dy, or calls to calcGestureTouchDistance are absent. All method logic is grant-event bookkeeping. The fix is simply updating the comment in src/ReactNativeZoomableView.tsx line 473 to something accurate (e.g., Handles the pan responder grant event) and regenerating the compiled outputs.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Updated the JSDoc in src/ReactNativeZoomableView.tsx to "Handles the pan responder grant event" and regenerated compiled outputs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants