From 228bc78bf9295b51bc1bb6fce46e56aaa1ca49c1 Mon Sep 17 00:00:00 2001
From: Yi-Ling Chuang <emilychuang@google.com>
Date: Tue, 11 Aug 2020 16:03:02 +0800
Subject: [PATCH] [DO NOT MERGE] Pre-allocate height for contextual cards.

To prevent the UI jank causing by the async card loads, we pre-allocate
some space for the card to fill in. After this change, only one card can
be shown at a time.

More details:
- When the card number configuration is set to 0, don't trigger the card
loader.
- The height adjusting logic is as follows.
When Settings is opened, pre-allocate a space first.
After the RV finish laying out, reset the RV to wrap_content. So if the
card has to be expanded(eg. wifi large mode or dismissal view), then it
will adjust the height accordingly. While if a card previously shown
becomes unavailable(dismissed or conditions not meet), we also reset the
RV so the space can be gone.

Bug: 163288869
Test: robotest
Change-Id: I0dcb2dae8f0533e562ad06f664b7ae7a9afecd21
---
 res/values/dimens.xml                         |  1 +
 .../contextualcards/ContextualCardLoader.java |  9 +++--
 .../ContextualCardManager.java                |  4 +++
 .../ContextualCardsAdapter.java               | 20 ++++++++++-
 .../ContextualCardsFragment.java              | 36 ++++++++++++++++++-
 .../ContextualCardLoaderTest.java             | 12 +++----
 6 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/res/values/dimens.xml b/res/values/dimens.xml
index dc23c3d8d06..28ab8f8323e 100755
--- a/res/values/dimens.xml
+++ b/res/values/dimens.xml
@@ -348,6 +348,7 @@
     <dimen name="contextual_half_card_padding_top">12dp</dimen>
     <dimen name="contextual_half_card_padding_bottom">16dp</dimen>
     <dimen name="contextual_half_card_title_margin_top">12dp</dimen>
+    <dimen name="contextual_card_preallocated_height">0dp</dimen>
 
     <!-- Homepage dismissal cards size and padding -->
     <dimen name="contextual_card_dismissal_margin_top">12dp</dimen>
