-
Notifications
You must be signed in to change notification settings - Fork 600
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
Added zoom control #559
Added zoom control #559
Conversation
scarlac
commented
Jul 12, 2023
- Added zoom control
- Added max zoom control
- Added onZoom handler
![](https://private-user-images.githubusercontent.com/895369/252829857-ce35796a-b6f0-4940-bbe1-6083018462a0.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5OTcwMDksIm5iZiI6MTczODk5NjcwOSwicGF0aCI6Ii84OTUzNjkvMjUyODI5ODU3LWNlMzU3OTZhLWI2ZjAtNDk0MC1iYmUxLTYwODMwMTg0NjJhMC5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwMjA4JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDIwOFQwNjM4MjlaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT0yMWNkZWJlNThhNGVjNWY4MThiNWIwNTUwOWM5Y2RkMjk3ODE3YmUzMTVjYzJlMmY5ZTdkOTVhOGMyMmNmNDY5JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.rt3FdUSfvTuRfHRWwc_TbDcAYWybxIU4aa072FXyP5E)
@DavidBertet in case you're interested. |
Added max zoom control Added onZoom handler
Looking at it! |
(must explicitly be reset using zoom={0} if that is wanted) Fixed non-unique queue name
example/src/CameraExample.tsx
Outdated
@@ -54,6 +55,7 @@ const CameraExample = ({ onBack }: { onBack: () => void }) => { | |||
const onSwitchCameraPressed = () => { | |||
const direction = cameraType === CameraType.Back ? CameraType.Front : CameraType.Back; | |||
setCameraType(direction); | |||
setZoom(0); // When changing camera type, reset to default zoom for that camera |
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.
Should that be done by the native side / Camera component? Sounds like the expected behavior as a user of the framework. RealCamera
probably does it already in update(cameraType
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.
Since it uses the declarative API it must fully control the zoom factor. Thus, it must also manually reset the value.
The only alternative to these things are to use an imperative API for zooming which circumvents the issues. It all boils down to the fact that the gesture control and default zoom are only available in native
src/Camera.ios.tsx
Outdated
<NativeCamera maxZoom={props.maxZoom ?? 20} style={{ minWidth: 100, minHeight: 100 }} ref={nativeRef} {...props} /> | ||
); | ||
}); | ||
|
||
Camera.defaultProps = { |
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.
We can use defaultProps
bellow to set 20 by default. We might want it on Android as well
<NativeCamera maxZoom={props.maxZoom ?? 20} style={{ minWidth: 100, minHeight: 100 }} ref={nativeRef} {...props} /> | |
); | |
}); | |
Camera.defaultProps = { | |
<NativeCamera style={{ minWidth: 100, minHeight: 100 }} ref={nativeRef} {...props} /> | |
); | |
}); | |
Camera.defaultProps = { | |
maxZoom: 20, |
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.
Good catch. I actually wanted to remove that. maxZoom shouldn't be required in RN, as that means we can't tell if it's undefined (unrestricted) or 20 (restricted).
src/Camera.d.ts
Outdated
zoomMode?: ZoomMode; | ||
/** | ||
* **iOS only.** |
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.
not "iOS only" anymore
Fixed inline docs for props Refactored Swift/Kotlin code to look alike (e.g. gesture handling)
Added a customizable capture button in example
pinchGestureStartedAt = detector.currentSpan | ||
return true | ||
} | ||
override fun onScale(detector: ScaleGestureDetector?): Boolean { |
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.
I think this may have reverted a change made in https://github.com/teslamotors/react-native-camera-kit/pull/551/files?w=1
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.
Thanks @elliottkember - This was re-fixed and released in v14.0.0-beta13.
teslamotors#559 broke a change in teslamotors#551 regarding override types. This restores the fix