Fix jank in showing conditions and suggestions in cold start.

When we first initialize the dashboard view, and register the condition
listener, it will trigger the condition changed callback immediately.
This results in unnecessary refresh of the dashboard header. Add check
to not do the refresh when we first initialize the view.

Change-Id: If7c69637463734c150b7f5eb7f3c042cf73837fa
Fixes: 64811475
Test: make RunSettingsRoboTests
This commit is contained in:
Doris Ling
2017-08-24 13:09:57 -07:00
parent efcc8be05c
commit 5b01294d84
2 changed files with 32 additions and 4 deletions

View File

@@ -75,6 +75,7 @@ public class DashboardSummary extends InstrumentedFragment
private DashboardFeatureProvider mDashboardFeatureProvider; private DashboardFeatureProvider mDashboardFeatureProvider;
private SuggestionFeatureProvider mSuggestionFeatureProvider; private SuggestionFeatureProvider mSuggestionFeatureProvider;
private boolean isOnCategoriesChangedCalled; private boolean isOnCategoriesChangedCalled;
private boolean mOnConditionsChangedCalled;
@Override @Override
public int getMetricsCategory() { public int getMetricsCategory() {
@@ -237,11 +238,22 @@ public class DashboardSummary extends InstrumentedFragment
@Override @Override
public void onConditionsChanged() { public void onConditionsChanged() {
Log.d(TAG, "onConditionsChanged"); Log.d(TAG, "onConditionsChanged");
final boolean scrollToTop = mLayoutManager.findFirstCompletelyVisibleItemPosition() <= 1; // Bypass refreshing the conditions on the first call of onConditionsChanged.
// onConditionsChanged is called immediately everytime we start listening to the conditions
// change when we gain window focus. Since the conditions are passed to the adapter's
// constructor when we create the view, the first handling is not necessary.
// But, on the subsequent calls we need to handle it because there might be real changes to
// conditions.
if (mOnConditionsChangedCalled) {
final boolean scrollToTop =
mLayoutManager.findFirstCompletelyVisibleItemPosition() <= 1;
mAdapter.setConditions(mConditionManager.getConditions()); mAdapter.setConditions(mConditionManager.getConditions());
if (scrollToTop) { if (scrollToTop) {
mDashboard.scrollToPosition(0); mDashboard.scrollToPosition(0);
} }
} else {
mOnConditionsChangedCalled = true;
}
} }
@Override @Override

View File

@@ -36,6 +36,7 @@ import org.robolectric.annotation.Config;
import org.robolectric.util.ReflectionHelpers; import org.robolectric.util.ReflectionHelpers;
import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.ArgumentMatchers.nullable;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
@@ -90,6 +91,7 @@ public class DashboardSummaryTest {
public void onConditionChanged_PositionAtTop_ScrollToTop() { public void onConditionChanged_PositionAtTop_ScrollToTop() {
when(mLayoutManager.findFirstCompletelyVisibleItemPosition()).thenReturn(1); when(mLayoutManager.findFirstCompletelyVisibleItemPosition()).thenReturn(1);
mSummary.onConditionsChanged(); mSummary.onConditionsChanged();
mSummary.onConditionsChanged();
verify(mDashboard).scrollToPosition(0); verify(mDashboard).scrollToPosition(0);
} }
@@ -97,9 +99,23 @@ public class DashboardSummaryTest {
public void onConditionChanged_PositionNotTop_RemainPosition() { public void onConditionChanged_PositionNotTop_RemainPosition() {
when(mLayoutManager.findFirstCompletelyVisibleItemPosition()).thenReturn(2); when(mLayoutManager.findFirstCompletelyVisibleItemPosition()).thenReturn(2);
mSummary.onConditionsChanged(); mSummary.onConditionsChanged();
mSummary.onConditionsChanged();
verify(mDashboard, never()).scrollToPosition(0); verify(mDashboard, never()).scrollToPosition(0);
} }
@Test
public void onConditionChanged_firstCall_shouldIgnore() {
mSummary.onConditionsChanged();
verify(mAdapter, never()).setConditions(any());
}
@Test
public void onConditionChanged_secondCall_shouldSetConditionsOnAdapter() {
mSummary.onConditionsChanged();
mSummary.onConditionsChanged();
verify(mAdapter).setConditions(any());
}
@Test @Test
public void onCategoryChanged_noRebuildOnFirstCall() { public void onCategoryChanged_noRebuildOnFirstCall() {
doReturn(mock(Activity.class)).when(mSummary).getActivity(); doReturn(mock(Activity.class)).when(mSummary).getActivity();