From 764c21b20791f68e3b3b9befe016cbcc615ce467 Mon Sep 17 00:00:00 2001 From: Andy Wickham Date: Fri, 24 Mar 2023 17:22:38 -0700 Subject: [PATCH] Don't return early if searching during rebindAdapters(). This was a rare case (made less rare by rocket gesture) which was not actually updating the UI correctly. Expected flow: 1. All Apps is inflated with a single recyclerview for apps (as defined in xml). 2. Later, rebindAdapters() is called, and if there are work apps, the recyclerview is removed and replaced by a viewpager with 2 children recyclerviews (one for personal and one for work). 3. At any point if you start searching, the app rv or viewpager is hidden and the search rv is shown. Actual flow in the error case: - Same as above, but if you were searching when 2 happens, we returned early, so we never replaced the app rv with the viewpager, so all the apps were dumped in the single rv, and the header with tabs showed above it. Fix: 272575605 Test: Manually force first rebind ta happen while searching, and verify this bug was hit before the fix but not after. Change-Id: I25b8991564645368840a390733aa893dee4cd10e --- .../allapps/ActivityAllAppsContainerView.java | 38 ++++++++++++------- .../allapps/AllAppsTransitionController.java | 1 - .../launcher3/allapps/FloatingHeaderView.java | 8 +++- .../allapps/SearchTransitionController.java | 1 + 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/com/android/launcher3/allapps/ActivityAllAppsContainerView.java b/src/com/android/launcher3/allapps/ActivityAllAppsContainerView.java index c5cf647156..66417d399a 100644 --- a/src/com/android/launcher3/allapps/ActivityAllAppsContainerView.java +++ b/src/com/android/launcher3/allapps/ActivityAllAppsContainerView.java @@ -380,7 +380,23 @@ public class ActivityAllAppsContainerView return rv.shouldContainerScroll(ev, dragLayer); } + /** + * Resets the UI to be ready for fresh interactions in the future. Exits search and returns to + * A-Z apps list. + * + * @param animate Whether to animate the header during the reset (e.g. switching profile tabs). + **/ public void reset(boolean animate) { + reset(animate, true); + } + + /** + * Resets the UI to be ready for fresh interactions in the future. + * + * @param animate Whether to animate the header during the reset (e.g. switching profile tabs). + * @param exitSearch Whether to force exit the search state and return to A-Z apps list. + **/ + public void reset(boolean animate, boolean exitSearch) { for (int i = 0; i < mAH.size(); i++) { if (mAH.get(i).mRecyclerView != null) { mAH.get(i).mRecyclerView.scrollToTop(); @@ -394,10 +410,12 @@ public class ActivityAllAppsContainerView } // Reset the base recycler view after transitioning home. updateHeaderScroll(0); - // Reset the search bar after transitioning home. - mSearchUiManager.resetSearch(); - // Animate to A-Z with 0 time to reset the animation with proper state management. - animateToSearchState(false, 0); + if (exitSearch) { + // Reset the search bar after transitioning home. + mSearchUiManager.resetSearch(); + // Animate to A-Z with 0 time to reset the animation with proper state management. + animateToSearchState(false, 0); + } } @Override @@ -441,7 +459,7 @@ public class ActivityAllAppsContainerView } // Header keeps track of active recycler view to properly render header protection. mHeader.setActiveRV(currentActivePage); - reset(true /* animate */); + reset(true /* animate */, !isSearching() /* exitSearch */); mWorkManager.onActivePageChanged(currentActivePage); } @@ -462,12 +480,6 @@ public class ActivityAllAppsContainerView return; } - if (isSearching()) { - mUsingTabs = showTabs; - mWorkManager.detachWorkModeSwitch(); - return; - } - if (!FeatureFlags.ENABLE_SEARCH_RESULT_BACKGROUND_DRAWABLES.get()) { RecyclerView.ItemDecoration decoration = getMainAdapterProvider().getDecorator(); getSearchRecyclerView().removeItemDecoration(decoration); @@ -532,7 +544,7 @@ public class ActivityAllAppsContainerView mAllAppsStore.registerIconContainer(mAH.get(AdapterHolder.SEARCH).mRecyclerView); } - protected View replaceAppsRVContainer(boolean showTabs) { + private void replaceAppsRVContainer(boolean showTabs) { for (int i = AdapterHolder.MAIN; i <= AdapterHolder.WORK; i++) { AdapterHolder adapterHolder = mAH.get(i); if (adapterHolder.mRecyclerView != null) { @@ -586,7 +598,7 @@ public class ActivityAllAppsContainerView layoutBelowSearchContainer(getSearchRecyclerView(), /* tabs= */ false); } - return rvContainer; + updateSearchResultsVisibility(); } void setupHeader() { diff --git a/src/com/android/launcher3/allapps/AllAppsTransitionController.java b/src/com/android/launcher3/allapps/AllAppsTransitionController.java index 85d7a05406..4d1006adba 100644 --- a/src/com/android/launcher3/allapps/AllAppsTransitionController.java +++ b/src/com/android/launcher3/allapps/AllAppsTransitionController.java @@ -521,7 +521,6 @@ public class AllAppsTransitionController */ private void onProgressAnimationEnd() { if (Float.compare(mProgress, 1f) == 0) { - mAppsView.reset(false /* animate */); if (mShouldControlKeyboard) { mLauncher.getAppsView().getSearchUiManager().getEditText().hideKeyboard(); } diff --git a/src/com/android/launcher3/allapps/FloatingHeaderView.java b/src/com/android/launcher3/allapps/FloatingHeaderView.java index b3ea3ab225..330d13da06 100644 --- a/src/com/android/launcher3/allapps/FloatingHeaderView.java +++ b/src/com/android/launcher3/allapps/FloatingHeaderView.java @@ -228,7 +228,7 @@ public class FloatingHeaderView extends LinearLayout implements updateExpectedHeight(); mTabsHidden = tabsHidden; - mTabLayout.setVisibility(tabsHidden ? View.GONE : View.VISIBLE); + maybeSetTabVisibility(VISIBLE); mMainRV = mainRV; mWorkRV = workRV; mSearchRV = searchRV; @@ -250,6 +250,12 @@ public class FloatingHeaderView extends LinearLayout implements rvType == AdapterHolder.MAIN ? mMainRV : rvType == AdapterHolder.WORK ? mWorkRV : mSearchRV; mCurrentRV.addOnScrollListener(mOnScrollListener); + maybeSetTabVisibility(rvType == AdapterHolder.SEARCH ? GONE : VISIBLE); + } + + /** Update tab visibility to the given state, only if tabs are active (work profile exists). */ + void maybeSetTabVisibility(int visibility) { + mTabLayout.setVisibility(mTabsHidden ? GONE : visibility); } private void updateExpectedHeight() { diff --git a/src/com/android/launcher3/allapps/SearchTransitionController.java b/src/com/android/launcher3/allapps/SearchTransitionController.java index 50567822a6..de653023cd 100644 --- a/src/com/android/launcher3/allapps/SearchTransitionController.java +++ b/src/com/android/launcher3/allapps/SearchTransitionController.java @@ -125,6 +125,7 @@ public class SearchTransitionController { mAllAppsContainerView.getFloatingHeaderView().setFloatingRowsCollapsed(true); mAllAppsContainerView.getFloatingHeaderView().setVisibility(VISIBLE); + mAllAppsContainerView.getFloatingHeaderView().maybeSetTabVisibility(VISIBLE); mAllAppsContainerView.getAppsRecyclerViewContainer().setVisibility(VISIBLE); getSearchRecyclerView().setVisibility(VISIBLE); getSearchRecyclerView().setChildAttachedConsumer(this::onSearchChildAttached);