From cbc97bc7dd845251dadfa1e6825d294827848903 Mon Sep 17 00:00:00 2001 From: Fan Zhang Date: Mon, 4 Dec 2017 14:31:30 -0800 Subject: [PATCH] Hack to wait for both suggestion/category to load - This is unfortunately necessary to avoid a jank when category load completes before suggestion load, in which case user has to manually scroll up to see the suggestions. - We could technically just add scrollTo(0), but that causes the list scroll on its own within 0.5 second of settings start, and that's bad. Change-Id: I8dc869a69a5bf11bbf7644b281cc1778dd1a90e8 Fixes: 69068691 Test: visual Test: robotests --- .../settings/dashboard/DashboardAdapter.java | 1 - .../settings/dashboard/DashboardSummary.java | 23 +++++++++++++++++- .../SuggestionControllerMixin.java | 8 +++++++ .../SuggestionControllerMixinTest.java | 24 +++++++++++++------ 4 files changed, 47 insertions(+), 9 deletions(-) diff --git a/src/com/android/settings/dashboard/DashboardAdapter.java b/src/com/android/settings/dashboard/DashboardAdapter.java index 54a1eeb3fb1..3d473f0a7ba 100644 --- a/src/com/android/settings/dashboard/DashboardAdapter.java +++ b/src/com/android/settings/dashboard/DashboardAdapter.java @@ -161,7 +161,6 @@ public class DashboardAdapter extends RecyclerView.Adapter data) { - // TODO: Tint icon final DashboardData prevData = mDashboardData; mDashboardData = new DashboardData.Builder(prevData) .setSuggestionsV2(data) diff --git a/src/com/android/settings/dashboard/DashboardSummary.java b/src/com/android/settings/dashboard/DashboardSummary.java index 8f66b67f4d9..ff2a76aaad3 100644 --- a/src/com/android/settings/dashboard/DashboardSummary.java +++ b/src/com/android/settings/dashboard/DashboardSummary.java @@ -81,6 +81,9 @@ public class DashboardSummary extends InstrumentedFragment private boolean isOnCategoriesChangedCalled; private boolean mOnConditionsChangedCalled; + private DashboardCategory mStagingCategory; + private List mStagingSuggestions; + @Override public int getMetricsCategory() { return MetricsEvent.DASHBOARD_SUMMARY; @@ -291,7 +294,13 @@ public class DashboardSummary extends InstrumentedFragment @Override public void onSuggestionReady(List suggestions) { + mStagingSuggestions = suggestions; mAdapter.setSuggestionsV2(suggestions); + if (mStagingCategory != null) { + Log.d(TAG, "Category has loaded, setting category from suggestionReady"); + mHandler.removeCallbacksAndMessages(null); + mAdapter.setCategory(mStagingCategory); + } } /** @@ -342,7 +351,19 @@ public class DashboardSummary extends InstrumentedFragment final DashboardCategory category = mDashboardFeatureProvider.getTilesForCategory( CategoryKey.CATEGORY_HOMEPAGE); mSummaryLoader.updateSummaryToCache(category); - ThreadUtils.postOnMainThread(() -> mAdapter.setCategory(category)); + mStagingCategory = category; + if (mSuggestionControllerMixin.isSuggestionLoaded()) { + Log.d(TAG, "Suggestion has loaded, setting suggestion/category"); + ThreadUtils.postOnMainThread(() -> { + if (mStagingSuggestions != null) { + mAdapter.setSuggestionsV2(mStagingSuggestions); + } + mAdapter.setCategory(mStagingCategory); + }); + } else { + Log.d(TAG, "Suggestion NOT loaded, delaying setCategory by " + MAX_WAIT_MILLIS + "ms"); + mHandler.postDelayed(() -> mAdapter.setCategory(mStagingCategory), MAX_WAIT_MILLIS); + } } /** diff --git a/src/com/android/settings/dashboard/suggestions/SuggestionControllerMixin.java b/src/com/android/settings/dashboard/suggestions/SuggestionControllerMixin.java index 71bf10771c4..81496eec38e 100644 --- a/src/com/android/settings/dashboard/suggestions/SuggestionControllerMixin.java +++ b/src/com/android/settings/dashboard/suggestions/SuggestionControllerMixin.java @@ -59,6 +59,8 @@ public class SuggestionControllerMixin implements SuggestionController.ServiceCo private final SuggestionController mSuggestionController; private final SuggestionControllerHost mHost; + private boolean mSuggestionLoaded; + public SuggestionControllerMixin(Context context, SuggestionControllerHost host, Lifecycle lifecycle) { mContext = context.getApplicationContext(); @@ -106,6 +108,7 @@ public class SuggestionControllerMixin implements SuggestionController.ServiceCo @Override public Loader> onCreateLoader(int id, Bundle args) { if (id == SuggestionLoader.LOADER_ID_SUGGESTIONS) { + mSuggestionLoaded = false; return new SuggestionLoader(mContext, mSuggestionController); } throw new IllegalArgumentException("This loader id is not supported " + id); @@ -113,12 +116,17 @@ public class SuggestionControllerMixin implements SuggestionController.ServiceCo @Override public void onLoadFinished(Loader> loader, List data) { + mSuggestionLoaded = true; mHost.onSuggestionReady(data); } @Override public void onLoaderReset(Loader> loader) { + mSuggestionLoaded = false; + } + public boolean isSuggestionLoaded() { + return mSuggestionLoaded; } public void dismissSuggestion(Suggestion suggestion) { diff --git a/tests/robotests/src/com/android/settings/dashboard/suggestions/SuggestionControllerMixinTest.java b/tests/robotests/src/com/android/settings/dashboard/suggestions/SuggestionControllerMixinTest.java index ad97e183f6e..790f16605ea 100644 --- a/tests/robotests/src/com/android/settings/dashboard/suggestions/SuggestionControllerMixinTest.java +++ b/tests/robotests/src/com/android/settings/dashboard/suggestions/SuggestionControllerMixinTest.java @@ -18,9 +18,7 @@ package com.android.settings.dashboard.suggestions; import static android.arch.lifecycle.Lifecycle.Event.ON_START; import static android.arch.lifecycle.Lifecycle.Event.ON_STOP; - import static com.google.common.truth.Truth.assertThat; - import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -37,9 +35,9 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.Answers; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; @RunWith(SettingsRobolectricTestRunner.class) @@ -49,19 +47,18 @@ import org.robolectric.annotation.Config; }) public class SuggestionControllerMixinTest { - @Mock(answer = Answers.RETURNS_DEEP_STUBS) - private Context mContext; @Mock private SuggestionControllerMixin.SuggestionControllerHost mHost; + private Context mContext; private Lifecycle mLifecycle; private SuggestionControllerMixin mMixin; @Before public void setUp() { MockitoAnnotations.initMocks(this); - FakeFeatureFactory.setupForTest(mContext); + mContext = RuntimeEnvironment.application; + FakeFeatureFactory.setupForTest(); mLifecycle = new Lifecycle(() -> mLifecycle); - when(mContext.getApplicationContext()).thenReturn(mContext); } @After @@ -111,4 +108,17 @@ public class SuggestionControllerMixinTest { verify(mHost).getLoaderManager(); } + + @Test + public void doneLoadingg_shouldSetSuggestionLoaded() { + mMixin = new SuggestionControllerMixin(mContext, mHost, mLifecycle); + + mMixin.onLoadFinished(mock(SuggestionLoader.class), null); + + assertThat(mMixin.isSuggestionLoaded()).isTrue(); + + mMixin.onLoaderReset(mock(SuggestionLoader.class)); + + assertThat(mMixin.isSuggestionLoaded()).isFalse(); + } }