Skip to content

feat: NonScalingOverlay translate-only marker overlay#178

Draft
thomasttvo wants to merge 29 commits into
masterfrom
thomas/nonscaling-overlay
Draft

feat: NonScalingOverlay translate-only marker overlay#178
thomasttvo wants to merge 29 commits into
masterfrom
thomas/nonscaling-overlay

Conversation

@thomasttvo
Copy link
Copy Markdown
Collaborator

@thomasttvo thomasttvo commented May 14, 2026

Adds NonScalingOverlay, a translate-only marker overlay sibling of the zoom subject — markers stay visually constant (no scale-comp work in consumer code) while still tracking the underlying content's translation. Replaces the old FixedSize model.

Key changes

  • src/components/NonScalingOverlay.tsx (new) — translate-only overlay component
  • src/components/StaticPin.tsx — built on the overlay; per-pin scale comp goes away
  • src/components/FixedSize.tsx — removed (redundant after overlay lands)
  • src/ReactNativeZoomableViewContext.tsx — drop inverseZoom / inverseZoomStyle (dead after FixedSize removal)
  • example/ — Expo prebuild hygiene, image source swap (placekitten → picsum), metro peer dedupe, container sizing via aspectRatio

Test Plan

  • Example app: pinch / pan with default StaticPin — pin stays visually constant size at all zoom levels
  • Example app: rotate, then pinch — pin tracks content translation without scale distortion
  • Diana smoke test against this beta tarball (NonScalingOverlay used by Diana's pin overlay)

🤖 Generated with Claude Code

@cursor
Copy link
Copy Markdown

cursor Bot commented May 14, 2026

PR Summary

Medium Risk
Adds a new overlay rendering path tied into the zoom/pan transform pipeline and updates public docs/contracts around callbacks/worklets, which may affect consumers relying on prior overlay/static-pin and gesture callback behavior.

Overview
Adds NonScalingOverlay, a translate-only overlay that tracks pan/zoom (and optional rotation) while keeping overlay children at constant on-screen size, and wires it into ReactNativeZoomableView via the renderOverlay pattern.

Updates the public contract/docs (README.md, SPECS.md) to reflect the current Reanimated/RNGH implementation: worklet-thread callbacks (*Worklet), updated static-pin move callback (onStaticPinPositionMoveWorklet), ref method signatures (including moveStaticPinTo), and various naming/semantics clarifications.

Modernizes the Expo example to exercise the overlay/static-pin/worklet APIs (including modal usage), adds the Reanimated Babel plugin, improves Metro peer-dependency deduping when aliasing src/, upgrades example dependencies, and tightens repo hygiene (ignore regenerated example/ios).

Reviewed by Cursor Bugbot for commit 8d9151b. Configure here.

thomasvo and others added 12 commits May 13, 2026 19:31
Adds a `NonScalingOverlay` component plus a `renderOverlay` prop on
`ReactNativeZoomableView` for layering markers (pins, dots, labels)
over zoomable content without inverse-scaling each child. Markers
render at 1:1 screen size at every zoom level — the overlay box
itself absorbs zoom/pan via a translate-only `useAnimatedStyle`,
while children position via `left: 'X%' / top: 'Y%'` against the
overlay's content-percentage frame.

Why this matters
- The pre-existing `FixedSize` primitive uses an outer scale + per-
  child inverse-scale transform. On iOS that reduces CALayer
  contentsScale and produces visible pixelation when zoomed in.
  It also pulls markers toward the transform origin (clumping)
  because every child shares the same scale anchor. The new
  translate-only model has neither problem.

Public API
- `renderOverlay?: () => React.ReactNode` on `ReactNativeZoomableView`.
  When supplied, the library mounts the returned tree inside a
  `<NonScalingOverlay>` as a sibling of the inner zoom layer, so
  the user wires zero plumbing.
- `NonScalingOverlay` exported as a standalone component for
  advanced consumers who need manual mount control. Self-contained
  props (numeric content/wrapper sizes + SharedValue zoom/offset/
  optional rotation), so it can be used outside the library's
  context if needed.
- `FixedSize` untouched (back-compat).

Implementation notes
- `useAnimatedStyle` sizes the overlay container to `contentW*z
  x contentH*z` and translates it to align with the inner zoom
  layer. With `rotation` the 5-element transform list is required
  to pivot around the overlay geometric center and apply pan in
  the rotated frame; comments in `NonScalingOverlay.tsx` describe
  the matrix-composition mechanism.
- Children are wrapped in a fake `ReactNativeZoomableViewProvider`
  with zoom=1 / offsets=0 / inverseZoom=1 so nested
  `useZoomableViewContext` consumers (including `FixedSize`)
  become no-ops and don't double-counteract zoom inside the
  overlay.
- The main component mirrors wrapper width/height from
  `measureZoomSubject` into React state alongside the existing
  SharedValue path, so `NonScalingOverlay` receives them as plain
  numbers for its worklet-closure math.

Example
- `example/App.tsx` now uses `renderOverlay` to render the 4x4
  dot grid instead of mapping `FixedSize` children. The dots use
  the existing `styles.marker` plus per-dot `left/top` overrides.

iOS Local Network Privacy
- `example/app.json` adds expo.ios.infoPlist.NSLocalNetworkUsageDescription
  so future expo prebuild regens preserve the key.
- `example/ios/.../Info.plist` carries the same key so the current
  prebuild artifact lets the example reach Metro over LAN on iOS
  14+ physical devices. Without it the OS blocks every Metro probe
  at the nw_path layer and RCTBundleURLProvider returns nil
  -> "No script URL provided" red screen.
…ruth)

