feat(feedback): Show feedback widget on device shake#5754
Merged
Conversation
Implement device shake detection to trigger the feedback widget.
No permissions are required on either platform:
- iOS: Uses UIKit's motionEnded:withEvent: via UIWindow swizzle
- Android: Uses SensorManager accelerometer (TYPE_ACCELEROMETER)
Public API:
- showFeedbackOnShake() / hideFeedbackOnShake() imperative APIs
- feedbackIntegration({ enableShakeToReport: true }) declarative option
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tion.sendEvent: The previous implementation swizzled UIWindow.motionEnded:withEvent: which was intercepted by React Native's dev menu before our handler could fire. Switching to UIApplication.sendEvent: intercepts events before the responder chain, so shake events are detected even when RN dev menu or another responder consumes the motion event without calling super. Added a 1-second cooldown to prevent double-firing since both motionBegan and motionEnded trigger UIEventSubtypeMotionShake.
…wizzle UIApplication.sendEvent: is not invoked by the iOS simulator for the simulated shake (Cmd+Ctrl+Z); it goes directly through UIWindow.motionEnded:withEvent: instead. React Native's dev menu swizzles UIWindow.motionEnded:withEvent: at bridge load time. Because we swizzle from startObserving (triggered by componentDidMount via NativeEventEmitter.addListener), our swizzle always runs after RN's — making sentry_motionEnded the outermost layer that calls through to RN's dev-menu handler via the stored original IMP. This approach works on both real devices and the iOS simulator.
Without FeedbackWidgetProvider rendered in the tree, componentDidMount never fires, startShakeListener is never called, and the native swizzle is never set up — so shake-to-report has no effect despite enableShakeToReport: true being configured on the integration.
…le app" This reverts commit 6054e19.
Instead of relying on startObserving (which fires for any event type on the module's first listener), mirror the Android approach: override addListener and explicitly call [RNSentryShakeDetector enable] when the shake event is subscribed to. This ensures the UIWindow swizzle is set up reliably regardless of listener ordering or TurboModule event-emitter behaviour.
Without this import SentryDefines.h is never included, SENTRY_HAS_UIKIT evaluates to 0, and the entire shake detector implementation is compiled out leaving only the no-op stubs. All other files in the module that use SENTRY_HAS_UIKIT (RNSentryOnDrawReporter.m, RNSentryDependencyContainer.m, etc.) include @import Sentry for exactly this reason.
@import Sentry caused a startup crash. Replace both the module import and SENTRY_HAS_UIKIT guard with TARGET_OS_IOS which has identical semantics for shake detection (iOS only) and needs no external import.
…eport On iOS with New Architecture (TurboModules), NativeEventEmitter.addListener does not dispatch to native addListener:, so the UIWindow swizzle for shake detection was never enabled. Adds explicit enableShakeDetection/disableShakeDetection RCT_EXPORT_METHODs on iOS and no-op stubs on Android. JS startShakeListener now calls enableShakeDetection directly after subscribing to the event, bypassing the unreliable NativeEventEmitter → native dispatch path on iOS. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
UIWindow inherits motionEnded:withEvent: from UIResponder and may not have its own implementation. Using method_setImplementation directly on the inherited Method would modify UIResponder, affecting all subclasses and causing a doesNotRecognizeSelector crash. Fix by calling class_addMethod first to ensure UIWindow has its own method before replacing the IMP. Also prevent duplicate NSNotification observers on component remount, and clean up debug logging. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… native SDKs Remove the RN-specific shake detector implementations and delegate to SentryShakeDetector (iOS: sentry-cocoa, Android: sentry-android-core) so the implementation is shared with other SDKs that have feedback UI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
This was referenced Mar 4, 2026
7 tasks
antonis
commented
Mar 4, 2026
Contributor
Author
antonis
left a comment
antonis
left a comment
There was a problem hiding this comment.
Marking as blocked till the native PRs are merged
antonis
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Replace `hasListeners` guard in `handleShakeDetected` with a dedicated `_shakeDetectionEnabled` ivar. `hasListeners` is managed by `startObserving`/`stopObserving`, which may not be called on New Architecture (TurboModules) when `NativeEventEmitter.addListener` is used. Using a dedicated flag ensures shake events are emitted whenever shake detection is explicitly enabled from JS, regardless of the `NativeEventEmitter` lifecycle. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fecycle - Wrap native bridge calls in `startShakeListener` with try/catch so exceptions from the native module never crash the host app. - Track whether `FeedbackWidgetProvider` started shake detection and only stop it on unmount if this component was the one that started it, preventing imperatively-started shake listeners from being silently killed on remount. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
If addListener succeeds but enableShakeDetection throws, the catch block now removes the subscription before nulling the reference, preventing a listener leak. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java
Show resolved
Hide resolved
…rashes Consistent with the existing pattern in the file: native operations that could throw are wrapped in try/catch(Throwable) with a warning log so SDK instrumentation errors never crash the host app. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…listener startShakeListener now returns boolean indicating whether a new subscription was created (false if one was already active). FeedbackWidgetProvider uses the return value to set _startedShakeListener, so it only stops the listener on unmount if it was the one that started it — preventing it from killing an imperatively-started listener. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java
Show resolved
Hide resolved
The lambda passed to shakeDetector.start() runs on the sensor thread at shake-time, outside the try/catch that protects the setup. Wrapping it ensures exceptions from getReactApplicationContext(), getJSModule(), or emit() never crash the sensor thread or host app. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
enableFeedbackOnShake now records whether it actually started the listener (using the boolean returned by startShakeListener). disableFeedbackOnShake only stops the listener if it was the one that started it, preventing it from interfering with a listener owned by FeedbackWidgetProvider. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
…fecycle - Wrap stopShakeListener native calls in try/catch so exceptions from subscription.remove() or disableShakeDetection() never crash the app. - Guard enableFeedbackOnShake so repeated calls don't overwrite _imperativeShakeListenerStarted with false, which would make disableFeedbackOnShake a permanent no-op. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
|
Looks solid! lets add the ready-to-merge label before merging. |
Contributor
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 4a17c8f+dirty | 406.62 ms | 400.58 ms | -6.04 ms |
| df1f7df+dirty | 442.64 ms | 427.16 ms | -15.48 ms |
| a483f9f+dirty | 396.82 ms | 453.28 ms | 56.46 ms |
| 60cd796+dirty | 445.84 ms | 492.45 ms | 46.61 ms |
| 5c16cdc+dirty | 423.48 ms | 452.35 ms | 28.88 ms |
| 80e4616+dirty | 411.58 ms | 462.12 ms | 50.54 ms |
| 55b77fc+dirty | 411.87 ms | 417.16 ms | 5.29 ms |
| bca62c0+dirty | 414.36 ms | 451.06 ms | 36.70 ms |
| 0b64753+dirty | 448.67 ms | 474.61 ms | 25.94 ms |
| 4e6d7d7+dirty | 480.73 ms | 515.73 ms | 35.00 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 4a17c8f+dirty | 43.75 MiB | 47.99 MiB | 4.24 MiB |
| df1f7df+dirty | 43.75 MiB | 48.08 MiB | 4.33 MiB |
| a483f9f+dirty | 43.75 MiB | 48.41 MiB | 4.66 MiB |
| 60cd796+dirty | 43.75 MiB | 48.07 MiB | 4.32 MiB |
| 5c16cdc+dirty | 17.75 MiB | 19.68 MiB | 1.94 MiB |
| 80e4616+dirty | 43.75 MiB | 48.55 MiB | 4.80 MiB |
| 55b77fc+dirty | 43.75 MiB | 47.99 MiB | 4.24 MiB |
| bca62c0+dirty | 43.75 MiB | 48.41 MiB | 4.66 MiB |
| 0b64753+dirty | 17.75 MiB | 19.70 MiB | 1.95 MiB |
| 4e6d7d7+dirty | 43.75 MiB | 48.40 MiB | 4.64 MiB |
Contributor
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ea3e26e+dirty | 1229.13 ms | 1228.46 ms | -0.67 ms |
| 80e4616+dirty | 1221.32 ms | 1225.64 ms | 4.32 ms |
| 818a608+dirty | 1205.76 ms | 1208.00 ms | 2.24 ms |
| 77061ed+dirty | 1233.16 ms | 1234.88 ms | 1.71 ms |
| bef3709+dirty | 1222.07 ms | 1220.24 ms | -1.83 ms |
| a206511+dirty | 1185.00 ms | 1186.35 ms | 1.35 ms |
| 74979ac+dirty | 1210.49 ms | 1213.31 ms | 2.82 ms |
| a2bb688+dirty | 1223.53 ms | 1232.90 ms | 9.37 ms |
| 8a868fe+dirty | 1221.50 ms | 1230.78 ms | 9.28 ms |
| d590428+dirty | 1211.77 ms | 1220.51 ms | 8.75 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ea3e26e+dirty | 3.41 MiB | 4.58 MiB | 1.17 MiB |
| 80e4616+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| 818a608+dirty | 2.63 MiB | 3.91 MiB | 1.28 MiB |
| 77061ed+dirty | 2.63 MiB | 3.98 MiB | 1.34 MiB |
| bef3709+dirty | 3.38 MiB | 4.78 MiB | 1.40 MiB |
| a206511+dirty | 3.41 MiB | 4.67 MiB | 1.25 MiB |
| 74979ac+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| a2bb688+dirty | 2.63 MiB | 3.99 MiB | 1.36 MiB |
| 8a868fe+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| d590428+dirty | 3.38 MiB | 4.78 MiB | 1.39 MiB |
Contributor
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ea3e26e+dirty | 1216.61 ms | 1214.15 ms | -2.47 ms |
| 80e4616+dirty | 1206.90 ms | 1205.94 ms | -0.96 ms |
| 818a608+dirty | 1218.84 ms | 1223.18 ms | 4.34 ms |
| 77061ed+dirty | 1210.77 ms | 1218.45 ms | 7.68 ms |
| bef3709+dirty | 1217.79 ms | 1225.33 ms | 7.54 ms |
| a206511+dirty | 1225.02 ms | 1223.74 ms | -1.28 ms |
| 74979ac+dirty | 1212.33 ms | 1212.54 ms | 0.21 ms |
| a2bb688+dirty | 1244.82 ms | 1238.60 ms | -6.22 ms |
| 8a868fe+dirty | 1206.85 ms | 1215.04 ms | 8.19 ms |
| d590428+dirty | 1221.23 ms | 1225.27 ms | 4.03 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ea3e26e+dirty | 3.41 MiB | 4.58 MiB | 1.17 MiB |
| 80e4616+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| 818a608+dirty | 3.19 MiB | 4.48 MiB | 1.29 MiB |
| 77061ed+dirty | 3.19 MiB | 4.54 MiB | 1.36 MiB |
| bef3709+dirty | 3.38 MiB | 4.78 MiB | 1.40 MiB |
| a206511+dirty | 3.41 MiB | 4.67 MiB | 1.25 MiB |
| 74979ac+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| a2bb688+dirty | 3.19 MiB | 4.56 MiB | 1.37 MiB |
| 8a868fe+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| d590428+dirty | 3.38 MiB | 4.78 MiB | 1.39 MiB |
Contributor
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 70250df+dirty | 418.08 ms | 480.84 ms | 62.76 ms |
| 8d89cc9+dirty | 357.69 ms | 415.79 ms | 58.10 ms |
| 1853710+dirty | 360.67 ms | 396.28 ms | 35.61 ms |
| 55b77fc+dirty | 410.46 ms | 414.11 ms | 3.65 ms |
| 69602ce+dirty | 375.37 ms | 405.28 ms | 29.91 ms |
| c1573b3+dirty | 355.65 ms | 448.82 ms | 93.17 ms |
| 90afdd3+dirty | 367.79 ms | 404.84 ms | 37.05 ms |
| 955f2eb+dirty | 388.13 ms | 433.56 ms | 45.44 ms |
| 80e4616+dirty | 427.31 ms | 461.15 ms | 33.84 ms |
| 276d348+dirty | 356.30 ms | 405.27 ms | 48.97 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 70250df+dirty | 43.94 MiB | 48.91 MiB | 4.97 MiB |
| 8d89cc9+dirty | 7.15 MiB | 8.41 MiB | 1.26 MiB |
| 1853710+dirty | 7.15 MiB | 8.41 MiB | 1.26 MiB |
| 55b77fc+dirty | 43.94 MiB | 48.82 MiB | 4.88 MiB |
| 69602ce+dirty | 7.15 MiB | 8.41 MiB | 1.26 MiB |
| c1573b3+dirty | 7.15 MiB | 8.42 MiB | 1.27 MiB |
| 90afdd3+dirty | 7.15 MiB | 8.43 MiB | 1.28 MiB |
| 955f2eb+dirty | 7.15 MiB | 8.42 MiB | 1.27 MiB |
| 80e4616+dirty | 43.94 MiB | 49.38 MiB | 5.44 MiB |
| 276d348+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
Contributor
Author
|
Thank you @lucas-zimerman 🙇 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📢 Type of change
📜 Description
Implements shake-to-report for the feedback widget and delegates shake detection to the implementations in the native SDKs:
SentryShakeDetectorfrom sentry-cocoa (feat(feedback): implement shake gesture detection sentry-cocoa#7579)SentryShakeDetectorfrom sentry-android-core (feat(feedback): implement shake gesture detection sentry-java#5150)The
RNSentry.mmnow imports<Sentry/SentryShakeDetector.h>and usesSentryShakeDetectedNotificationinstead of the RN-specific ones. On Android,RNSentryModuleImplusesio.sentry.android.core.SentryShakeDetector.💡 Motivation and Context
Users have requested a way to trigger the feedback widget by shaking the device, without the need for a visible button. This is a common UX pattern for in-app feedback
Fixes #4735
This PR depends on the two native SDK PRs above merging first.
💚 How did you test it?
yarn build,yarn test,yarn lint,yarn circularDepCheck📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
Merge the native SDK PRs, update the Cocoa/Java dependency versions in the RN SDK, then merge this PR.