Skip to content

Add contentX/contentY to ZoomableViewEvent #181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elliottkember merged 2 commits into from
May 19, 2026
Merged

Add contentX/contentY to ZoomableViewEvent #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2903b19
Add contentX/contentY to ZoomableViewEvent
elliottkember May 18, 2026
0e42844
Drop overwriteObj from _getZoomableViewEventObject
elliottkember May 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion SPECS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Exported from `src/index.tsx`:
- `ReactNativeZoomableView` — main component
- `ReactNativeZoomableViewProps` — prop type
- `ReactNativeZoomableViewRef` — imperative handle
- `ZoomableViewEvent` — `{ zoomLevel, offsetX, offsetY, originalWidth, originalHeight }`
- `ZoomableViewEvent` — `{ zoomLevel, offsetX, offsetY, originalWidth, originalHeight, contentX?, contentY? }`
- `useZoomableViewContext()` — hook returning `{ zoom, inverseZoom, inverseZoomStyle, offsetX, offsetY }` for descendants
- `FixedSize` — wrapper that keeps absolutely-positioned children at constant visual size regardless of zoom
- `applyContainResizeMode`, `getImageOriginOnTransformSubject`, `viewportPositionToImagePosition` — coordinate helpers
Expand Down Expand Up @@ -96,6 +96,8 @@ Exported from `src/index.tsx`:

All event-receiving callbacks accept `(event: GestureTouchEvent, zoomableViewEventObject: ZoomableViewEvent)`. `onZoomEnd`'s `event` is `GestureTouchEvent | undefined` (it's `undefined` on natural completion of a programmatic `zoomTo()`).

The `contentX` / `contentY` fields on `ZoomableViewEvent` carry the first touch's position in content (bitmap) coordinates — populated when the callback has an associated gesture event AND both `contentWidth` and `contentHeight` props are set. Undefined for `onTransformWorklet` (no gesture event) and for `onZoomEnd` after a programmatic `zoomTo()` completes naturally (its `event` is `undefined`). Computed under the same `contain` resize-mode assumption as `viewportPositionToImagePosition` — see [Coordinate system](#coordinate-system).

| Callback | Thread | When |
|----------|--------|------|
| `onLayoutWorklet` | UI | Internal measurements (origin/size of zoom subject) change. Receives `{ x, y, width, height }`. Skipped while measurements are zero (initial mount before the wrapper's `onLayout` fires). See [Worklet callback contract](#worklet-callback-contract). |
Expand Down
78 changes: 49 additions & 29 deletions src/ReactNativeZoomableView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,20 +344,41 @@ const ReactNativeZoomableViewInner: ForwardRefRenderFunction<
* @private
*/
const _getZoomableViewEventObject = (
overwriteObj: Partial<ZoomableViewEvent> = {}
gestureEvent?: GestureTouchEvent
): ZoomableViewEvent => {
'worklet';

return Object.assign(
{
zoomLevel: zoom.value,
offsetX: offsetX.value,
offsetY: offsetY.value,
originalHeight: originalHeight.value,
originalWidth: originalWidth.value,
},
overwriteObj
);
const event: ZoomableViewEvent = {
zoomLevel: zoom.value,
offsetX: offsetX.value,
offsetY: offsetY.value,
originalHeight: originalHeight.value,
originalWidth: originalWidth.value,
};

// Populate content (bitmap) coordinates of the first touch when caller
// supplied a gesture event AND content dimensions are known.
// `viewportPositionToImagePosition` divides by container size and image
// size, so it returns null pre-measurement or with no content size set —
// leave `contentX`/`contentY` undefined in that case rather than emit
// NaN/Infinity to consumers.
const touch = gestureEvent?.allTouches[0];

@thomasttvo thomasttvo May 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 we only need x and y from the touch here. Passing the whole event could lead to misinterpretation of the event when the first touch is not intended to be the source for the x and y.

@elliottkember elliottkember May 19, 2026 •
edited
Loading

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.

What do you mean by passing the whole event? We're only passing this touch to viewportPositionToImagePosition to calculate the content position?

@thomasttvo thomasttvo May 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry I meant passing the whole event into the getZoomableViewObject function

if (touch && contentWidth.value && contentHeight.value) {
const contentPos = viewportPositionToImagePosition({
viewportPosition: { x: touch.x, y: touch.y },
imageSize: {
width: contentWidth.value,
height: contentHeight.value,
},
zoomableEvent: event,
});
if (contentPos) {
event.contentX = contentPos.x;
event.contentY = contentPos.y;
}
}

return event;
};

const _staticPinPosition = () => {
Expand All @@ -381,11 +402,7 @@ const ReactNativeZoomableViewInner: ForwardRefRenderFunction<
height: contentHeight.value,
width: contentWidth.value,
},
zoomableEvent: _getZoomableViewEventObject({
offsetX: offsetX.value,
offsetY: offsetY.value,
zoomLevel: zoom.value,
}),
zoomableEvent: _getZoomableViewEventObject(),

@thomasttvo thomasttvo May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟣 _staticPinPosition can probably be replaced by _getZoomableViewEventObject(staticPinPosition.value) making the interface for static pin more standardized

@elliottkember elliottkember May 19, 2026 •
edited
Loading

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.

Not without changing the _getZoomableViewEventObject because it needs a GestureTouchEvent we don't have?

});
};

