Fix pre-existing bugs: stale closures, double-fire, zoom edge cases

[thomasttvo]

Summary

Stacks correctness and safety fixes on top of PR #149's feature strip. Targets bugs discovered during review of PR #149 that also exist in the current library codebase: stale pin-press closures, a listener leak and post-unmount onZoomAfter fire in zoomTo(), broken double-tap zoom cycle-back in two configurations, incorrect zoomCenter semantics (both in doubleTapZoomToCenter behavior and JSDoc), double-fire of onStaticPinPositionChange at pan-end, and an accidental onTransform double-fire on every pan frame. Supersedes the prematurely-merged PR #164 and incorporates five rounds of Claude Code Review feedback.

Test Plan

• Mount a ReactNativeZoomableView with a StaticPin and a pan-responder gesture; double-tap at increasing levels and confirm zoom cycles back to initialZoom when it hits maxZoom.
• Repeat with maxZoom={null} (unlimited) — confirm zoom still cycles back after ~3 steps rather than growing indefinitely.
• Repeat with zoomStep={null} + a configured maxZoom — confirm cycle-back still works on double-tap at max.
• Set doubleTapZoomToCenter={true} and double-tap on an off-center area; confirm the zoom centers on the viewport center (originalWidth/2, originalHeight/2), not the top-left corner.
• Pass an onZoomAfter callback and call zoomTo(); confirm the callback fires exactly once, only when the animation finishes naturally, and reads the final post-animation zoomLevel.
• Unmount the component mid-animation (with an external zoomAnimatedValue supplied) and confirm onZoomAfter does not fire after unmount.
• Call zoomTo() twice in rapid succession and confirm the first animation's completion callback does not remove the second animation's pan-sync listener.
• Wire up onStaticPinPositionChange and perform a single-tap pan; confirm the callback fires once with the final position (not twice ~100 ms apart).
• Swap onPress/onLongPress handlers on a StaticPin after mount (e.g., via state change); tap and confirm the new handler runs, not the stale one from first render.
• Regression: confirm zoomTo() still rejects values outside minZoom/maxZoom, and that minZoom={0} is now respected (previously silently skipped).

Risk

• zoomTo() gesture/animation lifecycle: capturedListenerId rename plus the added this.mounted guard and finished check in .start() change both listener cleanup and onZoomAfter firing semantics — consumers relying on the previous (synchronous / always-fires) behavior may see a behavior change.
• Double-tap zoom state machine in _getNextZoomStep: the guard reorder and new effectiveMax computation change the cycle-back trigger for several zoomStep / maxZoom combinations. Most likely regression surface.
• onStaticPinPositionChange debounce flush replaces a direct call in two places (_handlePanResponderEnd and single-tap pan animation end); consumers that expected the pre-debounced immediate fire will now see delivery deferred to the flush tick.
• StaticPin refs for onPress/onLongPress mean handler identity changes are now observed on every render — intentional, but worth spot-checking that no downstream consumer was relying on the old frozen-at-mount behavior.
• zoomCenter JSDoc correction and the {0,0} → {W/2, H/2} change in doubleTapZoomToCenter flip the semantics external callers may have been coding against.

Change details

Listener lifecycle in zoomTo()

• Renamed the captured outer listener id to capturedListenerId so the completion callback cannot accidentally remove a listener registered by a later zoomTo() call (no-shadow + correctness).
• Added if (!this.mounted) return at the top of the .start() callback to prevent post-unmount onZoomAfter fires when consumers supply an external zoomAnimatedValue (componentWillUnmount intentionally skips stopAnimation() in that case).
• Added finished check from the animation callback so onZoomAfter only fires when the animation completes naturally, not when interrupted or cancelled.
• After natural completion, flush() the debounced onStaticPinPositionChange before calling onZoomAfter so consumers reading pin position in onZoomAfter get the final post-animation value.

Double-tap zoom cycle-back (_getNextZoomStep)

