Skip to content
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

Add haptic feedback toggle for code refresh #1599

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michaelschattgen
Copy link
Member

This PR adds the ability to enable/disable haptic feedback when the codes are about to expire or when they refresh.

Copy link
Member

@alexbakker alexbakker left a comment

Choose a reason for hiding this comment

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

Neat! I like this more than I thought I would. I have a couple of comments.

public void onAnimationEnd(Animator animation) {
if (!_nonUniform) {
if (itemView.isShown()) {
VibrationHelper.vibratePattern(itemView.getContext(), VibrationPatterns.EXPIRING);
Copy link
Member

Choose a reason for hiding this comment

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

This results in starting a vibration for every single entry.

Since we're not relying on repeated animation callbacks for the "ticks" of the vibration, I don't think we need to link this to the animation. Perhaps a second postDelayed in UiRefresher would do the trick? This would also make the vibrations work in case animations are disabled, which is currently not the case.

@@ -144,6 +146,10 @@ public int getSpanSize(int position) {
@Override
public void onRefresh() {
refresh(false);

if (_recyclerView.isShown()) {
VibrationHelper.vibratePattern(getContext(), VibrationPatterns.REFRESH_CODE);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this is only the case on my device, but I don't feel this vibration.

}

public static boolean isHapticFeedbackEnabled(Context context) {
Preferences _prefs = EarlyEntryPoints.get(context.getApplicationContext(), PrefEntryPoint.class).getPreferences();
Copy link
Member

Choose a reason for hiding this comment

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

This isn't really what EarlyEntryPoints is meant for. I would suggest simply initializing a new Preferences based on the context. If it turns out we can move the vibration trigger to UiRefresher, we can do this once in the constructor of VibrationHelper.

@@ -23,7 +23,6 @@ public void start() {
}
_running = true;

_listener.onRefresh();
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure we can remove this?

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