-
Notifications
You must be signed in to change notification settings - Fork 63
Reanimated #151
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.
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
base: thomas/no-use-before-define
Are you sure you want to change the base?
Reanimated #151
Changes from all commits
fa598a6
d1c21ee
9945d2b
7c76154
fac493f
8604c3c
6c2adfe
b7b10af
4427fcb
ebcb7b8
77c9c80
d4fe8c0
490ae19
ff5b6c3
8809509
3601884
fbab873
44f0a85
eea8cb1
1019845
341414d
f893a93
71fdaf9
efdd4aa
7846d04
74070c3
e37a705
8c6b27f
86f67ca
4a2d93f
a327775
591db01
d23d301
5193634
4545946
f61f498
678b1c2
1540cb2
bcb7bba
468bd2d
29e1e33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,20 @@ | ||
| import { ReactNativeZoomableView } from '@openspacelabs/react-native-zoomable-view'; | ||
| import { | ||
| FixedSize, | ||
| ReactNativeZoomableView, | ||
| ReactNativeZoomableViewRef, | ||
| } from '@openspacelabs/react-native-zoomable-view'; | ||
| import { debounce } from 'lodash'; | ||
| import React, { useCallback, useRef, useState } from 'react'; | ||
| import { Animated, Button, Image, Text, View } from 'react-native'; | ||
| import React, { ReactNode, useCallback, useRef, useState } from 'react'; | ||
| import { | ||
| Alert, | ||
| Button, | ||
| Image, | ||
| Modal, | ||
| Text, | ||
| View, | ||
| ViewProps, | ||
| } from 'react-native'; | ||
| import { scheduleOnRN } from 'react-native-worklets'; | ||
|
|
||
| import { applyContainResizeMode } from '../src/helper/coordinateConversion'; | ||
| import { styles } from './style'; | ||
|
|
@@ -13,10 +26,24 @@ | |
| const stringifyPoint = (point?: { x: number; y: number }) => | ||
| point ? `${Math.round(point.x)}, ${Math.round(point.y)}` : 'Off map'; | ||
|
|
||
| const PageSheetModal = ({ | ||
| children, | ||
| style, | ||
| }: { | ||
| children: ReactNode; | ||
| style?: ViewProps['style']; | ||
| }) => { | ||
| return ( | ||
| <Modal animationType="slide" presentationStyle="pageSheet"> | ||
| <View style={style}>{children}</View> | ||
| </Modal> | ||
| ); | ||
| }; | ||
|
|
||
| export default function App() { | ||
| const zoomAnimatedValue = useRef(new Animated.Value(1)).current; | ||
| const scale = Animated.divide(1, zoomAnimatedValue); | ||
| const ref = useRef<ReactNativeZoomableViewRef>(null); | ||
| const [showMarkers, setShowMarkers] = useState(true); | ||
| const [modal, setModal] = useState(false); | ||
| const [size, setSize] = useState<{ width: number; height: number }>({ | ||
| width: 0, | ||
| height: 0, | ||
|
|
@@ -36,8 +63,10 @@ | |
| const staticPinPosition = { x: size.width / 2, y: size.height / 2 }; | ||
| const { size: contentSize } = applyContainResizeMode(imageSize, size); | ||
|
|
||
| const Wrapper = modal ? PageSheetModal : View; | ||
|
|
||
| return ( | ||
| <View style={styles.container}> | ||
| <Wrapper style={styles.container}> | ||
| <Text>ReactNativeZoomableView</Text> | ||
| <View | ||
| style={styles.box} | ||
|
|
@@ -46,35 +75,36 @@ | |
| }} | ||
| > | ||
| <ReactNativeZoomableView | ||
| ref={ref} | ||
| debug | ||
| onLongPress={() => { | ||
| Alert.alert('Long press detected'); | ||
| }} | ||
| // Where to put the pin in the content view | ||
| staticPinPosition={staticPinPosition} | ||
| // Callback that returns the position of the pin | ||
| // on the actual source image | ||
| onStaticPinPositionChange={debouncedUpdatePin} | ||
| onStaticPinPositionMove={debouncedUpdateMovePin} | ||
| onStaticPinPositionMove={(position) => { | ||
| 'worklet'; | ||
| scheduleOnRN(debouncedUpdateMovePin, position); | ||
| }} | ||
| maxZoom={30} | ||
| // Give these to the zoomable view so it can apply the boundaries around the actual content. | ||
| // Need to make sure the content is actually centered and the width and height are | ||
| // measured when it's rendered naturally. Not the intrinsic sizes. | ||
| contentWidth={contentSize?.width ?? 0} | ||
| contentHeight={contentSize?.height ?? 0} | ||
| zoomAnimatedValue={zoomAnimatedValue} | ||
| > | ||
| <View style={styles.contents}> | ||
| <Image style={styles.img} source={{ uri }} /> | ||
|
|
||
| {showMarkers && | ||
| (['20%', '40%', '60%', '80%'] as const).map((left) => | ||
| (['20%', '40%', '60%', '80%'] as const).map((top) => ( | ||
| <Animated.View | ||
| key={`${left}x${top}`} | ||
| // These markers will move and zoom with the image, but will retain their size | ||
| // because of the scale transformation. | ||
| style={[ | ||
| styles.marker, | ||
| { left, top, transform: [{ scale }] }, | ||
| ]} | ||
| /> | ||
| [20, 40, 60, 80].map((left) => | ||
| [20, 40, 60, 80].map((top) => ( | ||
| <FixedSize left={left} top={top} key={`${left}x${top}`}> | ||
| <View style={styles.marker} /> | ||
| </FixedSize> | ||
| )) | ||
| )} | ||
| </View> | ||
|
|
@@ -88,6 +118,15 @@ | |
| setShowMarkers((value) => !value); | ||
| }} | ||
| /> | ||
| </View> | ||
|
|
||
| <Button | ||
| // Toggle modal to test if zoomable view works correctly in modal, | ||
| // where pull-down-to-close gesture can interfere with pan gestures. | ||
| title={`Toggle Modal Mode`} | ||
| onPress={() => { | ||
| setModal((value) => !value); | ||
| }} | ||
| /> | ||
| </Wrapper> | ||
|
Comment on lines
+122
to
+130
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clever! I think this should exhibit the same behaviour as the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ module.exports = function (api) { | |
| }, | ||
| }, | ||
| ], | ||
| 'react-native-reanimated/plugin', | ||
| ], | ||
| }; | ||
| }; | ||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 The README Pan Responder Hooks table documents the prop as
onPanResponderMoveWorklet(line 273), but the actual prop defined insrc/typings/index.tsisonPanResponderMove. Any developer following the README who passesonPanResponderMoveWorklet={myWorklet}will have the callback silently ignored — movement interception will be completely non-functional with no TypeScript error to signal the mistake.Extended reasoning...
What the bug is and how it manifests
The README Pan Responder Hooks table (README.md, line 273) documents the move interception callback as
onPanResponderMoveWorklet. However, looking atsrc/typings/index.ts, the prop is declared asonPanResponderMove(typed asWorklet<(event: GestureTouchEvent, zoomableViewEventObject: ZoomableViewEvent) => boolean>). TheWorklet<T>alias is defined astype Worklet<T extends (...args: any[]) => any> = T, meaning it is structurally identical toT— it adds no nominal distinction that TypeScript can use to flag the mismatch.The specific code path that triggers it
A developer reads the README table, sees
onPanResponderMoveWorklet, and writes:Internally,
_handlePanResponderMovechecksonPanResponderMove?.(e, _getZoomableViewEventObject()). Since the prop that was actually passed isonPanResponderMoveWorklet(not a recognized key inReactNativeZoomableViewProps), the callback is never invoked.Why existing code doesn't prevent it
TypeScript's excess property checking only triggers in direct object literals passed inline. In React JSX, props are passed as an object literal, so TypeScript would flag an unknown prop... unless the component's prop type has an index signature, or the user passes via spread. More importantly,
Worklet<T> = Tmeans even if someone mistakenly thoughtonPanResponderMoveWorkletwas a valid prop name, there is no nominal type difference to catch it. In practice, the JSX compiler will simply drop the unrecognized prop into the component, where it is never read.What the impact would be
Any developer who uses the README as their primary reference for the Pan Responder Hooks API (its intended purpose) and tries to intercept move events will find that their callback is never called. Pinch and shift operations cannot be intercepted. There is no runtime error, no warning, and no TypeScript error in typical usage — the feature is just silently absent.
How to fix it
Change line 273 of README.md from
onPanResponderMoveWorklettoonPanResponderMoveto match the actual prop name declared insrc/typings/index.ts.Step-by-step proof
onPanResponderMoveWorklet.onPanResponderMoveWorklet={(e, eventObj) => { 'worklet'; return true; }}to their<ReactNativeZoomableView>.Worklet<T>=Tprovides no type guard)._handlePanResponderMoveevaluatesonPanResponderMove?.(e, ...)— this isundefinedbecause the prop was passed under the wrong name.