From 5416a5992bceb91bd3eda63629ba5420a714747b Mon Sep 17 00:00:00 2001 From: Matthew Fritze Date: Tue, 2 May 2017 18:43:31 -0700 Subject: [PATCH] Only add icons to settings items with icons Currently the icon for the category of setting is used for each setting result. Change it to only add an icon when it exists in xml. Bug: 37858983 Test: make RunSettingsRoboTests Change-Id: Ib95e76c5a9ba2e73738cea716d72c5994a6aa0ba --- .../search2/DatabaseIndexingManager.java | 12 ++++------ .../settings/search2/SearchViewHolder.java | 6 ++--- .../settings/search2/XmlParserUtils.java | 22 +++++++++--------- .../search/IntentSearchViewHolderTest.java | 22 ++++++++++++------ .../search2/DatabaseIndexingManagerTest.java | 23 +++++++++++++++---- 5 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/com/android/settings/search2/DatabaseIndexingManager.java b/src/com/android/settings/search2/DatabaseIndexingManager.java index c627ea56094..647219bee48 100644 --- a/src/com/android/settings/search2/DatabaseIndexingManager.java +++ b/src/com/android/settings/search2/DatabaseIndexingManager.java @@ -33,6 +33,7 @@ import android.os.Build; import android.provider.SearchIndexableData; import android.provider.SearchIndexableResource; import android.provider.SearchIndexablesContract; +import android.support.annotation.DrawableRes; import android.support.annotation.VisibleForTesting; import android.text.TextUtils; import android.util.AttributeSet; @@ -525,8 +526,6 @@ public class DatabaseIndexingManager { final int count = cursor.getCount(); if (count > 0) { while (cursor.moveToNext()) { - final int providerRank = cursor.getInt(COLUMN_INDEX_XML_RES_RANK); - // TODO remove provider rank final int xmlResId = cursor.getInt(COLUMN_INDEX_XML_RES_RESID); final String className = cursor.getString(COLUMN_INDEX_XML_RES_CLASS_NAME); @@ -720,7 +719,6 @@ public class DatabaseIndexingManager { final AttributeSet attrs = Xml.asAttributeSet(parser); final String screenTitle = XmlParserUtils.getDataTitle(context, attrs); - String key = XmlParserUtils.getDataKey(context, attrs); String title; @@ -730,10 +728,11 @@ public class DatabaseIndexingManager { String keywords; String headerKeywords; String childFragment; + @DrawableRes + int iconResId; ResultPayload payload; boolean enabled; final String fragmentName = sir.className; - final int iconResId = sir.iconResId; final int rank = sir.rank; final String intentAction = sir.intentAction; final String intentTargetPackage = sir.intentTargetPackage; @@ -784,6 +783,7 @@ public class DatabaseIndexingManager { key = XmlParserUtils.getDataKey(context, attrs); enabled = ! nonIndexableKeys.contains(key); keywords = XmlParserUtils.getDataKeywords(context, attrs); + iconResId = XmlParserUtils.getDataIcon(context, attrs); if (isHeaderUnique && TextUtils.equals(headerTitle, title)) { isHeaderUnique = false; @@ -853,7 +853,6 @@ public class DatabaseIndexingManager { List nonIndexableKeys) { final String className = sir.className; - final int iconResId = sir.iconResId; final int rank = sir.rank; if (provider == null) { @@ -881,7 +880,7 @@ public class DatabaseIndexingManager { .setEntries(raw.entries) .setClassName(className) .setScreenTitle(raw.screenTitle) - .setIconResId(iconResId) + .setIconResId(raw.iconResId) .setRank(rank) .setIntentAction(raw.intentAction) .setIntentTargetPackage(raw.intentTargetPackage) @@ -907,7 +906,6 @@ public class DatabaseIndexingManager { continue; } - item.iconResId = (item.iconResId == 0) ? iconResId : item.iconResId; item.className = (TextUtils.isEmpty(item.className)) ? className : item.className; indexFromResource(database, localeStr, item, nonIndexableKeys); diff --git a/src/com/android/settings/search2/SearchViewHolder.java b/src/com/android/settings/search2/SearchViewHolder.java index 67653e02aa5..1175fcb2490 100644 --- a/src/com/android/settings/search2/SearchViewHolder.java +++ b/src/com/android/settings/search2/SearchViewHolder.java @@ -69,11 +69,9 @@ public abstract class SearchViewHolder extends RecyclerView.ViewHolder { AppSearchResult appResult = (AppSearchResult) result; PackageManager pm = fragment.getActivity().getPackageManager(); iconView.setImageDrawable(appResult.info.loadIcon(pm)); - } else if (result.icon != null) { - iconView.setImageDrawable(result.icon); - // TODO set color of icon } else { - iconView.setBackgroundResource(R.drawable.empty_icon); + // Valid even when result.icon is null. + iconView.setImageDrawable(result.icon); } bindBreadcrumbView(result); diff --git a/src/com/android/settings/search2/XmlParserUtils.java b/src/com/android/settings/search2/XmlParserUtils.java index 90b1c1fe679..17f1743401e 100644 --- a/src/com/android/settings/search2/XmlParserUtils.java +++ b/src/com/android/settings/search2/XmlParserUtils.java @@ -71,6 +71,14 @@ public class XmlParserUtils { return getData(context, attrs, R.styleable.Preference, R.styleable.Preference_keywords); } + public static int getDataIcon(Context context, AttributeSet attrs) { + final TypedArray ta = context.obtainStyledAttributes(attrs, + com.android.internal.R.styleable.Preference); + final int dataIcon = ta.getResourceId(com.android.internal.R.styleable.Icon_icon, 0); + ta.recycle(); + return dataIcon; + } + /** * Returns the fragment name if this preference launches a child fragment. */ @@ -80,17 +88,9 @@ public class XmlParserUtils { } private static String getData(Context context, AttributeSet set, int[] attrs, int resId) { - final TypedArray sa = context.obtainStyledAttributes(set, attrs); - final TypedValue tv = sa.peekValue(resId); - - CharSequence data = null; - if (tv != null && tv.type == TypedValue.TYPE_STRING) { - if (tv.resourceId != 0) { - data = context.getText(tv.resourceId); - } else { - data = tv.string; - } - } + final TypedArray ta = context.obtainStyledAttributes(set, attrs); + String data = ta.getString(resId); + ta.recycle(); return (data != null) ? data.toString() : null; } diff --git a/tests/robotests/src/com/android/settings/search/IntentSearchViewHolderTest.java b/tests/robotests/src/com/android/settings/search/IntentSearchViewHolderTest.java index cc7e4ec4856..51cd48428e5 100644 --- a/tests/robotests/src/com/android/settings/search/IntentSearchViewHolderTest.java +++ b/tests/robotests/src/com/android/settings/search/IntentSearchViewHolderTest.java @@ -82,7 +82,7 @@ public class IntentSearchViewHolderTest { } @Test - public void testConstructor_MembersNotNull() { + public void testConstructor_membersNotNull() { assertThat(mHolder.titleView).isNotNull(); assertThat(mHolder.summaryView).isNotNull(); assertThat(mHolder.iconView).isNotNull(); @@ -90,8 +90,8 @@ public class IntentSearchViewHolderTest { } @Test - public void testBindViewElements_AllUpdated() { - SearchResult result = getSearchResult(); + public void testBindViewElements_allUpdated() { + SearchResult result = getSearchResult(TITLE, SUMMARY, mIcon); mHolder.onBind(mFragment, result); mHolder.itemView.performClick(); @@ -109,6 +109,14 @@ public class IntentSearchViewHolderTest { any(Pair.class)); } + @Test + public void testBindViewIcon_nullIcon_imageDrawableIsNull() { + final SearchResult result = getSearchResult(TITLE, SUMMARY, null); + mHolder.onBind(mFragment, result); + + assertThat(mHolder.iconView.getDrawable()).isNull(); + } + @Test public void testBindViewElements_emptySummary_hideSummaryView() { final SearchResult result = new Builder() @@ -155,15 +163,15 @@ public class IntentSearchViewHolderTest { assertThat(mHolder.summaryView.getVisibility()).isEqualTo(View.GONE); } - private SearchResult getSearchResult() { + private SearchResult getSearchResult(String title, String summary, Drawable icon) { Builder builder = new Builder(); - builder.addTitle(TITLE) - .addSummary(SUMMARY) + builder.addTitle(title) + .addSummary(summary) .addRank(1) .addPayload(new IntentPayload( new Intent().setComponent(new ComponentName("pkg", "class")))) .addBreadcrumbs(new ArrayList<>()) - .addIcon(mIcon); + .addIcon(icon); return builder.build(); } diff --git a/tests/robotests/src/com/android/settings/search2/DatabaseIndexingManagerTest.java b/tests/robotests/src/com/android/settings/search2/DatabaseIndexingManagerTest.java index 0a8326dd6f1..b76feff1a64 100644 --- a/tests/robotests/src/com/android/settings/search2/DatabaseIndexingManagerTest.java +++ b/tests/robotests/src/com/android/settings/search2/DatabaseIndexingManagerTest.java @@ -99,6 +99,7 @@ public class DatabaseIndexingManagerTest { private final String screenTitle = "screen title"; private final String className = "class name"; private final int iconResId = 0xff; + private final int noIcon = 0; private final String action = "action"; private final String targetPackage = "target package"; private final String targetClass = "target class"; @@ -388,7 +389,7 @@ public class DatabaseIndexingManagerTest { // Class Name assertThat(cursor.getString(11)).isEqualTo(className); // Icon - assertThat(cursor.getInt(12)).isEqualTo(iconResId); + assertThat(cursor.getInt(12)).isEqualTo(noIcon); // Intent Action assertThat(cursor.getString(13)).isEqualTo(action); // Target Package @@ -442,7 +443,7 @@ public class DatabaseIndexingManagerTest { // Class Name assertThat(cursor.getString(11)).isEqualTo(className); // Icon - assertThat(cursor.getInt(12)).isEqualTo(iconResId); + assertThat(cursor.getInt(12)).isEqualTo(noIcon); // Intent Action assertThat(cursor.getString(13)).isEqualTo(action); // Target Package @@ -496,7 +497,7 @@ public class DatabaseIndexingManagerTest { // Class Name assertThat(cursor.getString(11)).isEqualTo(className); // Icon - assertThat(cursor.getInt(12)).isEqualTo(iconResId); + assertThat(cursor.getInt(12)).isEqualTo(noIcon); // Intent Action assertThat(cursor.getString(13)).isEqualTo(action); // Target Package @@ -515,6 +516,18 @@ public class DatabaseIndexingManagerTest { assertThat(cursor.getBlob(20)).isNull(); } + @Test + public void testAddResource_iconAddedFromXml() { + SearchIndexableResource resource = getFakeResource(R.xml.connected_devices); + mManager.indexOneSearchIndexableData(mDb, localeStr, resource, new HashMap<>()); + + Cursor cursor = mDb.rawQuery("SELECT * FROM prefs_index ORDER BY data_title", null); + cursor.moveToPosition(0); + + // Icon + assertThat(cursor.getInt(12)).isNotEqualTo(noIcon); + } + // Tests for the flow: IndexOneResource -> IndexFromProvider -> IndexFromResource -> // UpdateOneRowWithFilteredData -> UpdateOneRow @@ -565,7 +578,7 @@ public class DatabaseIndexingManagerTest { assertThat(cursor.getString(11)) .isEqualTo("com.android.settings.display.ScreenZoomSettings"); // Icon - assertThat(cursor.getInt(12)).isEqualTo(iconResId); + assertThat(cursor.getInt(12)).isEqualTo(noIcon); // Intent Action assertThat(cursor.getString(13)).isNull(); // Target Package @@ -630,7 +643,7 @@ public class DatabaseIndexingManagerTest { assertThat(cursor.getString(11)) .isEqualTo("com.android.settings.display.ScreenZoomSettings"); // Icon - assertThat(cursor.getInt(12)).isEqualTo(iconResId); + assertThat(cursor.getInt(12)).isEqualTo(noIcon); // Intent Action assertThat(cursor.getString(13)).isNull(); // Target Package