Add an onTapToFocus event - #814
ZayanKhan-12 wants to merge 1 commit into
Conversation
Issue teslamotors#590 asks for a way to react to a tap on the camera area without losing the camera's own gestures. A sibling overlay with an onPress swallows the touch, which kills tap-to-focus and pinch-to-zoom, and the view does not accept children, so there was no way to have both. Report the tap instead. The gesture that already drives tap-to-focus now also emits onTapToFocus with the tap position, so an overlay can react to it while focus and zoom keep working. Nothing about the existing gesture handling changes, and the event costs nothing when the prop is not set. Coordinates are normalized to 0..1 of the preview rather than raw points, so they do not depend on device pixel dimensions. That matches the bounds onFaceDetected already reports. Not called onFocus, as the issue suggested, because CameraProps extends ViewProps and React Native already declares onFocus and onBlur there for accessibility focus; reusing the name would shadow it. The plumbing mirrors onZoom exactly: the codegen spec, the public prop, an RCTDirectEventBlock exported for the old architecture and forwarded to the generated event emitter for the new one, and on Android an Event subclass registered in both the newarch and oldarch view managers. FocusInterfaceView normalizes on iOS since it owns that coordinate space, which also keeps CameraView.swift from growing past the linter's file-length limit. Verified with `yarn build` (tsc), and by running codegen and checking the generated struct is `OnTapToFocus { double x; double y; }`, which is what the new-arch bridging code constructs. SwiftLint over the changed Swift files reports the same six pre-existing violations as master and no new ones. Fixes teslamotors#590 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MjiwHkBNkrUKxsQUVeeaSH
Add an onTapToFocus event. Open upstream as teslamotors#814.
|
I tested 36be2b6 and found two Android callback problems: lifting the last finger after a pinch emits onTapToFocus, and queued discrete taps can be combined into one callback. I opened a four file correction against your branch. It suppresses the new callback after multiple pointer input, keeps tap events from coalescing and clarifies the iOS focusMode requirement. Existing native focus and zoom handling retain their behavior. Your PR fails three subscribed pinch assertions in each Android renderer. Three controlled Paper trials each accept four native window taps but produce one callback. The correction passes both controls. Final Paper and Fabric each pass 44 checks covering gestures, cancellation, callback removal/restoration, remounts, queued delivery and ten Home returns. The full iOS Paper suite passes three methods; I also confirmed focus, zoom and callback behavior on my iPhone 15 Pro Max. Source fixtures, exact commits, commands, results and screenshots are available for reproduction. Android builds and TypeScript pass; iOS controls build with CLT clang 17. Default clang 21, Jest and explicit ESLint have matching baseline setup failures. The report retains earlier failed attempts and distinguishes the bundled queue tests from incomplete debug stress. Physical Android optics and hosted execution remain unverified; upstream workflows await approval with zero jobs. Cody |
The problem
#590 wants to react to a tap on the camera area without giving up the camera's own gestures. A sibling overlay with an
onPressswallows the touch, which kills tap-to-focus and pinch-to-zoom, and<Camera>doesn't accept children — so there was no way to have both.The approach
Rather than opening the view up to children, this reports the tap. The gesture that already drives tap-to-focus now also emits
onTapToFocuswith the tap position, so an overlay can react to it while focus and zoom keep working exactly as before.Three decisions worth surfacing:
onFocus, as the issue suggested.CameraProps extends ViewProps, and React Native declaresonFocus/onBlurthere for accessibility focus (ViewPropTypes.js), so reusing the name would shadow it.onFaceDetectedalready documents.focusMode="on"(the default), since that's when the tap gesture is attached — documented on the prop.The plumbing mirrors
onZoomexactly: codegen spec → public prop → anRCTDirectEventBlockexported for the old architecture and forwarded to the generated event emitter for the new one; on Android anEventsubclass registered in both thenewarchandoldarchview managers.FocusInterfaceViewdoes the normalization on iOS since it owns that coordinate space, which also keepsCameraView.swiftfrom growing further past SwiftLint'sfile_lengthlimit.Testing
yarn build(tsc) — clean.react-native codegen --platform ios --source libraryand checked the generated header:struct OnTapToFocus { double x; double y; };, which is exactly what the new-arch designated initializer inCKCameraViewComponentView.mmconstructs.yarn && cd example/android && ./gradlew assembleDebug→ BUILD SUCCESSFUL, 91 tasks,app-debug.apkproduced.TapToFocusEvent.class,CKCamera.classandCKCameraManager.classall compiled (the example hasnewArchEnabled=true, so this exercises thenewarchmanager — same as CI).masterand no new ones. It needs SourceKit, so with only the Command Line Tools installed it has to be pointed at the CLT copy:DYLD_FRAMEWORK_PATH=/Library/Developer/CommandLineTools/usr/lib swiftlint -- <files>.What I could not verify locally: the iOS example build, which needs a full Xcode install rather than the Command Line Tools. The iOS code is the part I'd look hardest at in review, particularly the new-arch block in
CKCameraViewComponentView.mm.One unrelated thing I noticed while setting up:
yarn lintiseslint -c .eslintrc.jswith no file arguments, so it lints nothing and exits 0 — the ESLint CI job passes regardless of what's insrc/. Andyarn testis declared butjestisn't a dependency. Happy to open a separate PR for either if useful; I've left both alone here.Fixes #590
🤖 Generated with Claude Code
https://claude.ai/code/session_01MjiwHkBNkrUKxsQUVeeaSH