From bed0f239407effd1219a86b860ecc56d031346e0 Mon Sep 17 00:00:00 2001 From: Yi-Ling Chuang Date: Fri, 29 May 2020 18:10:03 +0800 Subject: [PATCH] Fix the janky transition of contextual cards. When contextual cards are being laid out, there are two separate layout transitions, which brings users the feeling of slowness. In the current design, we bind slices in the adapter's onBindViewHolder(), where slice's binding is acutally done in the background thread and it's time consuming. So before getting the callback from the slice framework to have actual contents, the view is empty but the viewholder is already created. So the RecyclerView would treat it as completed and starts to lay them out. This introduces the first time transition. Once we get the actual slice content, the view will be refreshed and laid out, which is the second time transition. To tackle this, this CL caches slices that are created at pre-check time, and use them to render before getting updated slices to fill up the gap. Fixes: 156372414 Test: robotest and launch settings to see the transition being smooth. Change-Id: Ic0a27ff36f1824de499b75ec73b2635de9cbe6b5 --- .../contextualcards/ContextualCard.java | 18 +++++++++ .../contextualcards/EligibleCardChecker.java | 11 ++++-- .../slices/SliceContextualCardRenderer.java | 6 +++ .../EligibleCardCheckerTest.java | 11 ++++++ .../SliceContextualCardRendererTest.java | 39 ++++++++++++++++++- 5 files changed, 80 insertions(+), 5 deletions(-) diff --git a/src/com/android/settings/homepage/contextualcards/ContextualCard.java b/src/com/android/settings/homepage/contextualcards/ContextualCard.java index 262cd2fd971..510c36d13e9 100644 --- a/src/com/android/settings/homepage/contextualcards/ContextualCard.java +++ b/src/com/android/settings/homepage/contextualcards/ContextualCard.java @@ -23,6 +23,7 @@ import android.net.Uri; import android.text.TextUtils; import androidx.annotation.LayoutRes; +import androidx.slice.Slice; import com.android.settings.homepage.contextualcards.slices.SliceContextualCardRenderer; @@ -66,6 +67,7 @@ public class ContextualCard { private final int mViewType; private final boolean mIsPendingDismiss; private final boolean mHasInlineAction; + private final Slice mSlice; public String getName() { return mName; @@ -127,6 +129,10 @@ public class ContextualCard { return mHasInlineAction; } + public Slice getSlice() { + return mSlice; + } + public Builder mutate() { return mBuilder; } @@ -147,6 +153,7 @@ public class ContextualCard { mViewType = builder.mViewType; mIsPendingDismiss = builder.mIsPendingDismiss; mHasInlineAction = builder.mHasInlineAction; + mSlice = builder.mSlice; } ContextualCard(Cursor c) { @@ -179,6 +186,8 @@ public class ContextualCard { mBuilder.setIsPendingDismiss(mIsPendingDismiss); mHasInlineAction = false; mBuilder.setHasInlineAction(mHasInlineAction); + mSlice = null; + mBuilder.setSlice(mSlice); } @Override @@ -225,6 +234,7 @@ public class ContextualCard { private int mViewType; private boolean mIsPendingDismiss; private boolean mHasInlineAction; + private Slice mSlice; public Builder setName(String name) { mName = name; @@ -296,6 +306,14 @@ public class ContextualCard { return this; } + /** + * Cache a slice created at pre-check time for later usage. + */ + public Builder setSlice(Slice slice) { + mSlice = slice; + return this; + } + public ContextualCard build() { return new ContextualCard(this); } diff --git a/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java b/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java index 90b7f36683b..4e8c61f8fad 100644 --- a/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java +++ b/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java @@ -96,14 +96,17 @@ public class EligibleCardChecker implements Callable { final Slice slice = bindSlice(uri); - if (isSliceToggleable(slice)) { - mCard = card.mutate().setHasInlineAction(true).build(); - } - if (slice == null || slice.hasHint(HINT_ERROR)) { Log.w(TAG, "Failed to bind slice, not eligible for display " + uri); return false; } + + mCard = card.mutate().setSlice(slice).build(); + + if (isSliceToggleable(slice)) { + mCard = card.mutate().setHasInlineAction(true).build(); + } + return true; } diff --git a/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRenderer.java b/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRenderer.java index 6538cac4231..c9ec5cf8733 100644 --- a/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRenderer.java +++ b/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRenderer.java @@ -47,6 +47,7 @@ import com.android.settings.homepage.contextualcards.CardContentProvider; import com.android.settings.homepage.contextualcards.ContextualCard; import com.android.settings.homepage.contextualcards.ContextualCardRenderer; import com.android.settings.homepage.contextualcards.ControllerRendererPool; +import com.android.settings.homepage.contextualcards.slices.SliceFullCardRendererHelper.SliceViewHolder; import com.android.settingslib.utils.ThreadUtils; import java.util.Map; @@ -102,6 +103,11 @@ public class SliceContextualCardRenderer implements ContextualCardRenderer, Life return; } + // Show cached slice first before slice binding completed to avoid jank. + if (holder.getItemViewType() != VIEW_TYPE_HALF_WIDTH) { + ((SliceViewHolder) holder).sliceView.setSlice(card.getSlice()); + } + LiveData sliceLiveData = mSliceLiveDataMap.get(uri); if (sliceLiveData == null) { diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/EligibleCardCheckerTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/EligibleCardCheckerTest.java index 23ae2f3a75c..613062fea14 100644 --- a/tests/robotests/src/com/android/settings/homepage/contextualcards/EligibleCardCheckerTest.java +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/EligibleCardCheckerTest.java @@ -114,6 +114,17 @@ public class EligibleCardCheckerTest { .isFalse(); } + @Test + public void isCardEligibleToDisplay_sliceNotNull_cacheSliceToCard() { + final ContextualWifiSlice wifiSlice = new ContextualWifiSlice(mContext); + final Slice slice = wifiSlice.getSlice(); + doReturn(slice).when(mEligibleCardChecker).bindSlice(any(Uri.class)); + + mEligibleCardChecker.isCardEligibleToDisplay(getContextualCard(TEST_SLICE_URI)); + + assertThat(mEligibleCardChecker.mCard.getSlice()).isNotNull(); + } + private ContextualCard getContextualCard(Uri sliceUri) { return new ContextualCard.Builder() .setName("test_card") diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRendererTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRendererTest.java index 70761cf52d5..154d106c12d 100644 --- a/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRendererTest.java +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRendererTest.java @@ -17,10 +17,12 @@ package com.android.settings.homepage.contextualcards.slices; import static com.android.settings.homepage.contextualcards.slices.SliceContextualCardRenderer.VIEW_TYPE_FULL_WIDTH; +import static com.android.settings.homepage.contextualcards.slices.SliceContextualCardRenderer.VIEW_TYPE_STICKY; import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import android.app.Activity; @@ -39,6 +41,7 @@ import com.android.settings.R; import com.android.settings.homepage.contextualcards.ContextualCard; import com.android.settings.homepage.contextualcards.ContextualCardsFragment; import com.android.settings.homepage.contextualcards.ControllerRendererPool; +import com.android.settings.wifi.slice.ContextualWifiSlice; import org.junit.Before; import org.junit.Test; @@ -81,7 +84,7 @@ public class SliceContextualCardRendererTest { @Test public void bindView_invalidScheme_sliceShouldBeNull() { final Uri sliceUri = Uri.parse("contet://com.android.settings.slices/action/flashlight"); - RecyclerView.ViewHolder viewHolder = getSliceViewHolder(); + final RecyclerView.ViewHolder viewHolder = getSliceViewHolder(); mRenderer.bindView(viewHolder, buildContextualCard(sliceUri)); @@ -90,6 +93,29 @@ public class SliceContextualCardRendererTest { .isNull(); } + @Test + public void bindView_viewTypeFullWidth_shouldSetCachedSlice() { + final RecyclerView.ViewHolder viewHolder = getSliceViewHolder(); + + mRenderer.bindView(viewHolder, buildContextualCard(TEST_SLICE_URI)); + + assertThat( + ((SliceFullCardRendererHelper.SliceViewHolder) viewHolder).sliceView.getSlice()) + .isNotNull(); + } + + @Test + public void bindView_viewTypeSticky_shouldSetCachedSlice() { + final RecyclerView.ViewHolder viewHolder = spy(getStickyViewHolder()); + doReturn(VIEW_TYPE_STICKY).when(viewHolder).getItemViewType(); + + mRenderer.bindView(viewHolder, buildContextualCard(TEST_SLICE_URI)); + + assertThat( + ((SliceFullCardRendererHelper.SliceViewHolder) viewHolder).sliceView.getSlice()) + .isNotNull(); + } + @Test public void bindView_newSliceLiveData_shouldAddDataToMap() { mRenderer.bindView(getSliceViewHolder(), buildContextualCard(TEST_SLICE_URI)); @@ -246,12 +272,23 @@ public class SliceContextualCardRendererTest { return mRenderer.createViewHolder(view, VIEW_TYPE_FULL_WIDTH); } + private RecyclerView.ViewHolder getStickyViewHolder() { + final RecyclerView recyclerView = new RecyclerView(mActivity); + recyclerView.setLayoutManager(new LinearLayoutManager(mActivity)); + final View view = LayoutInflater.from(mActivity).inflate(VIEW_TYPE_STICKY, recyclerView, + false); + + return mRenderer.createViewHolder(view, VIEW_TYPE_STICKY); + } + private ContextualCard buildContextualCard(Uri sliceUri) { + final Slice slice = new ContextualWifiSlice(mActivity).getSlice(); return new ContextualCard.Builder() .setName("test_name") .setCardType(ContextualCard.CardType.SLICE) .setSliceUri(sliceUri) .setViewType(VIEW_TYPE_FULL_WIDTH) + .setSlice(slice) .build(); } }