From c217464dca9eb29dfaa8fc67cf019cbb0059da5e Mon Sep 17 00:00:00 2001 From: Andrew Sapperstein Date: Sun, 25 Jun 2017 15:18:46 -0700 Subject: [PATCH] Dashboard search bar polish Various tweaks to match redlines: - background is now grey all the time - the search icon is aligned with the other icons - the title text is primary colored instead of accent colored - the background of search bar doesn't scroll anymore Deletes a bunch of logic to add an initial view that would be colored based on the first view. It never quite worked right. Now it's consistent and never moves. Bug: 37477506 Change-Id: I82c584e8f8ecc6fc298c5cdbce08516c6a069a39 Fixes: 62267378 Fixes: 62379978 Fixes: 62361213 Test: robotests --- res/layout/dashboard_header_spacer.xml | 25 ------- res/layout/settings_main_dashboard.xml | 48 +++++++------- src/com/android/settings/SettingsActivity.java | 8 +++ .../settings/dashboard/DashboardAdapter.java | 31 --------- .../android/settings/dashboard/DashboardData.java | 6 +- .../settings/dashboard/DashboardAdapterTest.java | 76 ++-------------------- .../settings/dashboard/DashboardDataTest.java | 10 +-- 7 files changed, 44 insertions(+), 160 deletions(-) delete mode 100644 res/layout/dashboard_header_spacer.xml diff --git a/res/layout/dashboard_header_spacer.xml b/res/layout/dashboard_header_spacer.xml deleted file mode 100644 index 442ae48b54..0000000000 --- a/res/layout/dashboard_header_spacer.xml +++ /dev/null @@ -1,25 +0,0 @@ - - - - - - diff --git a/res/layout/settings_main_dashboard.xml b/res/layout/settings_main_dashboard.xml index 1a3b1335dd..189c7329e5 100644 --- a/res/layout/settings_main_dashboard.xml +++ b/res/layout/settings_main_dashboard.xml @@ -17,32 +17,36 @@ */ --> - + android:orientation="vertical"> - - + - - + android:layout_margin="@dimen/search_bar_margin" + app:cardCornerRadius="2dp" + app:cardBackgroundColor="?android:attr/colorBackground" + app:cardElevation="2dp"> + + + + + diff --git a/src/com/android/settings/SettingsActivity.java b/src/com/android/settings/SettingsActivity.java index 805f965ede..556dbfb5b3 100644 --- a/src/com/android/settings/SettingsActivity.java +++ b/src/com/android/settings/SettingsActivity.java @@ -337,6 +337,14 @@ public class SettingsActivity extends SettingsDrawerActivity Toolbar toolbar = findViewById(R.id.search_action_bar); toolbar.setOnClickListener(this); setActionBar(toolbar); + + // Please forgive me for what I am about to do. + // + // Need to make the navigation icon non-clickable so that the entire card is clickable + // and goes to the search UI. Also set the background to null so there's no ripple. + View navView = toolbar.getNavigationView(); + navView.setClickable(false); + navView.setBackground(null); } ActionBar actionBar = getActionBar(); diff --git a/src/com/android/settings/dashboard/DashboardAdapter.java b/src/com/android/settings/dashboard/DashboardAdapter.java index 02b82b9e71..38a65dadad 100644 --- a/src/com/android/settings/dashboard/DashboardAdapter.java +++ b/src/com/android/settings/dashboard/DashboardAdapter.java @@ -65,7 +65,6 @@ public class DashboardAdapter extends RecyclerView.Adapter (position + 1)) { - // The spacer that goes underneath the search bar needs to match the - // background of the first real view. That view is either a condition, - // a suggestion, or the dashboard item. - // - // If it's a dashboard item, set null background so it uses the parent's - // background like the other views. Otherwise, match the colors. - int nextType = mDashboardData.getItemTypeByPosition(position + 1); - int colorAttr = nextType == R.layout.suggestion_header - ? android.R.attr.colorSecondary - : nextType == R.layout.condition_card - ? android.R.attr.colorAccent - : DONT_SET_BACKGROUND_ATTR; - - if (colorAttr != DONT_SET_BACKGROUND_ATTR) { - TypedArray array = holder.itemView.getContext() - .obtainStyledAttributes(new int[]{colorAttr}); - @ColorInt int color = array.getColor(0, 0); - array.recycle(); - holder.itemView.setBackgroundColor(color); - } else { - holder.itemView.setBackground(null); - } - } - } - @VisibleForTesting void onBindSuggestionHeader(final DashboardItemHolder holder, DashboardData .SuggestionHeaderData data) { diff --git a/src/com/android/settings/dashboard/DashboardData.java b/src/com/android/settings/dashboard/DashboardData.java index 7bf7729323..9f0bb6cc05 100644 --- a/src/com/android/settings/dashboard/DashboardData.java +++ b/src/com/android/settings/dashboard/DashboardData.java @@ -59,8 +59,7 @@ public class DashboardData { public static final int DEFAULT_SUGGESTION_COUNT = 2; // id namespace for different type of items. - private static final int NS_HEADER_SPACER = 0; - private static final int NS_SPACER = 1000; + private static final int NS_SPACER = 0; private static final int NS_ITEMS = 2000; private static final int NS_CONDITION = 3000; private static final int NS_SUGGESTION_CONDITION = 4000; @@ -281,9 +280,6 @@ public class DashboardData { * and mIsShowingAll, mSuggestionMode flag. */ private void buildItemsData() { - // add the view that goes under the search bar - countItem(null, R.layout.dashboard_header_spacer, true, NS_HEADER_SPACER); - resetCount(); final boolean hasSuggestions = sizeOf(mSuggestions) > 0; if (!mCombineSuggestionAndCondition) { boolean hasConditions = false; diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardAdapterTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardAdapterTest.java index e9b29dde22..af355203fc 100644 --- a/tests/robotests/src/com/android/settings/dashboard/DashboardAdapterTest.java +++ b/tests/robotests/src/com/android/settings/dashboard/DashboardAdapterTest.java @@ -124,53 +124,6 @@ public class DashboardAdapterTest { } @Test - public void testOnBindViewHolder_spacer_noSuggestions_noConditions() { - makeCategory(); - DashboardAdapter.DashboardItemHolder holder = setupSpacer(); - - mDashboardAdapter.onBindViewHolder(holder, 0); - - assertThat(holder.itemView.getBackground()).isNull(); - } - - @Test - public void testOnBindViewHolder_spacer_suggestion_noConditions() { - setupSuggestions(makeSuggestions("pkg1")); - makeCategory(); - DashboardAdapter.DashboardItemHolder holder = setupSpacer(); - - mDashboardAdapter.onBindViewHolder(holder, 0); - - assertThat(holder.itemView.getBackground()).isNotNull(); - assertThat(holder.itemView.getBackground()).isInstanceOf(ColorDrawable.class); - } - - @Test - public void testOnBindViewHolder_spacer_noSuggestion_condition() { - makeCondition(); - makeCategory(); - DashboardAdapter.DashboardItemHolder holder = setupSpacer(); - - mDashboardAdapter.onBindViewHolder(holder, 0); - - assertThat(holder.itemView.getBackground()).isNotNull(); - assertThat(holder.itemView.getBackground()).isInstanceOf(ColorDrawable.class); - } - - @Test - public void testOnBindViewHolder_spacer_suggestion_condition() { - setupSuggestions(makeSuggestions("pkg1")); - makeCondition(); - makeCategory(); - DashboardAdapter.DashboardItemHolder holder = setupSpacer(); - - mDashboardAdapter.onBindViewHolder(holder, 0); - - assertThat(holder.itemView.getBackground()).isNotNull(); - assertThat(holder.itemView.getBackground()).isInstanceOf(ColorDrawable.class); - } - - @Test public void testSuggestionsLogs_NotExpanded() { setupSuggestions(makeSuggestions("pkg1", "pkg2", "pkg3")); verify(mFactory.metricsFeatureProvider, times(2)).action( @@ -388,7 +341,7 @@ public class DashboardAdapterTest { new FrameLayout(RuntimeEnvironment.application), R.layout.suggestion_tile_card); - mDashboardAdapter.onBindViewHolder(mSuggestionHolder, 2); + mDashboardAdapter.onBindViewHolder(mSuggestionHolder, 1); assertThat(textView.getParent()).isSameAs(mSuggestionHolder.itemView); mSuggestionHolder.itemView.performClick(); @@ -413,7 +366,7 @@ public class DashboardAdapterTest { new FrameLayout(context), R.layout.suggestion_tile_card); - mDashboardAdapter.onBindViewHolder(mSuggestionHolder, 2); + mDashboardAdapter.onBindViewHolder(mSuggestionHolder, 1); mSuggestionHolder.itemView.performClick(); assertThat(ShadowApplication.getInstance().getNextStartedActivity()).isNull(); @@ -442,8 +395,8 @@ public class DashboardAdapterTest { new FrameLayout(context), R.layout.suggestion_tile_card); - mDashboardAdapter.onBindViewHolder(mSuggestionHolder, 2); - mDashboardAdapter.onBindViewHolder(mSuggestionHolder, 2); + mDashboardAdapter.onBindViewHolder(mSuggestionHolder, 1); + mDashboardAdapter.onBindViewHolder(mSuggestionHolder, 1); ViewGroup itemView = (ViewGroup) mSuggestionHolder.itemView; assertThat(itemView.getChildCount()).isEqualTo(1); @@ -509,25 +462,4 @@ public class DashboardAdapterTest { new FrameLayout(RuntimeEnvironment.application), mDashboardAdapter.getItemViewType(1)); } - - private void makeCondition() { - final List conditions = new ArrayList<>(); - Condition condition = mock(Condition.class); - when(condition.shouldShow()).thenReturn(true); - conditions.add(condition); - mDashboardAdapter.setConditions(conditions); - } - - private void makeCategory() { - List categories = new ArrayList<>(); - categories.add(new DashboardCategory()); - mDashboardAdapter.setCategory(categories); - } - - private DashboardAdapter.DashboardItemHolder setupSpacer() { - Context context = RuntimeEnvironment.application; - final View view = LayoutInflater.from(context) - .inflate(R.layout.dashboard_header_spacer, new LinearLayout(context), false); - return new DashboardAdapter.DashboardItemHolder(view); - } } diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardDataTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardDataTest.java index 47ccbdcf8d..70a05286a7 100644 --- a/tests/robotests/src/com/android/settings/dashboard/DashboardDataTest.java +++ b/tests/robotests/src/com/android/settings/dashboard/DashboardDataTest.java @@ -118,8 +118,8 @@ public class DashboardDataTest { public void testBuildItemsData_containsAllData() { final DashboardData.SuggestionConditionHeaderData data = new DashboardData.SuggestionConditionHeaderData( - mDashboardDataWithOneConditions.getConditions(), 0); - final Object[] expectedObjects = {null, data, + mDashboardDataWithOneConditions.getConditions(), 0); + final Object[] expectedObjects = {data, mDashboardDataWithOneConditions.getSuggestions(), mDashboardDataWithOneConditions.getConditions(), null, mTestCategoryTile}; @@ -198,11 +198,11 @@ public class DashboardDataTest { // Item in position 1 is the header, which contains the number of conditions, changed from // 1 to 2 testResultData.add(new ListUpdateResult.ResultData( - ListUpdateResult.ResultData.TYPE_OPERATION_CHANGE, 1, 1)); + ListUpdateResult.ResultData.TYPE_OPERATION_CHANGE, 0, 1)); // Item in position 3 is the condition container containing the list of conditions, which // gets 1 more item testResultData.add(new ListUpdateResult.ResultData( - ListUpdateResult.ResultData.TYPE_OPERATION_CHANGE, 3, 1)); + ListUpdateResult.ResultData.TYPE_OPERATION_CHANGE, 2, 1)); testDiffUtil(mDashboardDataWithOneConditions, mDashboardDataWithTwoConditions, testResultData); @@ -213,7 +213,7 @@ public class DashboardDataTest { //Build testResultData final List testResultData = new ArrayList<>(); testResultData.add(new ListUpdateResult.ResultData( - ListUpdateResult.ResultData.TYPE_OPERATION_REMOVE, 1, 5)); + ListUpdateResult.ResultData.TYPE_OPERATION_REMOVE, 0, 5)); testDiffUtil(mDashboardDataWithOneConditions, mDashboardDataWithNoItems, testResultData); } -- 2.11.0