• Reordered guards so the configured-maxZoom equality check runs before the zoomStep == null guard — previously zoomStep={null} + configured maxZoom silently lost cycle-back on double-tap at max.
• Added an effectiveMax fallback of initialZoom * (1 + zoomStep)^3 when maxZoom == null, so unlimited-zoom consumers also cycle back instead of growing indefinitely.
• Inline comment clarifies that the second equality guard is only live when maxZoom == null.

doubleTapZoomToCenter + zoomTo JSDoc

• zoomCenter coordinates are viewport-relative (top-left origin), not center-relative. Updated the JSDoc to match reality.
• Changed doubleTapZoomToCenter to use { originalWidth/2, originalHeight/2 } as the center, matching the corrected semantics.

zoomTo() boundary checks

• Swapped truthy checks for != null so minZoom={0} is honored (previously silently skipped, allowing negative zoom / flipped content).

onStaticPinPositionChange firing

• Replaced the direct _updateStaticPin() call at the end of _handlePanResponderEnd with debouncedOnStaticPinPositionChange.flush() to avoid the immediate + ~100 ms debounce double-fire.
• Same replacement for the single-tap pan animation's completion callback.
• Deleted the now-unreachable private _updateStaticPin method.

_setNewOffsetPosition

• Removed the redundant zoomAnim.setValue(this.zoomLevel) call that caused onTransform (and therefore onStaticPinPositionMove) to fire twice per pan frame.

StaticPin stale-closure fix

• Added onPressRef / onLongPressRef refs updated on every render, and called through the refs inside the PanResponder callbacks so handler identity changes after mount are honored. Previously the PanResponder was created once inside useRef and froze the handlers at mount.

[thomasttvo]

@claude review

[thomasttvo]

@claude review

[claude]

Latest commit cleanly addresses my prior nit on the missed cf2d4d7 + cda405e double-tap state cleanups, with the right Move-transition + long-press-body strategy (not the reverted Grant-based one).

Extended reasoning...

Overview

This commit is a small, targeted fix: three short delete blocks in src/ReactNativeZoomableView.tsx (long-press timer body in _handlePanResponderGrant, pinch transition and shift transition in _handlePanResponderMove) plus a corresponding bullet update in SPECS.md and a SHARED_TASK_NOTES.md history entry. It restores two double-tap state cleanups that the class component had explicitly added (commits cf2d4d7 + cda405e) and that the PR #150 functional conversion dropped — exactly what my prior nit-severity comment flagged.

Security risks

None. The change manipulates internal gesture-disambiguation refs (doubleTapFirstTapReleaseTimestamp / doubleTapFirstTap) within an already-trusted local component. No I/O, no user input parsing, no auth-adjacent code paths.

Level of scrutiny

Low for this commit, despite being part of a larger PR. The fix is mechanical (delete two refs in three specific code paths), the strategy is precisely the one documented in the class-component git history, and the PR author explicitly avoids the previously-reverted a716099 Grant-based approach with a written rationale in SHARED_TASK_NOTES.md. Verified all three insertion points match the strategy in my prior review and the cited class commits.

Other factors

• The bug-hunting system found no new issues on this commit.
• All my previous findings on this PR have been addressed across the iteration cycle (mounted-guard in zoomTo(), JSDoc viewport-coordinate correction, double-tap cycle-back for both zoomStep=null+configured maxZoom and maxZoom=null cases, debounce flush at zoomTo() and single-tap completion, dead _updateStaticPin removal, capturedListenerId rename, redundant zoomAnim.setValue removal, pinch-during-zoomTo race via _cancelInFlightZoomToAnimation).
• CI is green; yarn typescript && yarn lint pass per the task notes.
• The PR has an existing human approval (elliottkember LGTM) and the only outstanding concern in the timeline was the one this commit resolves.

[thomasttvo]

@claude review

[thomasttvo]

@claude review

[thomasttvo]

@claude review

[thomasttvo]

@claude review

[thomasttvo]

@claude review

[thomasttvo]

@claude review