diff --git a/src/com/android/settings/homepage/contextualcards/ContextualCardLoader.java b/src/com/android/settings/homepage/contextualcards/ContextualCardLoader.java
index e4a644cf780..de7eda2197f 100644
--- a/src/com/android/settings/homepage/contextualcards/ContextualCardLoader.java
+++ b/src/com/android/settings/homepage/contextualcards/ContextualCardLoader.java
@@ -50,7 +50,7 @@ import java.util.stream.Collectors;
 public class ContextualCardLoader extends AsyncLoaderCompat<List<ContextualCard>> {
 
     @VisibleForTesting
-    static final int DEFAULT_CARD_COUNT = 3;
+    static final int DEFAULT_CARD_COUNT = 1;
     @VisibleForTesting
     static final String CONTEXTUAL_CARD_COUNT = "contextual_card_count";
     static final int CARD_CONTENT_LOADER_ID = 1;
@@ -131,7 +131,7 @@ public class ContextualCardLoader extends AsyncLoaderCompat<List<ContextualCard>
         final List<ContextualCard> visibleCards = new ArrayList<>();
         final List<ContextualCard> hiddenCards = new ArrayList<>();
 
-        final int maxCardCount = getCardCount();
+        final int maxCardCount = getCardCount(mContext);
         eligibleCards.forEach(card -> {
             if (card.getCategory() != STICKY_VALUE) {
                 return;
@@ -167,11 +167,10 @@ public class ContextualCardLoader extends AsyncLoaderCompat<List<ContextualCard>
         return visibleCards;
     }
 
-    @VisibleForTesting
-    int getCardCount() {
+    static int getCardCount(Context context) {
         // Return the card count if Settings.Global has KEY_CONTEXTUAL_CARD_COUNT key,
         // otherwise return the default one.
-        return Settings.Global.getInt(mContext.getContentResolver(),
+        return Settings.Global.getInt(context.getContentResolver(),
                 CONTEXTUAL_CARD_COUNT, DEFAULT_CARD_COUNT);
     }
 
diff --git a/src/com/android/settings/homepage/contextualcards/ContextualCardManager.java b/src/com/android/settings/homepage/contextualcards/ContextualCardManager.java
index ac35017baab..00755296d0a 100644
--- a/src/com/android/settings/homepage/contextualcards/ContextualCardManager.java
+++ b/src/com/android/settings/homepage/contextualcards/ContextualCardManager.java
@@ -122,6 +122,10 @@ public class ContextualCardManager implements ContextualCardLoader.CardContentLo
             Log.w(TAG, "Legacy suggestion contextual card enabled, skipping contextual cards.");
             return;
         }
+        if (ContextualCardLoader.getCardCount(mContext) <= 0) {
+            Log.w(TAG, "Card count is zero, skipping contextual cards.");
+            return;
+        }
         mStartTime = System.currentTimeMillis();
         final CardContentLoaderCallbacks cardContentLoaderCallbacks =
                 new CardContentLoaderCallbacks(mContext);
diff --git a/src/com/android/settings/homepage/contextualcards/ContextualCardsAdapter.java b/src/com/android/settings/homepage/contextualcards/ContextualCardsAdapter.java
index b9bc43b32b5..cf6f53c7075 100644
--- a/src/com/android/settings/homepage/contextualcards/ContextualCardsAdapter.java
+++ b/src/com/android/settings/homepage/contextualcards/ContextualCardsAdapter.java
@@ -16,7 +16,10 @@
 
 package com.android.settings.homepage.contextualcards;
 
+import static android.view.ViewGroup.LayoutParams.WRAP_CONTENT;
+
 import android.content.Context;
+import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
@@ -131,7 +134,22 @@ public class ContextualCardsAdapter extends RecyclerView.Adapter<RecyclerView.Vi
             diffResult.dispatchUpdatesTo(this);
         }
 
-        if (mRecyclerView != null && previouslyEmpty && !nowEmpty) {
+        if (mRecyclerView == null) {
+            return;
+        }
+
+        // When no card gets displayed either because a card's condition no longer meets
+        // or when it's dismissed, the height should be rearranged.
+        if (mContextualCards.isEmpty()) {
+            final ViewGroup.LayoutParams params = mRecyclerView.getLayoutParams();
+            if (params.height != WRAP_CONTENT) {
+                Log.d(TAG, "mContextualCards is empty. Set the RV to wrap_content");
+                params.height = WRAP_CONTENT;
+                mRecyclerView.setLayoutParams(params);
+            }
+        }
+
+        if (previouslyEmpty && !nowEmpty) {
             // Adding items to empty list, should animate.
             mRecyclerView.scheduleLayoutAnimation();
         }
diff --git a/src/com/android/settings/homepage/contextualcards/ContextualCardsFragment.java b/src/com/android/settings/homepage/contextualcards/ContextualCardsFragment.java
index 9f0023aa15c..290d69a4807 100644
--- a/src/com/android/settings/homepage/contextualcards/ContextualCardsFragment.java
+++ b/src/com/android/settings/homepage/contextualcards/ContextualCardsFragment.java
@@ -16,6 +16,8 @@
 
 package com.android.settings.homepage.contextualcards;
 
+import static android.view.ViewGroup.LayoutParams.WRAP_CONTENT;
+
 import static com.android.settings.homepage.contextualcards.ContextualCardsAdapter.SPAN_COUNT;
 
 import android.app.settings.SettingsEnums;
@@ -34,6 +36,7 @@ import androidx.annotation.VisibleForTesting;
 import androidx.loader.app.LoaderManager;
 import androidx.recyclerview.widget.GridLayoutManager;
 import androidx.recyclerview.widget.ItemTouchHelper;
+import androidx.recyclerview.widget.RecyclerView;
 
 import com.android.settings.R;
 import com.android.settings.core.InstrumentedFragment;
@@ -105,8 +108,20 @@ public class ContextualCardsFragment extends InstrumentedFragment implements
         final View rootView = inflater.inflate(R.layout.settings_homepage, container, false);
         mCardsContainer = rootView.findViewById(R.id.card_container);
         mLayoutManager = new GridLayoutManager(getActivity(), SPAN_COUNT,
-                GridLayoutManager.VERTICAL, false /* reverseLayout */);
+                GridLayoutManager.VERTICAL, false /* reverseLayout */) {
+            @Override
+            public void onLayoutCompleted(RecyclerView.State state) {
+                super.onLayoutCompleted(state);
+                // Once cards finish laying out, make the RV back to wrap content for flexibility.
+                final ViewGroup.LayoutParams params = mCardsContainer.getLayoutParams();
+                if (params.height != WRAP_CONTENT) {
+                    params.height = WRAP_CONTENT;
+                    mCardsContainer.setLayoutParams(params);
+                }
+            }
+        };
         mCardsContainer.setLayoutManager(mLayoutManager);
+        preAllocateHeight(context);
         mContextualCardsAdapter = new ContextualCardsAdapter(context, this /* lifecycleOwner */,
                 mContextualCardManager);
         mCardsContainer.setItemAnimator(null);
@@ -159,6 +174,25 @@ public class ContextualCardsFragment extends InstrumentedFragment implements
         FeatureFactory.getFactory(context).getSlicesFeatureProvider().newUiSession();
     }
 
+    private void preAllocateHeight(Context context) {
+        final int cardCount = ContextualCardLoader.getCardCount(context);
+        if (cardCount != 1) {
+            // only pre-allocate space when card count is one
+            Log.d(TAG, "Skip height pre-allocating. card count = " + cardCount);
+            return;
+        }
+
+        final int preAllocatedHeight = getResources().getDimensionPixelSize(
+                R.dimen.contextual_card_preallocated_height);
+        if (preAllocatedHeight == 0) {
+            return;
+        }
+
+        final ViewGroup.LayoutParams params = mCardsContainer.getLayoutParams();
+        params.height = preAllocatedHeight;
+        mCardsContainer.setLayoutParams(params);
+    }
+
     /**
      * Receiver for updating UI session when home key or recent app key is pressed.
      */
diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java
index fceb79dd767..7ccf38100c6 100644
--- a/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java
+++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java
@@ -71,25 +71,25 @@ public class ContextualCardLoaderTest {
     }
 
     @Test
-    public void getDisplayableCards_twoEligibleCards_shouldShowAll() {
+    public void getDisplayableCards_twoEligibleCards_notExceedDefaultCardCount() {
         final List<ContextualCard> cards = getContextualCardList().stream().limit(2)
                 .collect(Collectors.toList());
         doReturn(cards).when(mContextualCardLoader).filterEligibleCards(anyList());
 
         final List<ContextualCard> result = mContextualCardLoader.getDisplayableCards(cards);
 
-        assertThat(result).hasSize(cards.size());
+        assertThat(result).hasSize(Math.min(cards.size(), DEFAULT_CARD_COUNT));
     }
 
     @Test
-    public void getDisplayableCards_fourEligibleCards_shouldShowDefaultCardCount() {
+    public void getDisplayableCards_fourEligibleCards_notExceedDefaultCardCount() {
         final List<ContextualCard> cards = getContextualCardList().stream().limit(4)
                 .collect(Collectors.toList());
         doReturn(cards).when(mContextualCardLoader).filterEligibleCards(anyList());
 
         final List<ContextualCard> result = mContextualCardLoader.getDisplayableCards(cards);
 
-        assertThat(result).hasSize(DEFAULT_CARD_COUNT);
+        assertThat(result).hasSize(Math.min(cards.size(), DEFAULT_CARD_COUNT));
     }
 
     @Test
@@ -139,7 +139,7 @@ public class ContextualCardLoaderTest {
 
     @Test
     public void getCardCount_noConfiguredCardCount_returnDefaultCardCount() {
-        assertThat(mContextualCardLoader.getCardCount()).isEqualTo(DEFAULT_CARD_COUNT);
+        assertThat(mContextualCardLoader.getCardCount(mContext)).isEqualTo(DEFAULT_CARD_COUNT);
     }
 
     @Test
@@ -148,7 +148,7 @@ public class ContextualCardLoaderTest {
         Settings.Global.putLong(mContext.getContentResolver(),
                 ContextualCardLoader.CONTEXTUAL_CARD_COUNT, configCount);
 
-        assertThat(mContextualCardLoader.getCardCount()).isEqualTo(configCount);
+        assertThat(mContextualCardLoader.getCardCount(mContext)).isEqualTo(configCount);
     }
 
     private List<ContextualCard> getContextualCardList() {