example/ios/ is regenerated end-to-end by `expo prebuild` from
example/app.json. Tracking the prebuilt Info.plist (added in the
previous commit alongside the NonScalingOverlay feature) was a
mistake — the .plist will be re-emitted from
`expo.ios.infoPlist.NSLocalNetworkUsageDescription` on the next
prebuild, so committing it creates churn and lets stale artifacts
mask config drift. Anything that must persist across prebuilds
belongs in app.json's expo.ios section, which is what we already
landed.

- Remove example/ios/.../Info.plist from the index (file kept
  on disk so the current Metro session stays working).
- Add `example/ios/` to .gitignore so future contributions don't
  accidentally re-commit any prebuild output.
Second of a 3-commit sequence:
  1. feat: NonScalingOverlay + iOS Local Network privacy fix
     (abbeddf — added NSLocalNetworkUsageDescription in both
     app.json and the prebuilt Info.plist).
  2. chore: untrack prebuild Info.plist + add example/ios/ to
     .gitignore (a48cf44 — overreach: the gitignore add was
     unwarranted).
  3. THIS commit — revert only the gitignore add.

`example/ios/` was previously untracked (never committed), NOT
.gitignored. Adding a blanket `example/ios/` ignore in commit
`a48cf44` would have silently swallowed legitimate future native
edits — Podfile, AppDelegate, native modules, build settings —
that contributors expect to commit. Those edits don't come from
`expo prebuild` and shouldn't be hidden by gitignore.

What stays from a48cf44 (correct):
- `example/ios/.../Info.plist` remains removed from the index.
  app.json's `expo.ios.infoPlist.NSLocalNetworkUsageDescription`
  is the source of truth for the one key we needed; the .plist is
  regenerated by `expo prebuild`.

