Skip to content

feat(feedback): implement shake gesture detection#5150

Open
antonis wants to merge 8 commits intomainfrom
antonis/feedback-shake
Open

feat(feedback): implement shake gesture detection#5150
antonis wants to merge 8 commits intomainfrom
antonis/feedback-shake

Conversation

@antonis
Copy link
Contributor

@antonis antonis commented Mar 3, 2026

📜 Description

Implements shake gesture detection for the user feedback form. When useShakeGesture is enabled in SentryFeedbackOptions, shaking the device opens the feedback form.

The implementation uses the device's accelerometer (via SensorManager) with a 2.7G threshold and 1-second cooldown. A new ShakeDetectionIntegration registers lifecycle callbacks to start/stop the detector when the activity resumes/pauses.

Note: this is exposed to be used on React Native too getsentry/sentry-react-native#5754

💡 Motivation and Context

The useShakeGesture property already existed in SentryFeedbackOptions but was never wired up. This PR implements the feature. The implementation is placed here (sentry-java) rather than in each SDK separately so it can be reused by all SDKs that embed the feedback UI (React Native, Flutter, .NET MAUI, Unity).

No special permissions are required — TYPE_ACCELEROMETER is not a protected sensor. Only BODY_SENSORS requires a permission.

💚 How did you test it?

  • Tested in a sample Android app — shaking the device opens the feedback form
  • Unit tests added for SentryShakeDetector and ShakeDetectionIntegration

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

sentry-react-native will remove its own RNSentryShakeDetector and delegate to this class instead.

Adds SentryShakeDetector (accelerometer-based) and ShakeDetectionIntegration
that shows the feedback dialog when a shake is detected. Controlled by
SentryFeedbackOptions.useShakeGesture (default false).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Semver Impact of This PR

🟡 Minor (new features)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


New Features ✨

  • (feedback) Implement shake gesture detection by antonis in #5150

🤖 This preview updates automatically when you update the PR.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 0b090bc

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 314.38 ms 370.51 ms 56.13 ms
Size 1.58 MiB 2.29 MiB 727.37 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
91bb874 314.47 ms 440.00 ms 125.53 ms
dba088c 333.98 ms 381.16 ms 47.18 ms
d15471f 310.66 ms 368.19 ms 57.53 ms
bbc35bb 298.53 ms 372.17 ms 73.64 ms
d15471f 307.28 ms 381.85 ms 74.57 ms
ad8da22 362.98 ms 453.94 ms 90.96 ms
cf708bd 408.35 ms 458.98 ms 50.63 ms
96449e8 361.30 ms 423.39 ms 62.09 ms
27d7cf8 309.43 ms 364.27 ms 54.85 ms
91bb874 311.00 ms 363.47 ms 52.47 ms

App size

Revision Plain With Sentry Diff
91bb874 1.58 MiB 2.13 MiB 559.07 KiB
dba088c 1.58 MiB 2.13 MiB 558.99 KiB
d15471f 1.58 MiB 2.13 MiB 559.54 KiB
bbc35bb 1.58 MiB 2.12 MiB 553.01 KiB
d15471f 1.58 MiB 2.13 MiB 559.54 KiB
ad8da22 1.58 MiB 2.29 MiB 719.83 KiB
cf708bd 1.58 MiB 2.11 MiB 539.71 KiB
96449e8 1.58 MiB 2.11 MiB 539.35 KiB
27d7cf8 1.58 MiB 2.12 MiB 549.42 KiB
91bb874 1.58 MiB 2.13 MiB 559.07 KiB

Previous results on branch: antonis/feedback-shake

Startup times

Revision Plain With Sentry Diff
2052d5f 331.52 ms 401.71 ms 70.19 ms
a1a1e57 323.92 ms 367.22 ms 43.31 ms
ba57364 317.45 ms 360.35 ms 42.90 ms

App size

Revision Plain With Sentry Diff
2052d5f 1.58 MiB 2.29 MiB 726.85 KiB
a1a1e57 1.58 MiB 2.29 MiB 727.28 KiB
ba57364 1.58 MiB 2.29 MiB 727.15 KiB

antonis and others added 2 commits March 4, 2026 13:08
- Add volatile/AtomicLong for thread-safe cross-thread field access
- Use SystemClock.elapsedRealtime() instead of System.currentTimeMillis()
- Use SENSOR_DELAY_NORMAL for better battery efficiency
- Add multi-shake counting (2+ threshold crossings within 1.5s window)
- Handle deferred init for already-resumed activities
- Wrap showDialog() in try-catch to prevent app crashes
- Improve activity transition handling in onActivityPaused
- Mark SentryShakeDetector as @ApiStatus.Internal
- Add unit tests for SentryShakeDetector and ShakeDetectionIntegration

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@antonis antonis marked this pull request as ready for review March 4, 2026 13:12
@antonis antonis requested a review from alwx March 4, 2026 13:14
… shakes

Track dialog visibility with an isDialogShowing flag that is set before
showing and cleared via the onFormClose callback when the dialog is
dismissed. Double-checked on both sensor and UI threads to avoid races.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… growth

Save the user's original onFormClose once during register() and restore
it after each dialog dismiss, instead of wrapping it with a new lambda
each time.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ck flag

If showDialog silently fails (e.g. activity destroyed between post and
execution), isDialogShowing would stay true forever, permanently
disabling shake-to-feedback. Reset it in onActivityPaused since the
dialog cannot outlive its host activity.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ActivityDestroyed

AlertDialog survives pause/resume cycles (e.g. screen off/on), so
resetting isDialogShowing in onActivityPaused allowed duplicate dialogs.
Move the reset to onActivityDestroyed where the dialog truly cannot
survive.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

…back on error

- Only reset isDialogShowing in onActivityDestroyed when it's the
  activity that hosts the dialog, not any unrelated activity.
- Restore originalOnFormClose in the catch block when showDialog throws.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
import org.jetbrains.annotations.Nullable;

/**
* Detects shake gestures and shows the user feedback dialog when a shake is detected. Only active
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the end goal is to add this to improve user feedback, but the way it is, it's doing nothing related to user feedback.
Might be a good idea to rename this to FeedbackShakeIntegration so the name of it self explains the usage of it, instead of leaving it generic, what do you think?

(Application) context, buildInfoProvider, activityFramesTracker));
options.addIntegration(new ActivityBreadcrumbsIntegration((Application) context));
options.addIntegration(new UserInteractionIntegration((Application) context, loadClass));
options.addIntegration(new ShakeDetectionIntegration((Application) context));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: is it possible to only add when useShakeGesture is set to true?

Comment on lines +81 to +82
double gForce = Math.sqrt(gX * gX + gY * gY + gZ * gZ);
if (gForce > SHAKE_THRESHOLD_GRAVITY) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about

Suggested change
double gForce = Math.sqrt(gX * gX + gY * gY + gZ * gZ);
if (gForce > SHAKE_THRESHOLD_GRAVITY) {
double gForceSquared = gX * gX + gY * gY + gZ * gZ;
if (gForceSquared > SHAKE_THRESHOLD_GRAVITY * SHAKE_THRESHOLD_GRAVITY) {

sensorManager.unregisterListener(this);
sensorManager = null;
}
listener = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: why not set listener null first?

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.

2 participants