Skip to content

GitLab

T
tangled
Commit b23fda7e authored by Jordan Demeulenaere's avatar Jordan Demeulenaere Committed by Chromium LUCI CQ
Browse files
Options

[Autofill Assistant] Fix the details placeholder animation. 

This CL fixes the placeholder animation which was not correctly
supporting multiple details. Now, all details with placeholders
subscribe to the placeholder animation, which is stopped each time we
change the details.

This CL also fixes the wrong type of ShowDetailsProto.delay_ms, which
should be an int32 and not a bool.

Bug: b/174652960
Change-Id: Ib37479337bfc732d57c2b60ff168c28444e2575f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2625885
Commit-Queue: Jordan Demeulenaere <jdemeulenaere@chromium.org>
Auto-Submit: Jordan Demeulenaere <jdemeulenaere@chromium.org>
Reviewed-by: default avatarClemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#843439}
parent 0100ecc7
Show whitespace changes
Inline Side-by-side
Showing with 81 additions and 33 deletions
chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/details/AssistantDetailsAdapter.java
View file @ b23fda7e
......@@ -6,8 +6,6 @@ package org.chromium.chrome.browser.autofill_assistant.details;
import static org.chromium.chrome.browser.autofill_assistant.AssistantAccessibilityUtils.setAccessibility;
import android.animation.Animator;
import android.animation.AnimatorListenerAdapter;
import android.animation.ArgbEvaluator;
import android.animation.ValueAnimator;
import android.content.Context;
......@@ -18,6 +16,7 @@ import android.graphics.Typeface;
import android.graphics.drawable.Drawable;
import android.graphics.drawable.GradientDrawable;
import android.media.ThumbnailUtils;
import android.support.annotation.VisibleForTesting;
import android.text.TextUtils;
import android.util.TypedValue;
import android.view.View;
......@@ -34,6 +33,7 @@ import androidx.recyclerview.widget.DiffUtil;
import androidx.recyclerview.widget.RecyclerView;
import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.Callback;
import org.chromium.chrome.autofill_assistant.R;
import org.chromium.chrome.browser.autofill_assistant.AssistantTextUtils;
import org.chromium.chrome.browser.autofill_assistant.LayoutUtils;
......@@ -100,7 +100,8 @@ class AssistantDetailsAdapter extends RecyclerView.Adapter<AssistantDetailsAdapt
private final int mTextPlaceholdersHeight;
private final int mTextPlaceholdersMargin;
private ValueAnimator mPulseAnimation;
private ValueAnimator mPlaceholdersColorAnimation;
private List<Callback<Integer>> mPlaceholdersColorCallbacks = new ArrayList<>();
private ImageFetcher mImageFetcher;
AssistantDetailsAdapter(Context context, ImageFetcher imageFetcher) {
......@@ -163,6 +164,9 @@ class AssistantDetailsAdapter extends RecyclerView.Adapter<AssistantDetailsAdapt
}
}, /* detectMoves= */ false);
// Stop the placeholders animation.
stopPlaceholderAnimations();
// Set the details.
mDetails.clear();
mDetails.addAll(details);
......@@ -322,8 +326,6 @@ class AssistantDetailsAdapter extends RecyclerView.Adapter<AssistantDetailsAdapt
if (shouldStartOrContinuePlaceholderAnimation(details.getPlaceholdersConfiguration())) {
startOrContinuePlaceholderAnimations(details, viewHolder);
} else {
stopPlaceholderAnimations();
}
}
......@@ -361,29 +363,26 @@ class AssistantDetailsAdapter extends RecyclerView.Adapter<AssistantDetailsAdapt
private void startOrContinuePlaceholderAnimations(
AssistantDetails details, ViewHolder viewHolder) {
if (mPulseAnimation != null) {
return;
}
mPulseAnimation = ValueAnimator.ofInt(mPulseAnimationStartColor, mPulseAnimationEndColor);
mPulseAnimation.setDuration(PULSING_DURATION_MS);
mPulseAnimation.setEvaluator(new ArgbEvaluator());
mPulseAnimation.setRepeatCount(ValueAnimator.INFINITE);
mPulseAnimation.setRepeatMode(ValueAnimator.REVERSE);
mPulseAnimation.setInterpolator(Interpolators.ACCELERATE_INTERPOLATOR);
mPulseAnimation.addListener(new AnimatorListenerAdapter() {
@Override
public void onAnimationCancel(Animator animation) {
viewHolder.mTitleView.setBackgroundColor(Color.TRANSPARENT);
viewHolder.mDescriptionLine1View.setBackgroundColor(Color.TRANSPARENT);
viewHolder.mDescriptionLine2View.setBackgroundColor(Color.TRANSPARENT);
viewHolder.mDescriptionLine3View.setBackgroundColor(Color.TRANSPARENT);
viewHolder.mDefaultImage.setColor(Color.TRANSPARENT);
}
// Start the placeholders color animation if necessary.
if (mPlaceholdersColorAnimation == null) {
mPlaceholdersColorAnimation =
ValueAnimator.ofInt(mPulseAnimationStartColor, mPulseAnimationEndColor);
mPlaceholdersColorAnimation.setDuration(PULSING_DURATION_MS);
mPlaceholdersColorAnimation.setEvaluator(new ArgbEvaluator());
mPlaceholdersColorAnimation.setRepeatCount(ValueAnimator.INFINITE);
mPlaceholdersColorAnimation.setRepeatMode(ValueAnimator.REVERSE);
mPlaceholdersColorAnimation.setInterpolator(Interpolators.ACCELERATE_INTERPOLATOR);
mPlaceholdersColorAnimation.addUpdateListener(animation -> {
// Set the color of the placeholders.
int animatedValue = (int) animation.getAnimatedValue();
setPlaceholdersColor(animatedValue);
});
mPlaceholdersColorAnimation.start();
}
// Change the background color of the placeholders when the animated value changes.
AssistantPlaceholdersConfiguration placeholders = details.getPlaceholdersConfiguration();
mPulseAnimation.addUpdateListener(animation -> {
int animatedValue = (int) animation.getAnimatedValue();
mPlaceholdersColorCallbacks.add(animatedValue -> {
viewHolder.mTitleView.setBackgroundColor(
placeholders.getShowTitlePlaceholder() ? animatedValue : Color.TRANSPARENT);
viewHolder.mDescriptionLine1View.setBackgroundColor(
......@@ -398,16 +397,29 @@ class AssistantDetailsAdapter extends RecyclerView.Adapter<AssistantDetailsAdapt
viewHolder.mDefaultImage.setColor(
placeholders.getShowImagePlaceholder() ? animatedValue : Color.TRANSPARENT);
});
mPulseAnimation.start();
}
private void stopPlaceholderAnimations() {
if (mPulseAnimation != null) {
mPulseAnimation.cancel();
mPulseAnimation = null;
setPlaceholdersColor(Color.TRANSPARENT);
mPlaceholdersColorCallbacks.clear();
if (mPlaceholdersColorAnimation != null) {
mPlaceholdersColorAnimation.cancel();
mPlaceholdersColorAnimation = null;
}
}
private void setPlaceholdersColor(int color) {
for (Callback<Integer> callback : mPlaceholdersColorCallbacks) {
callback.onResult(color);
}
}
@VisibleForTesting
boolean isRunningPlaceholdersAnimation() {
return mPlaceholdersColorAnimation != null;
}
/**
* Clicking on the image will trigger a modal dialog asking whether the user wants to
* see the original image, if they choose to see it, a new custom tab pointing to the
......
chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/details/AssistantDetailsCoordinator.java
View file @ b23fda7e
......@@ -8,6 +8,7 @@ import android.content.Context;
import android.graphics.Canvas;
import android.graphics.Rect;
import android.graphics.drawable.Drawable;
import android.support.annotation.VisibleForTesting;
import android.view.View;
import android.view.ViewGroup;
......@@ -23,6 +24,7 @@ import org.chromium.chrome.browser.image_fetcher.ImageFetcher;
*/
public class AssistantDetailsCoordinator {
private final RecyclerView mView;
private final AssistantDetailsAdapter mAdapter;
public AssistantDetailsCoordinator(
Context context, AssistantDetailsModel model, ImageFetcher imageFetcher) {
......@@ -33,13 +35,13 @@ public class AssistantDetailsCoordinator {
context, LinearLayoutManager.VERTICAL, /* reverseLayout= */ false));
mView.setBackgroundResource(R.drawable.autofill_assistant_details_bg);
mView.addItemDecoration(new DetailsItemDecoration(context));
AssistantDetailsAdapter adapter = new AssistantDetailsAdapter(context, imageFetcher);
mView.setAdapter(adapter);
mAdapter = new AssistantDetailsAdapter(context, imageFetcher);
mView.setAdapter(mAdapter);
// Listen to the model and set the details on the adapter when they change.
model.addObserver((source, propertyKey) -> {
if (propertyKey == AssistantDetailsModel.DETAILS) {
adapter.setDetails(model.get(AssistantDetailsModel.DETAILS));
mAdapter.setDetails(model.get(AssistantDetailsModel.DETAILS));
}
});
......@@ -62,6 +64,11 @@ public class AssistantDetailsCoordinator {
return mView;
}
@VisibleForTesting
public boolean isRunningPlaceholdersAnimationForTesting() {
return mAdapter.isRunningPlaceholdersAnimation();
}
/** A divider that is drawn after each details, except the last one. */
private static class DetailsItemDecoration extends RecyclerView.ItemDecoration {
private final Drawable mDrawable;
......
chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantDetailsUiTest.java
View file @ b23fda7e
......@@ -122,6 +122,9 @@ public class AutofillAssistantDetailsUiTest {
private static void setDetails(AssistantDetailsModel model, AssistantDetails... details) {
runOnUiThreadBlocking(() -> model.setDetailsList(Arrays.asList(details)));
// Wait for the main thread to be idle (i.e. the UI should be stable).
InstrumentationRegistry.getInstrumentation().waitForIdleSync();
}
@Before
......@@ -488,4 +491,30 @@ public class AutofillAssistantDetailsUiTest {
onView(allOf(viewMatchers.mTitleView, withText("title 2"))).check(matches(isDisplayed()));
onView(allOf(viewMatchers.mTitleView, withText("title 3"))).check(matches(isDisplayed()));
}
@Test
@MediumTest
public void testPlaceholdersAnimation() throws Exception {
// Test that the placeholders animation is running only when details have placeholders.
AssistantDetailsModel model = new AssistantDetailsModel();
AssistantDetailsCoordinator coordinator = createCoordinator(model);
assertThat(coordinator.isRunningPlaceholdersAnimationForTesting(), is(false));
setDetails(model,
new AssistantDetails("title 1", "", "", null, "", "", "", "", "", "", false, false,
false, false, false, NO_PLACEHOLDERS));
assertThat(coordinator.isRunningPlaceholdersAnimationForTesting(), is(false));
setDetails(model,
new AssistantDetails("title 1", "", "", null, "", "", "", "", "", "", false, false,
false, false, false,
new AssistantPlaceholdersConfiguration(
/* showImagePlaceholder= */ true,
/* showTitlePlaceholder= */ false,
/* showDescriptionLine1Placeholder= */ false,
/* showDescriptionLine2Placeholder= */ false,
/* showDescriptionLine3Placeholder= */ false)));
assertThat(coordinator.isRunningPlaceholdersAnimationForTesting(), is(true));
setDetails(model);
assertThat(coordinator.isRunningPlaceholdersAnimationForTesting(), is(false));
}
}
components/autofill_assistant/browser/service.proto
View file @ b23fda7e
......@@ -2400,7 +2400,7 @@ message ShowDetailsProto {
//
// Note that it's not possible to delay the clearing of the details. If
// |data_to_show| is not set, then the details will be cleared directly.
optional bool delay_ms = 7;
optional int32 delay_ms = 7;
}
// Set the value of an form element.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment