feat(feedback): Support runtime enable/disable of shake-to-report #5827
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,9 +4,13 @@ | |
|
|
||
| import android.app.Activity; | ||
| import android.app.Application; | ||
| import android.app.Dialog; | ||
| import android.content.Context; | ||
| import android.content.ContextWrapper; | ||
| import android.os.Bundle; | ||
| import io.sentry.IScopes; | ||
| import io.sentry.Integration; | ||
| import io.sentry.SentryFeedbackOptions; | ||
| import io.sentry.SentryLevel; | ||
| import io.sentry.SentryOptions; | ||
| import io.sentry.util.Objects; | ||
|
|
@@ -17,18 +21,30 @@ | |
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| /** | ||
| * Detects shake gestures and shows the user feedback dialog when a shake is detected. {@link | ||
| * io.sentry.SentryFeedbackOptions#isUseShakeGesture()} determines the initial state; it can be | ||
| * toggled at runtime via {@code Sentry.feedback().enableFeedbackOnShake()} and {@code | ||
| * Sentry.feedback().disableFeedbackOnShake()}. | ||
| * | ||
| * <p>A single detector serves both the global toggle and individual dialogs set via {@link | ||
| * #setDialog(SentryFeedbackOptions.IShakeDialog, boolean)}. While a dialog is tracked, a shake on | ||
| * its host activity re-shows that dialog instead of creating a new form — a no-op when it is | ||
| * already visible, so a shake can never stack a second form on top of one that is showing. | ||
| */ | ||
| public final class FeedbackShakeIntegration | ||
| implements Integration, | ||
| Closeable, | ||
| Application.ActivityLifecycleCallbacks, | ||
| SentryFeedbackOptions.IShakeController { | ||
|
|
||
| private final @NotNull Application application; | ||
| private final @NotNull SentryShakeDetector shakeDetector; | ||
| private @Nullable SentryAndroidOptions options; | ||
| private volatile boolean enabled = false; | ||
| private boolean detecting = false; | ||
| private volatile @Nullable WeakReference<SentryFeedbackOptions.IShakeDialog> dialogRef; | ||
| private boolean dialogRequestedShakeDetection = false; | ||
| private volatile @Nullable WeakReference<Activity> currentActivityRef; | ||
|
|
||
| public FeedbackShakeIntegration(final @NotNull Application application) { | ||
| this.application = Objects.requireNonNull(application, "Application is required"); | ||
|
|
@@ -46,13 +62,74 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions | |
|
|
||
| final @NotNull SentryAndroidOptions options = this.options; | ||
|
|
||
| // Always expose the runtime toggle, even when the option starts out disabled. | ||
| options.getFeedbackOptions().setShakeController(this); | ||
|
|
||
| if (options.getFeedbackOptions().isUseShakeGesture()) { | ||
| enable(); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized void enable() { | ||
| final @Nullable SentryAndroidOptions options = this.options; | ||
| if (enabled || options == null) { | ||
| return; | ||
| } | ||
| enabled = true; | ||
| startDetecting(options); | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized void disable() { | ||
| if (!enabled) { | ||
| return; | ||
| } | ||
| enabled = false; | ||
| if (!dialogRequestedShakeDetection || getDialog() == null) { | ||
| stopDetecting(); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isEnabled() { | ||
| return enabled; | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized void setDialog( | ||
| final @Nullable SentryFeedbackOptions.IShakeDialog dialog, | ||
| final boolean startShakeDetection) { | ||
| final @Nullable SentryAndroidOptions options = this.options; | ||
| if (options == null) { | ||
| return; | ||
| } | ||
| if (dialog == null) { | ||
| dialogRef = null; | ||
| dialogRequestedShakeDetection = false; | ||
| if (!enabled) { | ||
| stopDetecting(); | ||
| } | ||
| return; | ||
| } | ||
| dialogRef = new WeakReference<>(dialog); | ||
| if (startShakeDetection) { | ||
| dialogRequestedShakeDetection = true; | ||
| startDetecting(options); | ||
| } else { | ||
| dialogRequestedShakeDetection = false; | ||
|
There was a problem hiding this comment. Bug: Calling Suggested FixModify the Prompt for AI AgentAlso affects:
|
||
| } | ||
|
There was a problem hiding this comment. Opt-in flag cleared on re-trackMedium Severity
Additional Locations (1)  Reviewed by Cursor Bugbot for commit ac4292c. Configure here. |
||
| } | ||
|
|
||
| private synchronized void startDetecting(final @NotNull SentryAndroidOptions options) { | ||
| if (detecting) { | ||
| return; | ||
| } | ||
| detecting = true; | ||
|
|
||
| // Re-arm the detector in case it was closed before, either by stopDetecting() or by a previous | ||
| // close() (e.g. a second Sentry.init reusing the same options), otherwise the closed latch | ||
| // would keep shake detection off permanently. | ||
| shakeDetector.reopen(); | ||
|
|
||
| // Resolving the accelerometer is the most expensive part of init (the first SensorManager | ||
|
|
@@ -72,43 +149,35 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions | |
| application.registerActivityLifecycleCallbacks(this); | ||
| options.getLogger().log(SentryLevel.DEBUG, "FeedbackShakeIntegration installed."); | ||
|
|
||
| // In case of a deferred init or runtime enable, hook into any already-resumed activity | ||
| final @Nullable Activity activity = CurrentActivityHolder.getInstance().getActivity(); | ||
| if (activity != null) { | ||
| currentActivityRef = new WeakReference<>(activity); | ||
| startShakeDetection(activity); | ||
| } | ||
| } | ||
|
|
||
| private synchronized void stopDetecting() { | ||
| if (!detecting) { | ||
| return; | ||
| } | ||
| detecting = false; | ||
|
|
||
| application.unregisterActivityLifecycleCallbacks(this); | ||
| shakeDetector.close(); | ||
| currentActivityRef = null; | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized void close() throws IOException { | ||
| enabled = false; | ||
| dialogRef = null; | ||
| dialogRequestedShakeDetection = false; | ||
| stopDetecting(); | ||
| } | ||
|
|
||
| @Override | ||
| public void onActivityResumed(final @NotNull Activity activity) { | ||
| currentActivityRef = new WeakReference<>(activity); | ||
| startShakeDetection(activity); | ||
| } | ||
|
|
@@ -118,16 +187,11 @@ public void onActivityPaused(final @NotNull Activity activity) { | |
| // Only stop if this is the activity we're tracking. When transitioning between | ||
| // activities, B.onResume may fire before A.onPause — stopping unconditionally | ||
| // would kill shake detection for the new activity. | ||
| final @Nullable WeakReference<Activity> currentRef = currentActivityRef; | ||
| final @Nullable Activity current = currentRef != null ? currentRef.get() : null; | ||
| if (activity == current) { | ||
| stopShakeDetection(); | ||
| currentActivityRef = null; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -147,17 +211,31 @@ public void onActivitySaveInstanceState( | |
|
|
||
| @Override | ||
| public void onActivityDestroyed(final @NotNull Activity activity) { | ||
| // A tracked dialog cannot outlive its host activity; drop it so detection doesn't keep | ||
| // running for it (and a shake can't try to show a dead dialog). | ||
| final @Nullable SentryFeedbackOptions.IShakeDialog dialog = getDialog(); | ||
| if (dialog != null && findDialogActivity(dialog) == activity) { | ||
| setDialog(null, false); | ||
| } | ||
| } | ||
|
|
||
| private @Nullable SentryFeedbackOptions.IShakeDialog getDialog() { | ||
| final @Nullable WeakReference<SentryFeedbackOptions.IShakeDialog> ref = dialogRef; | ||
| return ref != null ? ref.get() : null; | ||
| } | ||
|
|
||
| private static @Nullable Activity findDialogActivity( | ||
| final @NotNull SentryFeedbackOptions.IShakeDialog dialog) { | ||
| if (dialog instanceof Dialog) { | ||
| @Nullable Context context = ((Dialog) dialog).getContext(); | ||
| while (context instanceof ContextWrapper) { | ||
| if (context instanceof Activity) { | ||
| return (Activity) context; | ||
| } | ||
| context = ((ContextWrapper) context).getBaseContext(); | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| private void startShakeDetection(final @NotNull Activity activity) { | ||
|
|
@@ -166,47 +244,47 @@ private void startShakeDetection(final @NotNull Activity activity) { | |
| } | ||
| // Stop any existing detection (e.g. when transitioning between activities) | ||
| stopShakeDetection(); | ||
| // When detection runs only for a tracked dialog, don't listen on other activities — | ||
| // a shake there couldn't show the dialog anyway. | ||
| if (!enabled) { | ||
| final @Nullable SentryFeedbackOptions.IShakeDialog dialog = getDialog(); | ||
| if (dialog == null || findDialogActivity(dialog) != activity) { | ||
| return; | ||
| } | ||
| } | ||
| shakeDetector.start( | ||
| activity, | ||
| () -> { | ||
| final @Nullable WeakReference<Activity> ref = currentActivityRef; | ||
| final Activity active = ref != null ? ref.get() : null; | ||
| final Boolean inBackground = AppState.getInstance().isInBackground(); | ||
| if (active == null || options == null || Boolean.TRUE.equals(inBackground)) { | ||
| return; | ||
| } | ||
| // Decide on the main thread: show() sets the tracked dialog synchronously, so a | ||
| // second queued shake sees the form shown by the first instead of creating another. | ||
| active.runOnUiThread( | ||
| () -> { | ||
| if (active.isFinishing() || active.isDestroyed()) { | ||
| return; | ||
| } | ||
| // A dialog tracked for the active activity takes precedence over creating a | ||
| // new form — re-showing it is a no-op while it's already visible. | ||
| final @Nullable SentryFeedbackOptions.IShakeDialog dialog = getDialog(); | ||
| if (dialog != null && findDialogActivity(dialog) == active) { | ||
| dialog.show(); | ||
| return; | ||
| } | ||
| if (enabled) { | ||
| try { | ||
| new SentryUserFeedbackForm.Builder(active).create().show(); | ||
| } catch (Throwable e) { | ||
| options | ||
| .getLogger() | ||
| .log(SentryLevel.ERROR, "Failed to show feedback dialog on shake.", e); | ||
| } | ||
| } | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Opted-in dialog only weakly held
High Severity
Per-form opt-in stores the dialog in a
WeakReference, unlike the previous lifecycle-callback approach that kept the form alive. Acreate()call that does not retain the instance can be collected immediately, so a later shake finds no dialog and does nothing, while detection may keep running because nothing clears it when the weak ref goes null.Additional Locations (1)
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java#L44-L45Reviewed by Cursor Bugbot for commit ac4292c. Configure here.