From e8de94a21dde56dfcc60d2caf6706d8e5b666849 Mon Sep 17 00:00:00 2001 From: Arc Wang Date: Mon, 28 Jun 2021 11:33:10 +0800 Subject: [PATCH] Fix 'No Apps' UI issues of ManageApplications Fixes below UI issues - "No Apps" may not show in fragments of profile tab. Fix it by using ConstraintLayout to specify alignments of each view and removing extra padding. -- "No Apps" may flicker by moving position. The flicker is from unnecessary visibility changes. This change integrates empty view visibility control in LoadingViewController to simplify code and avoid unnecessary visibility changes. Bug: 189390795 Bug: 183398721 Test: atest com.android.settings.deviceinfo make RunSettingsRoboTests -j ROBOTEST_FILTER=com.android.settings.deviceinfo Manual visual, observe UI Settings -> Storage -> Games Settings -> Notifications -> App Settings Settings > Apps > Special app access > Media management apps Change-Id: I634209c6f8466e2adae703226902190bbdf470b9 --- res/layout/manage_applications_apps.xml | 62 ++++++++--------- res/values/dimens.xml | 2 - .../applications/RunningServices.java | 6 +- .../ManageApplications.java | 52 +++++--------- .../widget/LoadingViewController.java | 68 ++++++++++++++++--- .../ManageApplicationsTest.java | 28 +------- 6 files changed, 115 insertions(+), 103 deletions(-) diff --git a/res/layout/manage_applications_apps.xml b/res/layout/manage_applications_apps.xml index d814164647a..7ea88038734 100644 --- a/res/layout/manage_applications_apps.xml +++ b/res/layout/manage_applications_apps.xml @@ -14,7 +14,7 @@ limitations under the License. --> - + android:elevation="2dp" + settings:layout_constraintTop_toTopOf="parent"/> - + android:layout_height="wrap_content" + android:clipToPadding="false" + android:scrollbars="none" + android:visibility="invisible" + settings:fastScrollEnabled="true" + settings:fastScrollHorizontalThumbDrawable="@drawable/thumb_drawable" + settings:fastScrollHorizontalTrackDrawable="@drawable/line_drawable" + settings:fastScrollVerticalThumbDrawable="@drawable/thumb_drawable" + settings:fastScrollVerticalTrackDrawable="@drawable/line_drawable" + settings:layout_constraintTop_toBottomOf="@id/pinned_header"/> - + - - - - - - - + + diff --git a/res/values/dimens.xml b/res/values/dimens.xml index 908b6401aed..33fe1f572ac 100755 --- a/res/values/dimens.xml +++ b/res/values/dimens.xml @@ -149,8 +149,6 @@ 182dp 32dp 24dp - - 80dp 88dip diff --git a/src/com/android/settings/applications/RunningServices.java b/src/com/android/settings/applications/RunningServices.java index 4d13241126f..b1689d5c591 100644 --- a/src/com/android/settings/applications/RunningServices.java +++ b/src/com/android/settings/applications/RunningServices.java @@ -72,7 +72,11 @@ public class RunningServices extends SettingsPreferenceFragment { public void onResume() { super.onResume(); boolean haveData = mRunningProcessesView.doResume(this, mRunningProcessesAvail); - mLoadingViewController.handleLoadingContainer(haveData /* done */, false /* animate */); + if (haveData) { + mLoadingViewController.showContent(false /* animate */); + } else { + mLoadingViewController.showLoadingView(); + } } @Override diff --git a/src/com/android/settings/applications/manageapplications/ManageApplications.java b/src/com/android/settings/applications/manageapplications/ManageApplications.java index 6d675240b64..43e929b8054 100644 --- a/src/com/android/settings/applications/manageapplications/ManageApplications.java +++ b/src/com/android/settings/applications/manageapplications/ManageApplications.java @@ -208,7 +208,6 @@ public class ManageApplications extends InstrumentedFragment private ApplicationsAdapter mApplications; private View mLoadingContainer; - private View mListContainer; private SearchView mSearchView; // Size resource used for packages whose size computation failed for some reason @@ -402,25 +401,21 @@ public class ManageApplications extends InstrumentedFragment mRootView = inflater.inflate(R.layout.manage_applications_apps, null); mLoadingContainer = mRootView.findViewById(R.id.loading_container); - mListContainer = mRootView.findViewById(R.id.list_container); - if (mListContainer != null) { - // Create adapter and list view here - mEmptyView = mListContainer.findViewById(android.R.id.empty); + mEmptyView = mRootView.findViewById(android.R.id.empty); + mRecyclerView = mRootView.findViewById(R.id.apps_list); - mApplications = new ApplicationsAdapter(mApplicationsState, this, mFilter, - savedInstanceState); - if (savedInstanceState != null) { - mApplications.mHasReceivedLoadEntries = - savedInstanceState.getBoolean(EXTRA_HAS_ENTRIES, false); - mApplications.mHasReceivedBridgeCallback = - savedInstanceState.getBoolean(EXTRA_HAS_BRIDGE, false); - } - mRecyclerView = mListContainer.findViewById(R.id.apps_list); - mRecyclerView.setItemAnimator(null); - mRecyclerView.setLayoutManager(new LinearLayoutManager( - getContext(), RecyclerView.VERTICAL, false /* reverseLayout */)); - mRecyclerView.setAdapter(mApplications); + mApplications = new ApplicationsAdapter(mApplicationsState, this, mFilter, + savedInstanceState); + if (savedInstanceState != null) { + mApplications.mHasReceivedLoadEntries = + savedInstanceState.getBoolean(EXTRA_HAS_ENTRIES, false); + mApplications.mHasReceivedBridgeCallback = + savedInstanceState.getBoolean(EXTRA_HAS_BRIDGE, false); } + mRecyclerView.setItemAnimator(null); + mRecyclerView.setLayoutManager(new LinearLayoutManager( + getContext(), RecyclerView.VERTICAL, false /* reverseLayout */)); + mRecyclerView.setAdapter(mApplications); // We have to do this now because PreferenceFrameLayout looks at it // only when the view is added. @@ -985,16 +980,8 @@ public class ManageApplications extends InstrumentedFragment // overlapped by floating filter. if (hasFilter) { mManageApplications.mSpinnerHeader.setVisibility(View.VISIBLE); - mManageApplications.mRecyclerView.setPadding(0 /* left */, - mContext.getResources().getDimensionPixelSize( - R.dimen.app_bar_height) /* top */, - 0 /* right */, - 0 /* bottom */); } else { mManageApplications.mSpinnerHeader.setVisibility(View.GONE); - mManageApplications.mRecyclerView.setPadding(0 /* left */, 0 /* top */, - 0 /* right */, - 0 /* bottom */); } } } @@ -1044,7 +1031,8 @@ public class ManageApplications extends InstrumentedFragment mManageApplications = manageApplications; mLoadingViewController = new LoadingViewController( mManageApplications.mLoadingContainer, - mManageApplications.mListContainer + mManageApplications.mRecyclerView, + mManageApplications.mEmptyView ); mContext = manageApplications.getActivity(); mIconDrawableFactory = IconDrawableFactory.newInstance(mContext); @@ -1303,11 +1291,9 @@ public class ManageApplications extends InstrumentedFragment mOriginalEntries = entries; notifyDataSetChanged(); if (getItemCount() == 0) { - mManageApplications.mRecyclerView.setVisibility(View.GONE); - mManageApplications.mEmptyView.setVisibility(View.VISIBLE); + mLoadingViewController.showEmpty(false /* animate */); } else { - mManageApplications.mEmptyView.setVisibility(View.GONE); - mManageApplications.mRecyclerView.setVisibility(View.VISIBLE); + mLoadingViewController.showContent(false /* animate */); if (mManageApplications.mSearchView != null && mManageApplications.mSearchView.isVisibleToUser()) { @@ -1324,10 +1310,6 @@ public class ManageApplications extends InstrumentedFragment mLastIndex = -1; } - if (mSession.getAllApps().size() != 0 - && mManageApplications.mListContainer.getVisibility() != View.VISIBLE) { - mLoadingViewController.showContent(true /* animate */); - } if (mManageApplications.mListType == LIST_TYPE_USAGE_ACCESS) { // No enabled or disabled filters for usage access. return; diff --git a/src/com/android/settings/widget/LoadingViewController.java b/src/com/android/settings/widget/LoadingViewController.java index 294e55e7ea8..66eebf387ba 100644 --- a/src/com/android/settings/widget/LoadingViewController.java +++ b/src/com/android/settings/widget/LoadingViewController.java @@ -22,34 +22,66 @@ import android.view.View; import android.view.animation.Animation; import android.view.animation.AnimationUtils; +import androidx.annotation.Nullable; + /** - * A helper class that manages show/hide loading spinner. + * A helper class that manages show/hide loading spinner, content view and empty view (optional). */ public class LoadingViewController { private static final long DELAY_SHOW_LOADING_CONTAINER_THRESHOLD_MS = 100L; - public final Handler mFgHandler; - public final View mLoadingView; - public final View mContentView; + private final Handler mFgHandler; + private final View mLoadingView; + private final View mContentView; + private final View mEmptyView; public LoadingViewController(View loadingView, View contentView) { + this(loadingView, contentView, null /* emptyView*/); + } + + public LoadingViewController(View loadingView, View contentView, @Nullable View emptyView) { mLoadingView = loadingView; mContentView = contentView; + mEmptyView = emptyView; mFgHandler = new Handler(Looper.getMainLooper()); } private Runnable mShowLoadingContainerRunnable = new Runnable() { public void run() { - handleLoadingContainer(false /* done */, false /* animate */); + showLoadingView(); } }; + /** + * Shows content view and hides loading view & empty view. + */ public void showContent(boolean animate) { // Cancel any pending task to show the loading animation and show the list of // apps directly. mFgHandler.removeCallbacks(mShowLoadingContainerRunnable); - handleLoadingContainer(true /* show */, animate); + handleLoadingContainer(true /* showContent */, false /* showEmpty*/, animate); + } + + /** + * Shows empty view and hides loading view & content view. + */ + public void showEmpty(boolean animate) { + if (mEmptyView == null) { + return; + } + + // Cancel any pending task to show the loading animation and show the list of + // apps directly. + mFgHandler.removeCallbacks(mShowLoadingContainerRunnable); + handleLoadingContainer(false /* showContent */, true /* showEmpty */, animate); + } + + /** + * Shows loading view and hides content view & empty view. + */ + public void showLoadingView() { + handleLoadingContainer(false /* showContent */, false /* showEmpty */, false /* animate */); } public void showLoadingViewDelayed() { @@ -57,8 +89,9 @@ public class LoadingViewController { mShowLoadingContainerRunnable, DELAY_SHOW_LOADING_CONTAINER_THRESHOLD_MS); } - public void handleLoadingContainer(boolean done, boolean animate) { - handleLoadingContainer(mLoadingView, mContentView, done, animate); + private void handleLoadingContainer(boolean showContent, boolean showEmpty, boolean animate) { + handleLoadingContainer(mLoadingView, mContentView, mEmptyView, + showContent, showEmpty, animate); } /** @@ -75,6 +108,25 @@ public class LoadingViewController { setViewShown(content, done, animate); } + /** + * Show/hide loading view and content view and empty view. + * + * @param loading The loading spinner view + * @param content The content view + * @param empty The empty view shows no item summary to users. + * @param showContent If true, content is set visible and loading is set invisible. + * @param showEmpty If true, empty is set visible and loading is set invisible. + * @param animate Whether or not content/loading views should animate in/out. + */ + public static void handleLoadingContainer(View loading, View content, View empty, + boolean showContent, boolean showEmpty, boolean animate) { + if (empty != null) { + setViewShown(empty, showEmpty, animate); + } + setViewShown(content, showContent, animate); + setViewShown(loading, !showContent && !showEmpty, animate); + } + private static void setViewShown(final View view, boolean shown, boolean animate) { if (animate) { Animation animation = AnimationUtils.loadAnimation(view.getContext(), 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 86f5fe83c6c..25eca7adb22 100644 --- a/tests/robotests/src/com/android/settings/applications/manageapplications/ManageApplicationsTest.java +++ b/tests/robotests/src/com/android/settings/applications/manageapplications/ManageApplicationsTest.java @@ -28,7 +28,6 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -49,7 +48,6 @@ import android.view.Menu; import android.view.MenuInflater; import android.view.MenuItem; import android.view.View; -import android.view.ViewGroup; import android.widget.SearchView; import androidx.fragment.app.FragmentActivity; @@ -155,22 +153,6 @@ public class ManageApplicationsTest { assertThat(mMenu.findItem(R.id.sort_order_frequent_notification).isVisible()).isFalse(); } - @Test - public void onCreateView_shouldNotShowLoadingContainer() { - ReflectionHelpers.setField(mFragment, "mResetAppsHelper", mock(ResetAppsHelper.class)); - doNothing().when(mFragment).createHeader(); - - final LayoutInflater layoutInflater = mock(LayoutInflater.class); - final View view = mock(View.class); - final View loadingContainer = mock(View.class); - when(layoutInflater.inflate(anyInt(), eq(null))).thenReturn(view); - when(view.findViewById(R.id.loading_container)).thenReturn(loadingContainer); - - mFragment.onCreateView(layoutInflater, mock(ViewGroup.class), null); - - verify(loadingContainer, never()).setVisibility(View.VISIBLE); - } - @Test public void onCreateOptionsMenu_shouldSetSearchQueryListener() { final SearchView searchView = mock(SearchView.class); @@ -221,7 +203,6 @@ public class ManageApplicationsTest { @Test public void updateLoading_appLoaded_shouldNotDelayCallToHandleLoadingContainer() { ReflectionHelpers.setField(mFragment, "mLoadingContainer", mock(View.class)); - ReflectionHelpers.setField(mFragment, "mListContainer", mock(View.class)); final ManageApplications.ApplicationsAdapter adapter = spy(new ManageApplications.ApplicationsAdapter(mState, mFragment, AppFilterRegistry.getInstance().get(FILTER_APPS_ALL), new Bundle())); @@ -243,7 +224,6 @@ public class ManageApplicationsTest { @Test public void updateLoading_appNotLoaded_shouldDelayCallToHandleLoadingContainer() { ReflectionHelpers.setField(mFragment, "mLoadingContainer", mock(View.class)); - ReflectionHelpers.setField(mFragment, "mListContainer", mock(View.class)); final ManageApplications.ApplicationsAdapter adapter = spy(new ManageApplications.ApplicationsAdapter(mState, mFragment, AppFilterRegistry.getInstance().get(FILTER_APPS_ALL), new Bundle())); @@ -272,7 +252,6 @@ public class ManageApplicationsTest { when(listContainer.getVisibility()).thenReturn(View.INVISIBLE); when(listContainer.getContext()).thenReturn(context); ReflectionHelpers.setField(mFragment, "mLoadingContainer", loadingContainer); - ReflectionHelpers.setField(mFragment, "mListContainer", listContainer); final ManageApplications.ApplicationsAdapter adapter = spy(new ManageApplications.ApplicationsAdapter(mState, mFragment, AppFilterRegistry.getInstance().get(FILTER_APPS_ALL), new Bundle())); @@ -296,7 +275,7 @@ public class ManageApplicationsTest { adapter.onRebuildComplete(null); - verify(loadingViewController).showContent(true /* animate */); + verify(loadingViewController).showEmpty(false /* animate */); } @Test @@ -304,15 +283,16 @@ public class ManageApplicationsTest { final String query = "Test"; final RecyclerView recyclerView = mock(RecyclerView.class); final View emptyView = mock(View.class); + final View loadingContainer = mock(View.class); ReflectionHelpers.setField(mFragment, "mRecyclerView", recyclerView); ReflectionHelpers.setField(mFragment, "mEmptyView", emptyView); + ReflectionHelpers.setField(mFragment, "mLoadingContainer", loadingContainer); final SearchView searchView = mock(SearchView.class); ReflectionHelpers.setField(mFragment, "mSearchView", searchView); when(searchView.isVisibleToUser()).thenReturn(true); when(searchView.getQuery()).thenReturn(query); final View listContainer = mock(View.class); when(listContainer.getVisibility()).thenReturn(View.VISIBLE); - ReflectionHelpers.setField(mFragment, "mListContainer", listContainer); ReflectionHelpers.setField( mFragment, "mFilterAdapter", mock(ManageApplications.FilterSpinnerAdapter.class)); final ArrayList appList = new ArrayList<>(); @@ -491,8 +471,6 @@ public class ManageApplicationsTest { mFragment.mFilterAdapter.updateFilterView(true); assertThat(mFragment.mSpinnerHeader.getVisibility()).isEqualTo(View.VISIBLE); - assertThat(mFragment.mRecyclerView.getPaddingTop()).isEqualTo( - mContext.getResources().getDimensionPixelSize(R.dimen.app_bar_height)); } @Test