Commit 45c20d42 authored by Becky Zhou's avatar Becky Zhou Committed by Commit Bot

Fix selectable list UI checkmark reanimates when view rebound

+ Don't run animation when the checked state is first initialized
+ Add a workaround to fix an issue that clearing image tint list on L
  doesn't refresh the drawable state

Bug: 895866, 983686
Change-Id: Id5be04a2877439aafc4da61d15bec0ca486bf26d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1700466Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Commit-Queue: Becky Zhou <huayinz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678457}
parent 09f2244e
...@@ -335,6 +335,12 @@ public class ApiCompatibilityUtils { ...@@ -335,6 +335,12 @@ public class ApiCompatibilityUtils {
} }
} }
ImageViewCompat.setImageTintList(view, tintList); ImageViewCompat.setImageTintList(view, tintList);
if (Build.VERSION.SDK_INT == Build.VERSION_CODES.LOLLIPOP) {
// Work around that the tint list is not cleared when setting tint list to null on L in
// some cases. See https://crbug.com/983686.
if (tintList == null) view.refreshDrawableState();
}
} }
/** /**
......
...@@ -233,7 +233,7 @@ public class DownloadItemView extends SelectableItemView<DownloadHistoryItemWrap ...@@ -233,7 +233,7 @@ public class DownloadItemView extends SelectableItemView<DownloadHistoryItemWrap
// TODO(dfalcantara): Get thumbnails for audio and video files when possible. // TODO(dfalcantara): Get thumbnails for audio and video files when possible.
} }
if (mThumbnailBitmap == null) updateView(); if (mThumbnailBitmap == null) updateView(false);
Context context = mDescriptionCompletedView.getContext(); Context context = mDescriptionCompletedView.getContext();
mFilenameCompletedView.setText(item.getDisplayFileName()); mFilenameCompletedView.setText(item.getDisplayFileName());
...@@ -302,7 +302,7 @@ public class DownloadItemView extends SelectableItemView<DownloadHistoryItemWrap ...@@ -302,7 +302,7 @@ public class DownloadItemView extends SelectableItemView<DownloadHistoryItemWrap
private void setThumbnailBitmap(Bitmap thumbnail) { private void setThumbnailBitmap(Bitmap thumbnail) {
mThumbnailBitmap = thumbnail; mThumbnailBitmap = thumbnail;
updateView(); updateView(false);
} }
@Override @Override
...@@ -325,14 +325,14 @@ public class DownloadItemView extends SelectableItemView<DownloadHistoryItemWrap ...@@ -325,14 +325,14 @@ public class DownloadItemView extends SelectableItemView<DownloadHistoryItemWrap
} }
@Override @Override
protected void updateView() { protected void updateView(boolean animate) {
if (isChecked()) { if (isChecked()) {
mIconView.setBackgroundResource(mIconBackgroundResId); mIconView.setBackgroundResource(mIconBackgroundResId);
mIconView.getBackground().setLevel( mIconView.getBackground().setLevel(
getResources().getInteger(R.integer.list_item_level_selected)); getResources().getInteger(R.integer.list_item_level_selected));
mIconView.setImageDrawable(mCheckDrawable); mIconView.setImageDrawable(mCheckDrawable);
ApiCompatibilityUtils.setImageTintList(mIconView, mCheckedIconForegroundColorList); ApiCompatibilityUtils.setImageTintList(mIconView, mCheckedIconForegroundColorList);
mCheckDrawable.start(); if (animate) mCheckDrawable.start();
} else if (mThumbnailBitmap != null) { } else if (mThumbnailBitmap != null) {
assert !mThumbnailBitmap.isRecycled(); assert !mThumbnailBitmap.isRecycled();
mIconView.setBackground(null); mIconView.setBackground(null);
......
...@@ -66,13 +66,6 @@ public class OfflineGroupHeaderView ...@@ -66,13 +66,6 @@ public class OfflineGroupHeaderView
mAdapter = adapter; mAdapter = adapter;
} }
@Override
public void setChecked(boolean checked) {
if (checked == isChecked()) return;
super.setChecked(checked);
updateCheckIcon(checked);
}
@Override @Override
protected void onAttachedToWindow() { protected void onAttachedToWindow() {
super.onAttachedToWindow(); super.onAttachedToWindow();
...@@ -98,7 +91,6 @@ public class OfflineGroupHeaderView ...@@ -98,7 +91,6 @@ public class OfflineGroupHeaderView
mDescriptionTextView.setText(description); mDescriptionTextView.setText(description);
updateExpandIcon(header.isExpanded()); updateExpandIcon(header.isExpanded());
setChecked(mSelectionDelegate.isHeaderSelected(header)); setChecked(mSelectionDelegate.isHeaderSelected(header));
updateCheckIcon(isChecked());
} }
private void updateExpandIcon(boolean expanded) { private void updateExpandIcon(boolean expanded) {
...@@ -109,15 +101,16 @@ public class OfflineGroupHeaderView ...@@ -109,15 +101,16 @@ public class OfflineGroupHeaderView
: R.string.accessibility_expand_section_header)); : R.string.accessibility_expand_section_header));
} }
private void updateCheckIcon(boolean checked) { @Override
if (checked) { protected void updateView(boolean animate) {
if (isChecked()) {
mIconImageView.setBackgroundResource(mIconBackgroundResId); mIconImageView.setBackgroundResource(mIconBackgroundResId);
mIconImageView.getBackground().setLevel( mIconImageView.getBackground().setLevel(
getResources().getInteger(R.integer.list_item_level_selected)); getResources().getInteger(R.integer.list_item_level_selected));
mIconImageView.setImageDrawable(mCheckDrawable); mIconImageView.setImageDrawable(mCheckDrawable);
ApiCompatibilityUtils.setImageTintList(mIconImageView, mCheckedIconForegroundColorList); ApiCompatibilityUtils.setImageTintList(mIconImageView, mCheckedIconForegroundColorList);
mCheckDrawable.start(); if (animate) mCheckDrawable.start();
} else { } else {
mIconImageView.setBackgroundResource(mIconBackgroundResId); mIconImageView.setBackgroundResource(mIconBackgroundResId);
mIconImageView.getBackground().setLevel( mIconImageView.getBackground().setLevel(
......
...@@ -69,7 +69,7 @@ public abstract class SelectableItemView<E> extends SelectableItemViewBase<E> { ...@@ -69,7 +69,7 @@ public abstract class SelectableItemView<E> extends SelectableItemViewBase<E> {
*/ */
protected void setIconDrawable(Drawable iconDrawable) { protected void setIconDrawable(Drawable iconDrawable) {
mIconDrawable = iconDrawable; mIconDrawable = iconDrawable;
updateView(); updateView(false);
} }
/** /**
...@@ -83,7 +83,7 @@ public abstract class SelectableItemView<E> extends SelectableItemViewBase<E> { ...@@ -83,7 +83,7 @@ public abstract class SelectableItemView<E> extends SelectableItemViewBase<E> {
* Update icon image and background based on whether this item is selected. * Update icon image and background based on whether this item is selected.
*/ */
@Override @Override
protected void updateView() { protected void updateView(boolean animate) {
// TODO(huayinz): Refactor this method so that mIconView is not exposed to subclass. // TODO(huayinz): Refactor this method so that mIconView is not exposed to subclass.
if (mIconView == null) return; if (mIconView == null) return;
...@@ -91,7 +91,7 @@ public abstract class SelectableItemView<E> extends SelectableItemViewBase<E> { ...@@ -91,7 +91,7 @@ public abstract class SelectableItemView<E> extends SelectableItemViewBase<E> {
mIconView.getBackground().setLevel(mSelectedLevel); mIconView.getBackground().setLevel(mSelectedLevel);
mIconView.setImageDrawable(mCheckDrawable); mIconView.setImageDrawable(mCheckDrawable);
ApiCompatibilityUtils.setImageTintList(mIconView, mIconColorList); ApiCompatibilityUtils.setImageTintList(mIconView, mIconColorList);
mCheckDrawable.start(); if (animate) mCheckDrawable.start();
} else { } else {
mIconView.getBackground().setLevel(mDefaultLevel); mIconView.getBackground().setLevel(mDefaultLevel);
mIconView.setImageDrawable(mIconDrawable); mIconView.setImageDrawable(mIconDrawable);
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
package org.chromium.chrome.browser.widget.selection; package org.chromium.chrome.browser.widget.selection;
import android.content.Context; import android.content.Context;
import android.support.annotation.Nullable;
import android.util.AttributeSet; import android.util.AttributeSet;
import android.view.MotionEvent; import android.view.MotionEvent;
import android.view.View; import android.view.View;
...@@ -37,7 +38,7 @@ public abstract class SelectableItemViewBase<E> ...@@ -37,7 +38,7 @@ public abstract class SelectableItemViewBase<E>
private SelectionDelegate<E> mSelectionDelegate; private SelectionDelegate<E> mSelectionDelegate;
private E mItem; private E mItem;
private boolean mIsChecked; private @Nullable Boolean mIsChecked;
// Controls whether selection should happen during onLongClick. // Controls whether selection should happen during onLongClick.
private boolean mSelectOnLongClick = true; private boolean mSelectOnLongClick = true;
...@@ -121,7 +122,7 @@ public abstract class SelectableItemViewBase<E> ...@@ -121,7 +122,7 @@ public abstract class SelectableItemViewBase<E>
@Override @Override
protected void onDetachedFromWindow() { protected void onDetachedFromWindow() {
super.onDetachedFromWindow(); super.onDetachedFromWindow();
setChecked(false); resetCheckedState();
} }
// OnTouchListener implementation. // OnTouchListener implementation.
...@@ -187,7 +188,7 @@ public abstract class SelectableItemViewBase<E> ...@@ -187,7 +188,7 @@ public abstract class SelectableItemViewBase<E>
// Checkable implementations. // Checkable implementations.
@Override @Override
public boolean isChecked() { public boolean isChecked() {
return mIsChecked; return mIsChecked != null && mIsChecked;
} }
@Override @Override
...@@ -195,11 +196,28 @@ public abstract class SelectableItemViewBase<E> ...@@ -195,11 +196,28 @@ public abstract class SelectableItemViewBase<E>
setChecked(!isChecked()); setChecked(!isChecked());
} }
/**
* Sets whether the item is checked. Note that if the views to be updated run animations, you
* should override {@link #updateView(boolean)} to get the correct animation state instead of
* overriding this method to update the views.
* @param checked Whether the item is checked.
*/
@Override @Override
public void setChecked(boolean checked) { public void setChecked(boolean checked) {
if (checked == mIsChecked) return; if (mIsChecked != null && checked == mIsChecked) return;
// We shouldn't run the animation when mIsChecked is first initialized to the correct state.
final boolean animate = mIsChecked != null;
mIsChecked = checked; mIsChecked = checked;
updateView(); updateView(animate);
}
/**
* Resets the checked state to be uninitialized.
*/
private void resetCheckedState() {
setChecked(false);
mIsChecked = null;
} }
// SelectionObserver implementation. // SelectionObserver implementation.
...@@ -210,8 +228,9 @@ public abstract class SelectableItemViewBase<E> ...@@ -210,8 +228,9 @@ public abstract class SelectableItemViewBase<E>
/** /**
* Update the view based on whether this item is selected. * Update the view based on whether this item is selected.
* @param animate Whether to animate the selection state changing if applicable.
*/ */
protected void updateView() {} protected void updateView(boolean animate) {}
/** /**
* Same as {@link OnClickListener#onClick(View)} on this. * Same as {@link OnClickListener#onClick(View)} on this.
......
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