-
-
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
16
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.
+594
โ1
Open
Changes from all commits
Commits
Show all changes
16 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 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
162 changes: 162 additions & 0 deletions
162
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,162 @@ | ||
| 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 @Nullable SentryShakeDetector shakeDetector; | ||
| private @Nullable SentryAndroidOptions options; | ||
| private volatile @Nullable Activity currentActivity; | ||
| private volatile boolean isDialogShowing = false; | ||
| private volatile @Nullable Activity dialogActivity; | ||
| private volatile @Nullable Runnable previousOnFormClose; | ||
|
|
||
| public FeedbackShakeIntegration(final @NotNull Application application) { | ||
| this.application = Objects.requireNonNull(application, "Application is required"); | ||
| } | ||
|
|
||
| @Override | ||
| public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions sentryOptions) { | ||
| this.options = (SentryAndroidOptions) sentryOptions; | ||
|
|
||
| if (!this.options.getFeedbackOptions().isUseShakeGesture()) { | ||
| return; | ||
| } | ||
|
|
||
| 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(); | ||
| } | ||
|
|
||
| @Override | ||
| public void onActivityResumed(final @NotNull Activity activity) { | ||
| 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(); | ||
| 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 (activity == dialogActivity) { | ||
| isDialogShowing = false; | ||
| dialogActivity = null; | ||
| if (options != null) { | ||
| options.getFeedbackOptions().setOnFormClose(previousOnFormClose); | ||
| } | ||
| previousOnFormClose = null; | ||
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
|
||
|
|
||
| private void startShakeDetection(final @NotNull Activity activity) { | ||
| if (options == null) { | ||
| return; | ||
| } | ||
| // Stop any existing detector (e.g. when transitioning between activities) | ||
| stopShakeDetection(); | ||
| shakeDetector = new SentryShakeDetector(options.getLogger()); | ||
| shakeDetector.start( | ||
| activity, | ||
| () -> { | ||
| final Activity active = currentActivity; | ||
| if (active != null && options != null && !isDialogShowing) { | ||
| active.runOnUiThread( | ||
| () -> { | ||
| if (isDialogShowing) { | ||
| return; | ||
| } | ||
| try { | ||
| isDialogShowing = true; | ||
| dialogActivity = active; | ||
| previousOnFormClose = options.getFeedbackOptions().getOnFormClose(); | ||
| options | ||
| .getFeedbackOptions() | ||
| .setOnFormClose( | ||
| () -> { | ||
| isDialogShowing = false; | ||
| dialogActivity = null; | ||
| options.getFeedbackOptions().setOnFormClose(previousOnFormClose); | ||
| if (previousOnFormClose != null) { | ||
| previousOnFormClose.run(); | ||
| } | ||
| previousOnFormClose = null; | ||
| }); | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| options.getFeedbackOptions().getDialogHandler().showDialog(null, null); | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
antonis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } catch (Throwable e) { | ||
| isDialogShowing = false; | ||
| dialogActivity = null; | ||
| 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() { | ||
| if (shakeDetector != null) { | ||
| shakeDetector.stop(); | ||
| shakeDetector = null; | ||
| } | ||
| } | ||
| } | ||
112 changes: 112 additions & 0 deletions
112
sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.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,112 @@ | ||
| package io.sentry.android.core; | ||
|
|
||
| import android.content.Context; | ||
| import android.hardware.Sensor; | ||
| import android.hardware.SensorEvent; | ||
| import android.hardware.SensorEventListener; | ||
| import android.hardware.SensorManager; | ||
| import android.os.SystemClock; | ||
| import io.sentry.ILogger; | ||
| import io.sentry.SentryLevel; | ||
| import java.util.concurrent.atomic.AtomicLong; | ||
| import org.jetbrains.annotations.ApiStatus; | ||
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| /** | ||
| * Detects shake gestures using the device's accelerometer. | ||
| * | ||
| * <p>The accelerometer sensor (TYPE_ACCELEROMETER) does NOT require any special permissions on | ||
| * Android. The BODY_SENSORS permission is only needed for heart rate and similar body sensors. | ||
| * | ||
| * <p>Requires at least {@link #SHAKE_COUNT_THRESHOLD} accelerometer readings above {@link | ||
| * #SHAKE_THRESHOLD_GRAVITY} within {@link #SHAKE_WINDOW_MS} to trigger a shake event. | ||
| */ | ||
| @ApiStatus.Internal | ||
| public final class SentryShakeDetector implements SensorEventListener { | ||
|
|
||
| private static final float SHAKE_THRESHOLD_GRAVITY = 2.7f; | ||
| private static final int SHAKE_WINDOW_MS = 1500; | ||
| private static final int SHAKE_COUNT_THRESHOLD = 2; | ||
| private static final int SHAKE_COOLDOWN_MS = 1000; | ||
|
|
||
| private @Nullable SensorManager sensorManager; | ||
| private final @NotNull AtomicLong lastShakeTimestamp = new AtomicLong(0); | ||
| private volatile @Nullable Listener listener; | ||
| private final @NotNull ILogger logger; | ||
|
|
||
| private int shakeCount = 0; | ||
| private long firstShakeTimestamp = 0; | ||
|
|
||
| public interface Listener { | ||
| void onShake(); | ||
| } | ||
|
|
||
| public SentryShakeDetector(final @NotNull ILogger logger) { | ||
| this.logger = logger; | ||
| } | ||
|
|
||
| public void start(final @NotNull Context context, final @NotNull Listener shakeListener) { | ||
| this.listener = shakeListener; | ||
| sensorManager = (SensorManager) context.getSystemService(Context.SENSOR_SERVICE); | ||
| if (sensorManager == null) { | ||
| logger.log(SentryLevel.WARNING, "SensorManager is not available. Shake detection disabled."); | ||
| return; | ||
| } | ||
| Sensor accelerometer = sensorManager.getDefaultSensor(Sensor.TYPE_ACCELEROMETER); | ||
| if (accelerometer == null) { | ||
| logger.log( | ||
| SentryLevel.WARNING, "Accelerometer sensor not available. Shake detection disabled."); | ||
| return; | ||
| } | ||
| sensorManager.registerListener(this, accelerometer, SensorManager.SENSOR_DELAY_NORMAL); | ||
| } | ||
antonis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| public void stop() { | ||
| listener = null; | ||
| if (sensorManager != null) { | ||
| sensorManager.unregisterListener(this); | ||
| sensorManager = null; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void onSensorChanged(final @NotNull SensorEvent event) { | ||
| if (event.sensor.getType() != Sensor.TYPE_ACCELEROMETER) { | ||
| return; | ||
| } | ||
| float gX = event.values[0] / SensorManager.GRAVITY_EARTH; | ||
| float gY = event.values[1] / SensorManager.GRAVITY_EARTH; | ||
| float gZ = event.values[2] / SensorManager.GRAVITY_EARTH; | ||
| double gForceSquared = gX * gX + gY * gY + gZ * gZ; | ||
| if (gForceSquared > SHAKE_THRESHOLD_GRAVITY * SHAKE_THRESHOLD_GRAVITY) { | ||
| long now = SystemClock.elapsedRealtime(); | ||
|
|
||
| // Reset counter if outside the detection window | ||
| if (now - firstShakeTimestamp > SHAKE_WINDOW_MS) { | ||
| shakeCount = 0; | ||
| firstShakeTimestamp = now; | ||
| } | ||
|
|
||
| shakeCount++; | ||
|
|
||
| if (shakeCount >= SHAKE_COUNT_THRESHOLD) { | ||
| // Enforce cooldown so we don't fire repeatedly | ||
| long lastShake = lastShakeTimestamp.get(); | ||
| if (now - lastShake > SHAKE_COOLDOWN_MS) { | ||
| lastShakeTimestamp.set(now); | ||
| shakeCount = 0; | ||
| final @Nullable Listener currentListener = listener; | ||
| if (currentListener != null) { | ||
| currentListener.onShake(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void onAccuracyChanged(final @NotNull Sensor sensor, final int accuracy) { | ||
| // Not needed for shake detection. | ||
| } | ||
| } | ||
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