-
-
Notifications
You must be signed in to change notification settings - Fork 467
feat(feedback): implement shake gesture detection #5150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c04bebe
177bb48
d13da1b
0cc2f3b
83eed6d
dbbd37e
527abb7
0b090bc
d77d60d
1fd4f5b
43b5ebc
0cb8d00
285afde
87d508a
8e2dee3
d180230
344d6fe
6993267
bac65b8
9aa138e
60b8a66
65122ca
59cfc0a
69ee931
7ba0447
a9ac3e1
a734533
8b9bab2
301cc33
1eb6c23
a4b5a97
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,188 @@ | ||
| package io.sentry.android.core; | ||
|
|
||
| import static io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion; | ||
|
|
||
| import android.app.Activity; | ||
| import android.app.Application; | ||
| import android.os.Bundle; | ||
| import io.sentry.IScopes; | ||
| import io.sentry.Integration; | ||
| import io.sentry.SentryLevel; | ||
| import io.sentry.SentryOptions; | ||
| import io.sentry.util.Objects; | ||
| import java.io.Closeable; | ||
| import java.io.IOException; | ||
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| /** | ||
| * Detects shake gestures and shows the user feedback dialog when a shake is detected. Only active | ||
| * when {@link io.sentry.SentryFeedbackOptions#isUseShakeGesture()} returns {@code true}. | ||
| */ | ||
| public final class FeedbackShakeIntegration | ||
| implements Integration, Closeable, Application.ActivityLifecycleCallbacks { | ||
|
|
||
| private final @NotNull Application application; | ||
| private final @NotNull SentryShakeDetector shakeDetector; | ||
| private @Nullable SentryAndroidOptions options; | ||
| private volatile @Nullable Activity currentActivity; | ||
| private volatile boolean isDialogShowing = false; | ||
| private volatile @Nullable Runnable previousOnFormClose; | ||
|
|
||
| public FeedbackShakeIntegration(final @NotNull Application application) { | ||
| this.application = Objects.requireNonNull(application, "Application is required"); | ||
| this.shakeDetector = new SentryShakeDetector(io.sentry.NoOpLogger.getInstance()); | ||
antonis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| @Override | ||
| public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions sentryOptions) { | ||
| this.options = (SentryAndroidOptions) sentryOptions; | ||
|
|
||
| if (!this.options.getFeedbackOptions().isUseShakeGesture()) { | ||
| return; | ||
| } | ||
|
|
||
| shakeDetector.init(application, options.getLogger()); | ||
|
|
||
| addIntegrationToSdkVersion("FeedbackShake"); | ||
| application.registerActivityLifecycleCallbacks(this); | ||
| options.getLogger().log(SentryLevel.DEBUG, "FeedbackShakeIntegration installed."); | ||
|
|
||
| // In case of a deferred init, hook into any already-resumed activity | ||
| final @Nullable Activity activity = CurrentActivityHolder.getInstance().getActivity(); | ||
| if (activity != null) { | ||
| currentActivity = activity; | ||
| startShakeDetection(activity); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void close() throws IOException { | ||
| application.unregisterActivityLifecycleCallbacks(this); | ||
| stopShakeDetection(); | ||
| // Restore onFormClose if a dialog is still showing, since lifecycle callbacks | ||
| // are now unregistered and onActivityDestroyed cleanup won't fire. | ||
| if (isDialogShowing) { | ||
| isDialogShowing = false; | ||
| if (options != null) { | ||
| options.getFeedbackOptions().setOnFormClose(previousOnFormClose); | ||
| } | ||
| previousOnFormClose = null; | ||
| } | ||
| currentActivity = null; | ||
| } | ||
antonis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| @Override | ||
| public void onActivityResumed(final @NotNull Activity activity) { | ||
| // If a dialog is showing on a different activity (e.g. user navigated via notification), | ||
| // clean up since the dialog's host activity is going away and onActivityDestroyed | ||
| // won't match currentActivity anymore. | ||
| if (isDialogShowing && currentActivity != null && currentActivity != activity) { | ||
| isDialogShowing = false; | ||
| if (options != null) { | ||
| options.getFeedbackOptions().setOnFormClose(previousOnFormClose); | ||
| } | ||
| previousOnFormClose = null; | ||
| } | ||
| currentActivity = activity; | ||
| startShakeDetection(activity); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Concurrent dialog wrappers corrupt
|
||
|
|
||
| @Override | ||
| 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. | ||
| if (activity == currentActivity) { | ||
| stopShakeDetection(); | ||
| // Keep currentActivity set when a dialog is showing so onActivityDestroyed | ||
| // can still match and clean up. Otherwise the cleanup condition | ||
| // (activity == currentActivity) would always be false since onPause fires | ||
| // before onDestroy. | ||
| if (!isDialogShowing) { | ||
| currentActivity = null; | ||
| } | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| @Override | ||
| public void onActivityCreated( | ||
| final @NotNull Activity activity, final @Nullable Bundle savedInstanceState) {} | ||
|
|
||
| @Override | ||
| public void onActivityStarted(final @NotNull Activity activity) {} | ||
|
|
||
| @Override | ||
| public void onActivityStopped(final @NotNull Activity activity) {} | ||
|
|
||
| @Override | ||
| public void onActivitySaveInstanceState( | ||
| final @NotNull Activity activity, final @NotNull Bundle outState) {} | ||
|
|
||
| @Override | ||
| public void onActivityDestroyed(final @NotNull Activity activity) { | ||
| // Only reset if this is the activity that hosts the dialog โ the dialog cannot | ||
| // outlive its host activity being destroyed. | ||
| if (isDialogShowing && activity == currentActivity) { | ||
sentry[bot] marked this conversation as resolved.
Show resolved
Hide resolved
sentry[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| isDialogShowing = false; | ||
| currentActivity = null; | ||
| if (options != null) { | ||
| options.getFeedbackOptions().setOnFormClose(previousOnFormClose); | ||
| } | ||
| previousOnFormClose = null; | ||
sentry[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
sentry[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
antonis marked this conversation as resolved.
Show resolved
Hide resolved
antonis marked this conversation as resolved.
Show resolved
Hide resolved
antonis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| private void startShakeDetection(final @NotNull Activity activity) { | ||
| if (options == null) { | ||
| return; | ||
| } | ||
| // Stop any existing detection (e.g. when transitioning between activities) | ||
| stopShakeDetection(); | ||
| shakeDetector.start( | ||
| activity, | ||
| () -> { | ||
| final Activity active = currentActivity; | ||
| final Boolean inBackground = AppState.getInstance().isInBackground(); | ||
| if (active != null | ||
| && options != null | ||
| && !isDialogShowing | ||
| && !Boolean.TRUE.equals(inBackground)) { | ||
| active.runOnUiThread( | ||
| () -> { | ||
| if (isDialogShowing) { | ||
| return; | ||
| } | ||
| try { | ||
| isDialogShowing = true; | ||
| final Runnable captured = options.getFeedbackOptions().getOnFormClose(); | ||
| previousOnFormClose = captured; | ||
| options | ||
| .getFeedbackOptions() | ||
| .setOnFormClose( | ||
| () -> { | ||
| isDialogShowing = false; | ||
| options.getFeedbackOptions().setOnFormClose(captured); | ||
| if (captured != null) { | ||
| captured.run(); | ||
| } | ||
| previousOnFormClose = null; | ||
antonis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }); | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| new SentryUserFeedbackDialog.Builder(active).create().show(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing activity validity check before showing dialogLow Severity The shake callback captures |
||
| } catch (Throwable e) { | ||
| isDialogShowing = false; | ||
| options.getFeedbackOptions().setOnFormClose(previousOnFormClose); | ||
| previousOnFormClose = null; | ||
| options | ||
| .getLogger() | ||
| .log(SentryLevel.ERROR, "Failed to show feedback dialog on shake.", e); | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }); | ||
| } | ||
| }); | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| private void stopShakeDetection() { | ||
| shakeDetector.stop(); | ||
| } | ||
| } | ||


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.
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
FeedbackShakeIntegrationso the name of it self explains the usage of it, instead of leaving it generic, what do you think?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.
Updated with 1fd4f5b