From de052eb2b9222161769da4ac2004501194eb9af3 Mon Sep 17 00:00:00 2001 From: Fan Zhang Date: Fri, 23 Mar 2018 17:03:38 -0700 Subject: [PATCH] Fix text overlap issue when flinging in ManageApplication The issue is we have a background handler to find app size and set to summary asynchronously. When flinging quickly, the view being request to update by the background handler could be scrolled off screen already. This change forces onPackageSizeChanged update to only happen when it's not scrolling. Change-Id: Ia7ccab776c3c789c8d4c0b55104b48e257b9cebf Fixes: 76176014 Test: manually fling --- .../ManageApplications.java | 56 +++++++++++++++++-- .../ManageApplicationsTest.java | 36 ++++++++++++ 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/src/com/android/settings/applications/manageapplications/ManageApplications.java b/src/com/android/settings/applications/manageapplications/ManageApplications.java index d144169af2c..6521056d800 100644 --- a/src/com/android/settings/applications/manageapplications/ManageApplications.java +++ b/src/com/android/settings/applications/manageapplications/ManageApplications.java @@ -16,6 +16,7 @@ package com.android.settings.applications.manageapplications; +import static android.support.v7.widget.RecyclerView.SCROLL_STATE_IDLE; import static com.android.settings.applications.manageapplications.AppFilterRegistry .FILTER_APPS_ALL; import static com.android.settings.applications.manageapplications.AppFilterRegistry @@ -50,6 +51,7 @@ import android.os.Environment; import android.os.UserHandle; import android.os.UserManager; import android.preference.PreferenceFrameLayout; +import android.support.annotation.NonNull; import android.support.annotation.VisibleForTesting; import android.support.v7.widget.LinearLayoutManager; import android.support.v7.widget.RecyclerView; @@ -844,11 +846,16 @@ public class ManageApplications extends InstrumentedFragment private boolean mHasReceivedBridgeCallback; private FileViewHolderController mExtraViewController; - // These two variables are used to remember and restore the last scroll position when this + // This is to remember and restore the last scroll position when this // fragment is paused. We need this special handling because app entries are added gradually // when we rebuild the list after the user made some changes, like uninstalling an app. private int mLastIndex = -1; + @VisibleForTesting + OnScrollListener mOnScrollListener; + private RecyclerView mRecyclerView; + + public ApplicationsAdapter(ApplicationsState state, ManageApplications manageApplications, AppFilterItem appFilter, Bundle savedInstanceState) { setHasStableIds(true); @@ -886,6 +893,22 @@ public class ManageApplications extends InstrumentedFragment } } + @Override + public void onAttachedToRecyclerView(@NonNull RecyclerView recyclerView) { + super.onAttachedToRecyclerView(recyclerView); + mRecyclerView = recyclerView; + mOnScrollListener = new OnScrollListener(this); + mRecyclerView.addOnScrollListener(mOnScrollListener); + } + + @Override + public void onDetachedFromRecyclerView(@NonNull RecyclerView recyclerView) { + super.onDetachedFromRecyclerView(recyclerView); + mRecyclerView.removeOnScrollListener(mOnScrollListener); + mOnScrollListener = null; + mRecyclerView = null; + } + public void setCompositeFilter(AppFilter compositeFilter) { mCompositeFilter = compositeFilter; rebuild(); @@ -1180,9 +1203,8 @@ public class ManageApplications extends InstrumentedFragment rebuild(); return; } else { - notifyItemChanged(i); + mOnScrollListener.postNotifyItemChange(i); } - } } @@ -1310,13 +1332,39 @@ public class ManageApplications extends InstrumentedFragment return mExtraViewController != null && mExtraViewController.shouldShow(); } + + public static class OnScrollListener extends RecyclerView.OnScrollListener { + private int mScrollState = SCROLL_STATE_IDLE; + private boolean mDelayNotifyDataChange; + private ApplicationsAdapter mAdapter; + + public OnScrollListener(ApplicationsAdapter adapter) { + mAdapter = adapter; + } + + @Override + public void onScrollStateChanged(@NonNull RecyclerView recyclerView, int newState) { + mScrollState = newState; + if (mScrollState == SCROLL_STATE_IDLE && mDelayNotifyDataChange) { + mDelayNotifyDataChange = false; + mAdapter.notifyDataSetChanged(); + } + } + + public void postNotifyItemChange(int index) { + if (mScrollState == SCROLL_STATE_IDLE) { + mAdapter.notifyItemChanged(index); + } else { + mDelayNotifyDataChange = true; + } + } + } } private static class SummaryProvider implements SummaryLoader.SummaryProvider { private final Context mContext; private final SummaryLoader mLoader; - private ApplicationsState.Session mSession; private SummaryProvider(Context context, SummaryLoader loader) { mContext = context; diff --git a/tests/robotests/src/com/android/settings/applications/manageapplications/ManageApplicationsTest.java b/tests/robotests/src/com/android/settings/applications/manageapplications/ManageApplicationsTest.java index f60c7b0733f..fb7b59d1efa 100644 --- a/tests/robotests/src/com/android/settings/applications/manageapplications/ManageApplicationsTest.java +++ b/tests/robotests/src/com/android/settings/applications/manageapplications/ManageApplicationsTest.java @@ -16,6 +16,8 @@ package com.android.settings.applications.manageapplications; +import static android.support.v7.widget.RecyclerView.SCROLL_STATE_DRAGGING; +import static android.support.v7.widget.RecyclerView.SCROLL_STATE_IDLE; import static com.android.settings.applications.manageapplications.AppFilterRegistry.FILTER_APPS_ALL; import static com.android.settings.applications.manageapplications.ManageApplications.LIST_TYPE_MAIN; import static com.android.settings.applications.manageapplications.ManageApplications.LIST_TYPE_NOTIFICATION; @@ -229,6 +231,40 @@ public class ManageApplicationsTest { verify(loadingViewController).showContent(true /* animate */); } + @Test + public void notifyItemChange_recyclerViewIdle_shouldNotify() { + final RecyclerView recyclerView = mock(RecyclerView.class); + final ManageApplications.ApplicationsAdapter adapter = + spy(new ManageApplications.ApplicationsAdapter(mState, + mock(ManageApplications.class), + AppFilterRegistry.getInstance().get(FILTER_APPS_ALL), new Bundle())); + + adapter.onAttachedToRecyclerView(recyclerView); + adapter.mOnScrollListener.onScrollStateChanged(recyclerView, SCROLL_STATE_IDLE); + adapter.mOnScrollListener.postNotifyItemChange(0 /* index */); + + verify(adapter).notifyItemChanged(0); + } + + @Test + public void notifyItemChange_recyclerViewScrolling_shouldNotifyWhenIdle() { + final RecyclerView recyclerView = mock(RecyclerView.class); + final ManageApplications.ApplicationsAdapter adapter = + spy(new ManageApplications.ApplicationsAdapter(mState, + mock(ManageApplications.class), + AppFilterRegistry.getInstance().get(FILTER_APPS_ALL), new Bundle())); + + adapter.onAttachedToRecyclerView(recyclerView); + adapter.mOnScrollListener.onScrollStateChanged(recyclerView, SCROLL_STATE_DRAGGING); + adapter.mOnScrollListener.postNotifyItemChange(0 /* index */); + + verify(adapter, never()).notifyItemChanged(0); + verify(adapter, never()).notifyDataSetChanged(); + + adapter.mOnScrollListener.onScrollStateChanged(recyclerView, SCROLL_STATE_IDLE); + verify(adapter).notifyDataSetChanged(); + } + private void setUpOptionMenus() { when(mMenu.findItem(anyInt())).thenAnswer(invocation -> { final Object[] args = invocation.getArguments();