-
-
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
Open
antonis
wants to merge
30
commits into
main
Choose a base branch
from
antonis/feedback-shake
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+695
โ1
Open
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
c04bebe
feat(feedback): implement shake gesture detection for user feedback form
antonis 177bb48
fix(feedback): improve shake detection robustness and add tests
antonis d13da1b
Merge branch 'main' into antonis/feedback-shake
antonis 0cc2f3b
fix(feedback): prevent stacking multiple feedback dialogs on repeatedโฆ
antonis 83eed6d
fix(feedback): restore original onFormClose to prevent callback chainโฆ
antonis dbbd37e
fix(feedback): reset isDialogShowing on activity pause to prevent stuโฆ
antonis 527abb7
fix(feedback): move isDialogShowing reset from onActivityPaused to onโฆ
antonis 0b090bc
fix(feedback): scope dialog flag to hosting activity and restore callโฆ
antonis d77d60d
Optimise comparison
antonis 1fd4f5b
ref(feedback): address review feedback from lucas-zimerman
antonis 43b5ebc
fix(feedback): capture onFormClose at shake time instead of registration
antonis 0cb8d00
Reverse sample changes
antonis 285afde
fix(feedback): restore onFormClose in onActivityDestroyed fallback path
antonis 87d508a
fix(feedback): make previousOnFormClose volatile for thread safety
antonis 8e2dee3
fix(feedback): always restore onFormClose in onActivityDestroyed evenโฆ
antonis d180230
Merge branch 'main' into antonis/feedback-shake
antonis 344d6fe
Merge branch 'main' into antonis/feedback-shake
antonis 6993267
Update changelog
antonis bac65b8
Merge remote-tracking branch 'origin/main' into antonis/feedback-shake
antonis 9aa138e
ref(feedback): address review feedback for shake gesture detection
antonis 60b8a66
Format code
getsentry-bot 65122ca
feat(feedback): add manifest meta-data support for useShakeGesture
antonis 59cfc0a
fix(feedback): preserve currentActivity in onActivityPaused when dialโฆ
antonis 69ee931
fix(feedback): pass real logger to SentryShakeDetector on init
antonis 7ba0447
Format code
getsentry-bot a9ac3e1
Merge branch 'antonis/feedback-shake' of github.com:getsentry/sentry-โฆ
antonis a734533
fix(feedback): clear stale activity ref and reset shake state on stop
antonis 8b9bab2
fix(feedback): clean up dialog state when a different activity resumes
antonis 301cc33
fix(feedback): capture onFormClose as local variable in lambda
antonis 1eb6c23
fix(feedback): restore onFormClose in close() when dialog is showing
antonis File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
188 changes: 188 additions & 0 deletions
188
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
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 |
|---|---|---|
| @@ -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); | ||
| } | ||
|
|
||
| @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(); | ||
| } 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(); | ||
| } | ||
| } | ||
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
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.
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