What this revert removes (incorrect from a48cf44):
- The `example/ios/` block in .gitignore — the existing
  `example/ios/Pods` entry is preserved (Pods are genuinely
  generated by `pod install` and shouldn't be tracked).
This project uses the bare iOS workflow — `example/ios/` is
committed source, not regenerated by `expo prebuild`. That makes
the `expo.ios.infoPlist.*` block in `app.json` inert: it only
takes effect during prebuild, which we don't run. The actual
`Info.plist` is the source of truth, so we should track it directly.

Changes vs the previous two commits in this sequence (`abbeddf`
landed both, `a48cf44` removed the .plist):
- Revert the `expo.ios.infoPlist` block in `example/app.json` —
  it was added on the (incorrect) assumption that prebuild would
  regenerate the .plist.
- Re-add `example/ios/.../Info.plist` to the tracked set with
  `NSLocalNetworkUsageDescription` present. Without this key, iOS
  14+ blocks every Metro probe at the `nw_path` layer
  (`unsatisfied (Local network prohibited)`), `RCTBundleURLProvider`
  returns `nil`, and the example shows the "No script URL provided"
  red screen on physical devices.

Commit sequence on this branch:
  1. `abbeddf`: feat — NonScalingOverlay + initial Info.plist +
     redundant app.json infoPlist.
  2. `a48cf44`: chore — untrack Info.plist + overreaching gitignore add.
  3. `1562181`: revert — drop the gitignore add only.
  4. THIS commit — drop app.json prebuild config; track Info.plist
     as source.
`placekitten.com` returns DNS NXDOMAIN — it has been offline for an
extended period and the example app renders the kitten URL into a
white box, leaving `applyContainResizeMode` with no visible content
to fit the 4x4 marker grid against.

`picsum.photos` (Lorem Picsum) is the reliable drop-in: same
`/<W>/<H>` URL shape, CDN-backed via Cloudflare, HTTPS with TLSv1.3.
Loses the cat theme — `placecats.com` is the cat-themed alternative
and was tried first — but `placecats.com` would not render visibly
on the iPhone 16 Pro during verification (image element stayed
invisible across multiple kill+launch cycles), whereas `picsum.photos`
renders a fresh photo behind the dots immediately on reload.
Reliability wins for an example app meant to render the image on
every fresh install.
…currentRotation=0 default

The 5-element list `[translate-center, translate-center, rotate,
translate-pan, translate-pan]` with rotation=0 reduces to the
2-element combined-translate form (identity rotation contributes
nothing under matrix composition), so a single code path covers
both cases. Removes the duplicated math in the ternary branches
and the long matrix-composition tutorial that was inline.

Renames the worklet locals from `z`/`ox`/`oy` to
`currentZoom`/`currentOffsetX`/`currentOffsetY`/`currentRotation`
so the math at the call sites reads in full words.

No behavioral change. Sanity-screenshot confirms the 4x4 marker
grid still renders correctly over the example app's image.
…nly model

`FixedSize` used the inverse-scale model: an outer scale transform
on the zoom subject + per-child `scale: 1/zoom` to keep markers at
constant visual size. On iOS, the per-child inverse-scale reduces
CALayer.contentsScale and causes visible pixelation at high zoom.
It also pulls markers toward the transform origin (clumping) because
every child shares the same scale anchor.

`NonScalingOverlay` (added in `abbeddf`) supersedes it via the
translate-only model: a single overlay box absorbs zoom/pan in its
own transform, children render at 1:1 screen size, no per-child
inverse-scale, no CALayer pixelation, no clumping. The lib's
`renderOverlay` prop wraps it for the common case.

Changes
- Delete `src/components/FixedSize.tsx`.
- Drop `FixedSize` from `src/index.tsx` public exports.
- Refresh the `NonScalingOverlay` fake-context WHY-comment so it no
  longer name-drops `FixedSize` — the rationale (neutralize any
  nested consumer of `useZoomableViewContext` that would multiply
  the outer `inverseZoomStyle` on top of the translate-only model)
  applies to any consumer, not just the deleted primitive.
- SPECS.md: replace the `FixedSize` entry under New API and New
  public exports with the `NonScalingOverlay` description.

Potential follow-up (NOT in this commit)
- `ZoomableViewContextValue.inverseZoom` and `inverseZoomStyle` were
  effectively only consumed by `FixedSize`. They remain on the
  context so external consumers depending on those fields don't
  break silently. Deciding to remove them is a separate breaking
  change and should be evaluated against downstream callers.
Root cause: the lib's wrapper container uses
`alignItems: 'center', justifyContent: 'center'`. Unlike standard CSS,
RN/Yoga positions a `position: 'absolute'` child via the parent's
`alignItems`/`justifyContent` whenever the child has a measurable size.
`useAnimatedStyle` set `width: contentW * z, height: contentH * z`,
which Yoga DID see, so Yoga centered the (340x340) overlay inside the
(340x590) wrapper at wrapper-y=125. The transform's
`translateY = wrapperH/2 - z*contentH/2 = 125` then added another
125pt, painting the overlay at wrapper-y=250 instead of 125 — every
dot landed below where it should have. On iPhone 12 Pro this surfaced
as the bottom row of dots rendering in the white letterbox below the
image.

Forcing `top: 0, left: 0` overrides the parent's alignment so the
transform's translates are the SOLE source of position. Verified on
iPhone 12 Pro / Expo Go SDK 54: at z=1 the overlay precisely overlaps
the rendered image; pinch-in fans dots outward symmetrically around
image center; pan moves dots together with image content.

Also collapses the `transform: rotation ? [5] : [2]` ternary into a
single 5-element list with a constant-zero `SharedValue` default. The
no-rotation case is mathematically the same product with `rotate(0) =
I`; the conditional was redundant. Removed the in-progress
`runOnJS`/`react-native-worklets` console-log diagnostic that was no
longer needed once the root cause was identified.

example/style.ts: replace `box.width: 480` with
`Dimensions.get('window').width - 40`. The fixed 480pt overflows the
390pt iPhone 12 Pro viewport. `width: '100%'` doesn't work because
the nested container chain uses `alignItems: 'center'`, leaving the
parent's cross-axis intrinsic-sized — '100%' resolves to 0.

example/App.tsx + style.ts: add an \`overlayDebugBox\` View as a child
of \`renderOverlay\` (100% × 100% magenta-bordered fill) so the
overlay's bounding box is visible at every zoom level. Keeps debug
visualization in example code only — library code stays clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…hemeral

The example app deploys via Expo Go (SDK 54), not via a bare/prebuild
iOS build. `example/ios/` was committed solely so an earlier Local
Network Privacy fix (NSLocalNetworkUsageDescription in Info.plist)
would survive on-device builds — that fix is now sourced from
`example/app.json`'s `expo.ios.infoPlist` block (which a future
`expo prebuild` would regenerate the Info.plist from), and Expo Go
loads the JS bundle without ever touching the native iOS project, so
nothing in `example/ios/` participates in the deploy path.

Untrack the lone tracked file (`Info.plist`) and gitignore
`example/ios/` so any future regenerated prebuild output stays out of
git.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The example previously measured the <Image> element box and ran the
contain-fit math (`applyContainResizeMode`) to derive the rendered-
pixel frame for `NonScalingOverlay`'s `contentWidth`/`contentHeight`.
Two pieces of state, one helper call, all to recover the
letterbox-stripped frame.

Setting `aspectRatio` on the contents View (inline, computed from the
image's source dims captured via `onLoad`) makes the element's frame
match the image's aspect exactly. resizeMode:contain produces zero
letterbox in that case — the element frame IS the rendered frame —
and the contents View's `onLayout` returns the contentSize directly.
`applyContainResizeMode` is no longer imported; `imageElementBox`
state is gone; the contentSize useMemo collapses into a plain
useState set from onLayout.

Tradeoff: this works when the contents fit within the wrapper at
parent-cross-axis × aspect (true here — picsum is square in a
340×590 wrapper). For arbitrary aspects in arbitrary wrappers, callers
who can't guarantee that constraint can either pre-compute the binding
axis externally or fall back to the previous contain-math pattern.

Also restores the in-overlay debug HUD (yellow text pinned to the
overlay's top-left, showing wW / wH / cW / cH / tXjs / tYjs) so
alignment can be visually cross-checked against raw values without
leaving the screen — moved from library code to the example via a new
`overlayDebugHud` style. Library code stays clean.

Pre-commit lint: removed the now-unused `applyContainResizeMode`
import.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the original disableHierarchicalLookup + peer-blocklist +
per-peer extraNodeModules stack with a single resolveRequest
interceptor that redirects every peer import (and any subpath of it)
to example/node_modules/<peer> regardless of where the importing
file lives. resolveRequest runs BEFORE hierarchical lookup, so it
dedupes peers even when ../src/ imports them via the lib alias.

The previous stack worked by disabling hierarchical lookup wholesale
and using extraNodeModules as the resolver — but with lookup
disabled, transitive subpath imports outside the explicit allow-list
(most notably Reanimated 4.1.x's validate-worklets-version.js
requiring semver/functions/satisfies) could not be resolved without
dragging an extra "semver": "7.7.2" dep into example/package.json
solely to plant a v7 copy where Metro could find it.

With hierarchical lookup back on (default) plus the resolveRequest
peer dedupe, Reanimated's nested semver@7.7.2 resolves naturally
from node_modules/react-native-reanimated/node_modules/semver/, and
the explicit example-level pin comes out.

Verified on iPhone 12 Pro / Expo Go SDK 54: bundle compiles (~8.8 MB,
down from ~16 MB with the previous redundant peer copies), app
renders, peers single-instance, dots align at default zoom and pinch
tracks.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ce after FixedSize removal

FixedSize was the sole consumer (removed in 85daf6b). NSOL's
fake-context provider was solving for that consumer and is also
gone. Context now exposes the minimal surface: { zoom, offsetX,
offsetY }. Client code that wants inverse-scale behavior reads
zoom and computes 1/zoom themselves.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@thomasttvo thomasttvo force-pushed the thomas/nonscaling-overlay branch from 8d9151b to b8a6df6 Compare May 14, 2026 02:32
@thomasttvo
Copy link
Copy Markdown
Collaborator Author

@claude review once

Comment on lines +99 to +103
{ translateX: z * ox },
{ translateY: z * oy },
],
};
}, [contentWidth, contentHeight, wrapperWidth, wrapperHeight]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 NonScalingOverlay: useAnimatedStyle's explicit deps array [contentWidth, contentHeight, wrapperWidth, wrapperHeight] omits rotation, but the worklet reads rotationValue.value (= rotation ?? zeroRotation). Consumers who toggle the optional rotation prop between undefined and a SharedValue — a pattern explicitly enabled by the JSDoc and SPECS.md L42 — silently freeze rotation at 0 because no listed dep changes, so the worklet is never re-created. Fix: include rotation (the prop reference) in the deps array.

Extended reasoning...

What goes wrong

NonScalingOverlay.tsx:99-103 passes an explicit deps array to useAnimatedStyle:

const overlayStyle = useAnimatedStyle(() => {
  const z = zoom.value;
  const ox = offsetX.value;
  const oy = offsetY.value;
  const r = rotationValue.value;
  ...
}, [contentWidth, contentHeight, wrapperWidth, wrapperHeight]);

An explicit deps array overrides Reanimated/Babel's auto-detection — only the listed values control when the worklet is re-serialized to the UI thread. SharedValue auto-tracking subscriptions are established at worklet-creation time, so whichever SharedValue is read in the captured worklet body is the one Reanimated subscribes to.

The worklet reads from rotationValue, which is derived per render at line 80:

const zeroRotation = useSharedValue(0);
const rotationValue = rotation ?? zeroRotation;

Neither rotation nor rotationValue is in the deps array.

Step-by-step proof

Consider a consumer using the documented pattern (the rotation prop is public per SPECS.md L42 and the JSDoc on NonScalingOverlayProps.rotation):

const [rotationOn, setRotationOn] = useState(false);
const rotSv = useSharedValue(0);

<NonScalingOverlay
  rotation={rotationOn ? rotSv : undefined}
  zoom={zoom} offsetX={offsetX} offsetY={offsetY}
  contentWidth={cw} contentHeight={ch}
  wrapperWidth={ww} wrapperHeight={wh}
/>
  1. Mount with rotationOn=false. rotation is undefined, so rotationValue = zeroRotation. The worklet is created closing over zeroRotation; Reanimated subscribes to zeroRotation (which never changes).
  2. User flips rotationOn to true (no content/wrapper resize). rotation is now rotSv, so on this render rotationValue = rotSv. None of [contentWidth, contentHeight, wrapperWidth, wrapperHeight] changed, so the worklet is not re-created — the UI thread keeps executing the original body which still references the captured zeroRotation.
  3. Consumer animates rotSv.value via gesture or withTiming. Reanimated is not subscribed to rotSv (auto-tracking was on zeroRotation), so the worklet does not re-run. The overlay's rotate transform stays at 0rad.
  4. Symmetrically, swapping SharedValue identity (rotation={enabled ? svA : svB}) suffers the same fate — the worklet keeps the original SharedValue binding regardless of swap.

Why existing code doesn't prevent it

The zeroRotation = useSharedValue(0) indirection was introduced specifically to make the transform list shape identical in the rotated/non-rotated cases (per the JSDoc comment at line 75-78), but it has the side effect that rotationValue is always a fresh local binding rather than the prop itself. There is no useLayoutEffect mirroring the prop into a stable inner SharedValue either.

The in-tree call site at ReactNativeZoomableView.tsx:1736-1744 does not pass rotation, so the library's own renderOverlay path is unaffected — but the prop is exported (NonScalingOverlayProps from src/index.tsx) and SPECS.md L42 explicitly documents manual mounting with (and optional rotation). A first-party consumer hitting this would observe a silent failure with no error.

Fix

Include rotation (the prop itself, treating undefined as a sentinel) in the deps array:

}, [contentWidth, contentHeight, wrapperWidth, wrapperHeight, rotation]);

rotation is either undefined or a stable SharedValue reference, so this only invalidates the worklet at the toggle/swap moment — not per gesture frame. Alternative: mirror the prop into a single inner useSharedValue and update it via useLayoutEffect whenever the prop reference changes; the worklet would then always close over the same stable SharedValue.

Severity

Normal — this is a new public-API correctness bug introduced by this PR. The in-tree usage doesn't exercise it, but rotation is a documented, exported, optional prop and the toggle/swap pattern is the natural way a consumer would gate rotation on user input.

Comment thread example/App.tsx
Comment on lines +284 to +310
{/* DEBUG: visualize the overlay's bounding box.
Sized 100% × 100% of the overlay so it tracks
contentSize × zoom and reveals where the
translate-only overlay is actually painting on
screen. Example-only — remove for production. */}
<View style={styles.overlayDebugBox} />
{/* DEBUG HUD pinned to overlay's top-left so it
tracks the overlay's transform and provides
the live numbers used by the translate math
(translateX/Y at z=1, ox=oy=0). */}
<Text style={styles.overlayDebugHud}>
NSOL wW≈{Math.round(wrapperApproxW)} wH≈
{Math.round(wrapperApproxH)} cW=
{Math.round(contentSize.width)} cH=
{Math.round(contentSize.height)} tXjs=
{Math.round(
wrapperApproxW / 2 - contentSize.width / 2
)}{' '}
tYjs=
{Math.round(
wrapperApproxH / 2 - contentSize.height / 2
)}
</Text>
{[20, 40, 60, 80].map((left) =>
[20, 40, 60, 80].map((top) => (
<View
key={`${left}x${top}`}
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 example app ships debug-only visualizations the author themselves flagged for removal: overlayDebugBox (red 18%-opacity rect with magenta border), overlayDebugHud (yellow text overlay with raw transform math), and a DBG contentSize: Text line below the zoomable view — all with comments saying "Example-only — remove for production". Separately, the inline comment on the contents View (around lines 320-336) describes maxWidth/maxHeight: '100%' and alignSelf: 'center' styles that don't exist anywhere — styles.contents only sets alignSelf: 'stretch', and the style.ts comment is the accurate one. Either delete the debug surfaces and fix the comment before merge, or gate them behind a __DEV__ toggle.

Extended reasoning...

Two cleanup items in example/App.tsx, both example-only

1. Debug surfaces marked "remove for production" still ship unconditionally

Three debug-only render outputs are committed:

  • example/App.tsx:288-289<View style={styles.overlayDebugBox} />, immediately preceded by the comment /* DEBUG: visualize the overlay's bounding box. ... Example-only — remove for production. */. The style itself (example/style.ts:60-72) is a magenta-bordered, 18%-opacity red rect filling the overlay; its own comment reads Example-only debug visualization for NonScalingOverlay's bounding box. ... Not exported by the library.
  • example/App.tsx:290-306 — yellow overlayDebugHud Text pinned to the overlay's top-left, displaying raw transform math: NSOL wW≈… wH≈… cW=… cH=… tXjs=… tYjs=…. Style at example/style.ts:73-85.
  • example/App.tsx:362-366<Text>DBG contentSize: …×… box: …×…</Text> rendered below the zoomable view (outside the showMarkers gate — it always renders).

The overlay debug surfaces are gated only by showMarkers, which is a user-facing toggle defaulting to true. The DBG Text below has no gate at all.

2. App.tsx contents-View comment contradicts the actual styles

example/App.tsx:320-336 (the comment on the contents <View>) describes the layout strategy as:

Combined with maxWidth/maxHeight: '100%' and alignSelf: 'center', the element fits within the wrapper on whichever axis is binding…

But example/style.ts:28-41 (styles.contents) only sets alignSelf: 'stretch'. There is no maxWidth, no maxHeight, and alignSelf is stretch, not center. The only inline override applied at use time is { aspectRatio: sourceAspect }. The neighboring style.ts comment (alignSelf: 'stretch' anchors cross-axis, aspectRatio derives main-axis, parent's justifyContent: 'center' centers it vertically) is the accurate description — so the two comments actively contradict each other.

Step-by-step proof

Take example/App.tsx at HEAD on this branch, render in a simulator with default props (showMarkers = true, the initial state on line 196):

  1. renderOverlay fires the truthy branch (line 273-330).
  2. The overlay tree contains <View style={styles.overlayDebugBox} /> — a magenta-bordered, red 18%-opacity rectangle fills the entire overlay region.
  3. <Text style={styles.overlayDebugHud}>NSOL wW≈… cH=…</Text> paints a yellow band of debug math across the top of the image.
  4. Below the zoom box, <Text>DBG contentSize: …×… box: …×…</Text> renders unconditionally — flipping showMarkers off does not hide it.

For the comment mismatch: grep -n 'maxWidth\|maxHeight\|alignSelf' example/style.ts returns exactly one match — alignSelf: 'stretch' inside styles.contents. The strings maxWidth, maxHeight, and alignSelf: 'center' cited by the App.tsx comment do not appear anywhere in the file.

Why this is "nit" severity

This is the example app, not library source — published consumers are unaffected. The library exports NonScalingOverlay and the renderOverlay prop; the debug rectangles, HUD, and DBG line live in the consumer-side demo and never reach the npm artifact. But the example app is the library's primary public demo ("Example app: pinch / pan with default StaticPin" appears in this PR's own test plan), and the author's own "Example-only — remove for production" annotation is the strongest possible evidence the items are unintentional carryover from development.

Fix

Either:

  • Delete the three debug surfaces (App.tsx:288-306, App.tsx:362-366) and the two style entries (style.ts:56-85) before merge, OR
  • Wrap each in an if (__DEV__ && debug) (or a dedicated state toggle distinct from showMarkers) so the default demo experience is clean.

Either way, update the App.tsx contents-View comment (lines 320-336) to match what's actually applied — drop the maxWidth/maxHeight / alignSelf: center description and reference the same alignSelf: stretch + aspectRatio explanation that the style.ts comment already gives.

Comment on lines +105 to +110
// The translate math (`wrapperW/2 - z*contentW/2 + z*ox`) requires real
// dimensions; with 0s it resolves to 0 and paints the overlay at the
// wrong location for one frame before measurements arrive.
if (!contentWidth || !contentHeight) return null;

return (
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 NonScalingOverlay early-return guard misses wrapper dimensions. The check at src/components/NonScalingOverlay.tsx:108 only short-circuits on !contentWidth || !contentHeight, but the transform math (wrapperW/2 - z*contentW/2, wrapperH/2 - z*contentH/2) is equally sensitive to wrapper dimensions — when those are still 0 (between mount and the wrapper's onLayout firing), the overlay paints one frame at (-contentW/2, -contentH/2) (off the top-left) before settling. Fix: extend the guard to all four — if (!contentWidth || !contentHeight || !wrapperWidth || !wrapperHeight) return null;

Extended reasoning...

What the bug is

NonScalingOverlay's early-return guard at src/components/NonScalingOverlay.tsx:108 is asymmetric with the transform math it is supposed to protect:

transform: [
  { translateX: wrapperWidth / 2 - (z * contentWidth) / 2 },
  { translateY: wrapperHeight / 2 - (z * contentHeight) / 2 },
  { rotate: `${r}rad` },
  { translateX: z * ox },
  { translateY: z * oy },
],
// ...
if (!contentWidth || !contentHeight) return null;

The author's own comment on lines 105-107 articulates the invariant correctly — "real dimensions" (plural) are required — but the guard only enforces it for half the inputs. When wrapperWidth/wrapperHeight are 0 but contentWidth/contentHeight have non-zero values, the worklet runs and the first two translates collapse to -(z*contentW)/2 and -(z*contentH)/2, painting the overlay roughly half its width and half its height off the top-left for one frame before re-rendering with the real wrapper dims.

Why existing code doesn't prevent it

In src/ReactNativeZoomableView.tsx (~L82), wrapperSize initializes as { width: 0, height: 0 } and only updates when the wrapper View's onLayout fires. NonScalingOverlay is mounted at L1734 with wrapperWidth={wrapperSize.width} / wrapperHeight={wrapperSize.height} and contentWidth={propContentWidth ?? 0} / contentHeight={propContentHeight ?? 0}. The documented public-API pattern (per SPECS.md and the JSDoc on NonScalingOverlayProps) lets a consumer pass contentWidth/contentHeight as static numeric props, which means those values are non-zero on the very first render while wrapperSize is still {0, 0}. The guard at line 108 lets the render through; the worklet computes the misposition.

Step-by-step proof

Consumer code (matches the documented API):

<ReactNativeZoomableView
  contentWidth={500}
  contentHeight={500}
  renderOverlay={() => <Marker />}
>
  ...
</ReactNativeZoomableView>
  1. Initial render of ReactNativeZoomableView: wrapperSize state = { width: 0, height: 0 } (declared on L82, never written yet).
  2. NonScalingOverlay mounts with contentWidth=500, contentHeight=500, wrapperWidth=0, wrapperHeight=0, zoom.value=1, offsetX=offsetY=0.
  3. Guard at L108 evaluates !500 || !500false || falsefalse — does not return null.
  4. useAnimatedStyle worklet computes translateX = 0/2 - (1 * 500)/2 = -250, translateY = -250.
  5. First commit paints the overlay 250pt off the top-left of the wrapper — visible as a one-frame flash before settling.
  6. The wrapper's onLayout fires (a tick later), setWrapperSize({width: W, height: H}) runs, the next commit paints the overlay correctly.

Impact

A one-frame flash on initial mount (and on any wrapper resize that lands wrapperSize to {0,0} momentarily) for consumers who use the documented pattern of passing static contentWidth/contentHeight numeric props. The example app happens to mask this because it derives contentSize from a child View's onLayout (after the image loads), so contentSize is also {0,0} until well after wrapperSize lands — but consumers following the public-API docs (or driving the overlay from known image dimensions, animation source, etc.) hit it. Not a persistent bug, just a visual transient at mount.

Fix

One-line, matching the comment's own stated invariant:

if (!contentWidth || !contentHeight || !wrapperWidth || !wrapperHeight) return null;

thomasvo and others added 4 commits May 13, 2026 23:05
- devDeps: jest, babel-jest, @testing-library/react-native, react-test-renderer, @types/jest
- jest.setup.ts wires reanimated/mock + RNGH jestSetup
- package.json jest config: setupFiles + explicit transformIgnorePatterns
- `yarn jest --passWithNoTests` passes locally

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ests

- Pure helper covering EC-NSO-3/4/5/6 transform-math contracts
- Worklet directive preserved; useAnimatedStyle now delegates
- Unit tests cover 5-element shape invariant, centering math, zoom scaling,
  pan in rotated frame, negative/zero/large inputs

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Zero-dim guard (contentWidth/Height = 0 -> null)
- Rotation prop toggle does not change hook count
- Static styles: position:absolute, top:0, left:0, overflow:visible
- pointerEvents='none' prop
- Children pass-through
- Width/height/transform plumbing under reanimated mock

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-NSO-10–14

- Overlay branch gating by contentWidth/Height
- Mount order: overlay paints under StaticPin (sibling order)
- Overlay shares coordinate frame with GestureDetector (wrapper sibling)
- onLayout 0×0 doesn't overwrite wrapperSize state
- Identical onLayout dims dedup (no spurious re-renders)
- Adds testID="zoom-subject-wrapper" on wrapper View for RTL reachability

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@thomasttvo
Copy link
Copy Markdown
Collaborator Author

Test suite added (24 tests, 4 suites)

Added test infrastructure + 24 tests covering NonScalingOverlay and the renderOverlay integration surface — all 14 EC-NSO edge cases pulled from PR #151's review now have regression tests.

Commits:

  • e76d21f chore(test): add jest + RTL + reanimated mock infra
  • d41f10d refactor(NonScalingOverlay): extract computeOverlayTransform + unit tests
  • 0ae8f80 test(NonScalingOverlay): component tests for EC-NSO-1, 2, 7, 8, 9 (also contains the CI test-step addition + the ## NonScalingOverlay contract section in SPECS.md — commit message under-reports the content; see diff for full picture)
  • a22d18e test(RNZV): renderOverlay integration tests for EC-NSO-10–14

Coverage map:

Layer Tests Edge cases
computeOverlayTransform unit 9 EC-NSO-3, 4, 5, 6 + numerics
<NonScalingOverlay> component (RTL + reanimated/mock) 9 EC-NSO-1, 2, 7, 8, 9
ReactNativeZoomableView.renderOverlay integration (RTL + RNGH mock) 6 EC-NSO-10, 11, 12, 13, 14

Test infra:

  • Jest 29, @testing-library/react-native v12, react-test-renderer@18.3.1, react-native-reanimated/mock, RNGH jestSetup + inline RNGH mock for ReactNativeZoomableView integration imports.
  • New CI step in .github/workflows/lint.yml: yarn test --ci --runInBand.
  • SPECS.md now has a ## NonScalingOverlay contract section codifying the prop shape, transform formula, static-style rules, mount-order rules, and wrapperSize mirror semantics.

Follow-up planned (separate commits, same PR): the rest of SPECS.md — gesture state machine, double-tap, zoom helpers, static-pin behavior, etc. Researcher is inventorying now.

thomasvo and others added 9 commits May 13, 2026 23:47
…stGestureState

Pure-math helper for SPEC-027/028/109 unit tests. movementSensitivity=0
returns {dxShift:0, dyShift:0} (SPEC-028 silently disables panning).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…math

Pure-math helper for SPEC-095 unit tests. Formula
deltaGrowth * (1 - sensitivity*9/100) is identical at the call site.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pure-math helper for SPEC-020/023/097 unit tests. null/undefined bounds
mean unbounded on that side, matching the existing != null gate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pure-logic helper for SPEC-029/108 unit tests. Returns true when panEnabled
is false OR (disablePanOnInitialZoom && zoom === initialZoom).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Enables Phase C gesture-driven integration tests to acquire the
Gesture.Manual() instance via getByGestureTestId('canvas-gesture').

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Covers SPEC-001/004/008/009/010 (exports + peer deps),
SPEC-020/022/023/097/099-104 (getNextZoomStep cycle + clamp),
SPEC-027/028/109 (calcShiftDelta), SPEC-029/108 (shouldSkipShift),
SPEC-095 (applyPinchSensitivity), SPEC-096 (calcNewScaledOffsetForZoomCentering),
SPEC-010/133 (coordinateConversion), plus regression test for the
dy/dx swap previously seen in calcGestureTouchDistance (PR #151
thread #3076242724).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…7, 112-119, 047)

- Wrapper tree: GestureDetector + StaticPin siblings (not nested)
- StaticPin pinProps style merge (caller transform replaces anchor)
- pointerEvents defaults (wrapper box-none, icon none); pinProps override survives destructure-before-spread (threads #3107340687, #3179480336)
- Opacity 0->1 on icon onLayout, left/top from staticPinPosition

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…4, 053-055, 065-080, 107, 137-149)

- Default prop values + initial zoom/offset
- 4 cancellation paths verified onZoomEnd does NOT fire
- zoomTo timing, zoomCenter math, return-value contract
- Programmatic methods bypass panEnabled
- onLayoutWorklet unwrapped payload, onTransformWorklet centering invariant
- movementSensibility legacy alias

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…85, 117-122)

- Settle 100ms debounce + ε dedup + cancellation (threads #3164939942, #3179477073)
- Pin opacity gate, internal bottom-centre transform
- onStaticPinPositionMoveWorklet content-dim gating + payload math
- visualTouchFeedbackEnabled / debug conditional branches
- useLatestWorklet ref identity update (threads #3179033549, #3238350220)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
thomasvo and others added 4 commits May 14, 2026 10:16
…31-033, 037-041, 060-063, 092, 105-106, 123-130, 135)

Direct-gesture invocation via getByGestureTestId('canvas-gesture').
Acquires Gesture.Manual() and invokes onTouchesDown/Move/Up directly.
Covers PR #151 threads #3179084848 (doubleTapZoomToCenter math),
#3179033552 (singleTapTimeoutId cleanup), and #3179193006 cluster
(sentinel survival across recovery).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…SPEC-056-059, 081, 082)

- onPanResponderGrant/End/Terminate ordering across natural release, 3+ force-end, RNGH cancel
- gestureStarted true during gesture, reset to false AFTER end-callbacks (SPEC-082 clear-ordering)
- onPanResponderMoveWorklet intercept return-truthy short-circuits library (externallyHandled), preserves sentinel against spurious onSingleTap after intercepted drag

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-091, 093, 094, 098, 108, 110, 130, 150)

Direct-gesture invocation. Pinch/shift gesture classification, 3+ finger
force-end + recovery, tap-classification only on genuine release.
Covers PR #151 threads #3179193006 (no spurious tap on cancel) and
#3179193011 (firstTouch stability across finger lifts).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reverts the 16 test-suite commits that were inadvertently pushed onto
this branch. The test suite lives on branch `thomas/zoomable-tests`,
which now has its own PR targeting this branch as base.

This is a content-only revert — when `thomas/zoomable-tests` later
merges, the tests are re-applied. No history rewrite, no force-push.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant