Skip to content

Add an onTapToFocus event - #814

Open
ZayanKhan-12 wants to merge 1 commit into
teslamotors:masterfrom
ZayanKhan-12:feat/590-on-tap-to-focus
Open

ZayanKhan-12 wants to merge 1 commit into
teslamotors:masterfrom
ZayanKhan-12:feat/590-on-tap-to-focus

Conversation

@ZayanKhan-12

Copy link
Copy Markdown

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 onPress swallows 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 onTapToFocus with the tap position, so an overlay can react to it while focus and zoom keep working exactly as before.

<Camera
  onTapToFocus={(e) => {
    console.log('tapped at', e.nativeEvent.x, e.nativeEvent.y);
  }}
/>

Three decisions worth surfacing:

  • Not called onFocus, as the issue suggested. CameraProps extends ViewProps, and React Native declares onFocus/onBlur there for accessibility focus (ViewPropTypes.js), so reusing the name would shadow it.
  • Coordinates are normalized 0–1 of the preview, not raw points, so payloads don't depend on device pixel dimensions. That matches the bounds onFaceDetected already documents.
  • No new gesture recognizers. The event rides the existing tap handler, so nothing changes for anyone not passing the prop, and there's no new way for gestures to conflict. On iOS that does mean it requires focusMode="on" (the default), since that's when the tap gesture is attached — documented on the prop.

The plumbing mirrors onZoom exactly: codegen spec → public prop → an RCTDirectEventBlock exported for the old architecture and forwarded to the generated event emitter for the new one; on Android an Event subclass registered in both the newarch and oldarch view managers. FocusInterfaceView does the normalization on iOS since it owns that coordinate space, which also keeps CameraView.swift from growing further past SwiftLint's file_length limit.

Testing

  • yarn build (tsc) — clean.
  • Codegen verified rather than assumed. Ran react-native codegen --platform ios --source library and checked the generated header: struct OnTapToFocus { double x; double y; };, which is exactly what the new-arch designated initializer in CKCameraViewComponentView.mm constructs.
  • Android example built as CI does — yarn && cd example/android && ./gradlew assembleDebug → BUILD SUCCESSFUL, 91 tasks, app-debug.apk produced. TapToFocusEvent.class, CKCamera.class and CKCameraManager.class all compiled (the example has newArchEnabled=true, so this exercises the newarch manager — same as CI).
  • SwiftLint over the changed Swift files reports the same six pre-existing violations as master and 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 lint is eslint -c .eslintrc.js with no file arguments, so it lints nothing and exits 0 — the ESLint CI job passes regardless of what's in src/. And yarn test is declared but jest isn'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

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
@ZayanKhan-12 ZayanKhan-12 mentioned this pull request Sep 15, 2026
ZayanKhan-12 pushed a commit to ZayanKhan-12/react-native-camera-kit that referenced this pull request Sep 15, 2026
Add an onTapToFocus event. Open upstream as
teslamotors#814.
@SproutSeeds

Copy link
Copy Markdown

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

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.

onFocus event OR allow child of camera

3 participants