Expand Down Expand Up @@ -768,7 +785,7 @@ const ReactNativeZoomableViewInner: ForwardRefRenderFunction<
// `props.onLongPress` — the closure was captured at schedule time
// and would fire a stale callback if the parent re-rendered during
// the timer window.
onLongPress(e, _getZoomableViewEventObject());
onLongPress(e, _getZoomableViewEventObject(e));
}, props.longPressDuration);
}
});
Expand Down Expand Up @@ -828,7 +845,7 @@ const ReactNativeZoomableViewInner: ForwardRefRenderFunction<
// the resumed gesture.
runOnJS(setGestureStartedJS)(true);
if (!isRecovery) {
runOnJS(_safeOnPanResponderGrant)(e, _getZoomableViewEventObject());
runOnJS(_safeOnPanResponderGrant)(e, _getZoomableViewEventObject(e));
}

cancelAnimation(zoom);
Expand Down Expand Up @@ -1077,7 +1094,7 @@ const ReactNativeZoomableViewInner: ForwardRefRenderFunction<
const { onDoubleTapBefore, onDoubleTapAfter, doubleTapZoomToCenter } =
props;

onDoubleTapBefore?.(e, _getZoomableViewEventObject());
onDoubleTapBefore?.(e, _getZoomableViewEventObject(e));

const nextZoomStep = getNextZoomStep({
zoomLevel: zoom.value,
Expand All @@ -1104,10 +1121,10 @@ const ReactNativeZoomableViewInner: ForwardRefRenderFunction<

publicZoomTo(nextZoomStep, zoomPositionCoordinates);

onDoubleTapAfter?.(
e,
_getZoomableViewEventObject({ zoomLevel: nextZoomStep })
);
onDoubleTapAfter?.(e, {
..._getZoomableViewEventObject(e),
zoomLevel: nextZoomStep,
});
});

/**
Expand Down Expand Up @@ -1187,7 +1204,7 @@ const ReactNativeZoomableViewInner: ForwardRefRenderFunction<

// Invoke the stable `onSingleTap` wrapper rather than the captured
// `props.onSingleTap` — same staleness reasoning as `onLongPress`.
onSingleTap(e, _getZoomableViewEventObject());
onSingleTap(e, _getZoomableViewEventObject(e));
}, props.doubleTapDelay);
}
};
Expand Down Expand Up @@ -1358,12 +1375,12 @@ const ReactNativeZoomableViewInner: ForwardRefRenderFunction<

runOnJS(clearLongPressTimeout)();

runOnJS(_safeOnPanResponderEnd)(e, _getZoomableViewEventObject());
runOnJS(_safeOnPanResponderEnd)(e, _getZoomableViewEventObject(e));

if (gestureType.value === 'pinch') {
runOnJS(_safeOnZoomEnd)(e, _getZoomableViewEventObject());
runOnJS(_safeOnZoomEnd)(e, _getZoomableViewEventObject(e));
} else if (gestureType.value === 'shift') {
runOnJS(_safeOnShiftingEnd)(e, _getZoomableViewEventObject());
runOnJS(_safeOnShiftingEnd)(e, _getZoomableViewEventObject(e));
}

// RNGH cancellation: queue `onPanResponderTerminate` HERE — inside the
Expand All @@ -1372,7 +1389,7 @@ const ReactNativeZoomableViewInner: ForwardRefRenderFunction<
// `ref.current.gestureStarted` from inside `onPanResponderTerminate`
// observes `true`, matching SPECS L157 for the other end-callbacks.
if (isCancellation) {
runOnJS(_safeOnPanResponderTerminate)(e, _getZoomableViewEventObject());
runOnJS(_safeOnPanResponderTerminate)(e, _getZoomableViewEventObject(e));
// wasReleased=false skips the suppression branch above; clear here.
doubleTapFirstTapReleaseTimestamp.value = undefined;
doubleTapFirstTap.value = undefined;
Expand Down Expand Up @@ -1414,7 +1431,10 @@ const ReactNativeZoomableViewInner: ForwardRefRenderFunction<
'worklet';

if (
onPanResponderMoveWorkletShared.value.fn(e, _getZoomableViewEventObject())
onPanResponderMoveWorkletShared.value.fn(
e,
_getZoomableViewEventObject(e)
)
) {
// Consumer intercepted this move. The early-return below skips the
// gesture-classification branches that would otherwise assign
Expand Down
9 changes: 9 additions & 0 deletions src/typings/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ export interface ZoomableViewEvent {
offsetY: number;
originalHeight: number;
originalWidth: number;
/**
* Position of the gesture's first touch in content (bitmap) coordinates.
* Populated only when the callback has an associated gesture event AND
* both `contentWidth` and `contentHeight` props are set. Undefined for
* non-gesture call sites (`onTransformWorklet`, `onZoomEnd` after a
* programmatic `zoomTo()`) and when content dimensions are unknown.
*/
contentX?: number;
contentY?: number;

@thomasttvo thomasttvo May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟣 this might create confusion from the client in the form of: "is there a time where we have contentX but not contentY?" it's probably best to use TS to make them either both undefined or exists, or even better, group them in a property

@thomasttvo thomasttvo May 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe all this complexity is an indicator that ZoomableViewEvent is doing too much. Thinking about it - why would onSingleTap need to return originalHeight and originalWidth. This is probably a flawed legacy design we need to move away in v3.

@elliottkember elliottkember May 19, 2026

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.

I don't know, but I imagine it's so that the consumer can have all the info it needs packed into the event. Which I think is fairly reasonable if you already have an event object. It could definitely be better arranged, that's for sure. I don't really know why most of these attributes are exposed to the client

@elliottkember elliottkember May 19, 2026 •
edited
Loading

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.

Separate optional values is okay here, it's consistent with contentWidth? / contentHeight? which are both optional. They're all flat in this interface:

  initialOffsetX?: number;
  initialOffsetY?: number;
  contentWidth?: number;
  contentHeight?: number;

}

export type ReactNativeZoomableViewRef = {
Expand Down
Loading