From 3d516e7607baf12b8f46ec91b1d4bb8672def3b5 Mon Sep 17 00:00:00 2001 From: Fan Zhang Date: Wed, 31 Jan 2018 14:14:41 -0800 Subject: [PATCH] Highlight row (instead of ripple) when come from search. - Moved HighlightableAdapter to its own class - Replaced ripple with blue background - Bluebackground only stays for 5 seconds. After that it's replaced by ?attr/selectableBackground Misc fixes. - Fix NPE in UserSettings - Update char limit on a new string Change-Id: I4687e54e71fd7b9243f520b7630563df58be23d4 Fixes: 71715698 Fixes: 72761974 Test: robotests --- res/color/preference_highligh_color.xml | 20 +++ res/values/ids.xml | 2 +- res/values/strings.xml | 2 +- .../settings/SettingsPreferenceFragment.java | 125 +++-------------- .../android/settings/users/UserSettings.java | 2 +- .../HighlightablePreferenceGroupAdapter.java | 102 ++++++++++++++ ...ghlightablePreferenceGroupAdapterTest.java | 127 ++++++++++++++++++ .../SettingsPreferenceFragmentTest.java | 70 ---------- 8 files changed, 273 insertions(+), 177 deletions(-) create mode 100644 res/color/preference_highligh_color.xml create mode 100644 src/com/android/settings/widget/HighlightablePreferenceGroupAdapter.java create mode 100644 tests/robotests/src/com/android/settings/widget/HighlightablePreferenceGroupAdapterTest.java delete mode 100644 tests/unit/src/com/android/settings/SettingsPreferenceFragmentTest.java diff --git a/res/color/preference_highligh_color.xml b/res/color/preference_highligh_color.xml new file mode 100644 index 00000000000..0a8f7706dc1 --- /dev/null +++ b/res/color/preference_highligh_color.xml @@ -0,0 +1,20 @@ + + + + + + \ No newline at end of file diff --git a/res/values/ids.xml b/res/values/ids.xml index dcf279aac0e..66af163e2b8 100644 --- a/res/values/ids.xml +++ b/res/values/ids.xml @@ -17,7 +17,7 @@ */ --> - + diff --git a/res/values/strings.xml b/res/values/strings.xml index 9a6ff90dbc1..430c0d4647e 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -6145,7 +6145,7 @@ - + Multiple users Users & profiles diff --git a/src/com/android/settings/SettingsPreferenceFragment.java b/src/com/android/settings/SettingsPreferenceFragment.java index c5d477aad0b..2a593c277d9 100644 --- a/src/com/android/settings/SettingsPreferenceFragment.java +++ b/src/com/android/settings/SettingsPreferenceFragment.java @@ -30,9 +30,7 @@ import android.support.annotation.VisibleForTesting; import android.support.annotation.XmlRes; import android.support.v7.preference.Preference; import android.support.v7.preference.PreferenceGroup; -import android.support.v7.preference.PreferenceGroupAdapter; import android.support.v7.preference.PreferenceScreen; -import android.support.v7.preference.PreferenceViewHolder; import android.support.v7.widget.LinearLayoutManager; import android.support.v7.widget.RecyclerView; import android.text.TextUtils; @@ -49,6 +47,7 @@ import com.android.settings.core.instrumentation.InstrumentedDialogFragment; import com.android.settings.search.actionbar.SearchMenuController; import com.android.settings.support.actionbar.HelpMenuController; import com.android.settings.support.actionbar.HelpResourceProvider; +import com.android.settings.widget.HighlightablePreferenceGroupAdapter; import com.android.settings.widget.LoadingViewController; import com.android.settingslib.CustomDialogPreference; import com.android.settingslib.CustomEditTextPreference; @@ -65,9 +64,6 @@ public abstract class SettingsPreferenceFragment extends InstrumentedPreferenceF private static final String TAG = "SettingsPreference"; - @VisibleForTesting - static final int DELAY_HIGHLIGHT_DURATION_MILLIS = 600; - private static final String SAVE_HIGHLIGHTED_KEY = "android:preference_highlighted"; protected final FooterPreferenceMixin mFooterPreferenceMixin = @@ -75,14 +71,11 @@ public abstract class SettingsPreferenceFragment extends InstrumentedPreferenceF private static final int ORDER_FIRST = -1; - private static final int ORDER_LAST = Integer.MAX_VALUE -1; private SettingsDialogFragment mDialogFragment; // Cache the content resolver for async callbacks private ContentResolver mContentResolver; - private String mPreferenceKey; - private RecyclerView.Adapter mCurrentRootAdapter; private boolean mIsDataSetObserverRegistered = false; private RecyclerView.AdapterDataObserver mDataSetObserver = @@ -146,8 +139,9 @@ public abstract class SettingsPreferenceFragment extends InstrumentedPreferenceF // Check if we should keep the preferences expanded. if (arguments != null) { - mPreferenceKey = arguments.getString(SettingsActivity.EXTRA_FRAGMENT_ARG_KEY); - if (!TextUtils.isEmpty(mPreferenceKey)) { + final String highlightKey = + arguments.getString(SettingsActivity.EXTRA_FRAGMENT_ARG_KEY); + if (!TextUtils.isEmpty(highlightKey)) { final PreferenceScreen screen = getPreferenceScreen(); if (screen != null) { screen.setInitialExpandedChildrenCount(Integer.MAX_VALUE); @@ -205,7 +199,9 @@ public abstract class SettingsPreferenceFragment extends InstrumentedPreferenceF public void onSaveInstanceState(Bundle outState) { super.onSaveInstanceState(outState); - outState.putBoolean(SAVE_HIGHLIGHTED_KEY, mPreferenceHighlighted); + if (mAdapter != null) { + outState.putBoolean(SAVE_HIGHLIGHTED_KEY, mAdapter.isHighlightRequested()); + } } @Override @@ -217,10 +213,7 @@ public abstract class SettingsPreferenceFragment extends InstrumentedPreferenceF @Override public void onResume() { super.onResume(); - - if (mPreferenceKey != null) { - highlightPreferenceIfNeeded(); - } + highlightPreferenceIfNeeded(); } @Override @@ -263,13 +256,11 @@ public abstract class SettingsPreferenceFragment extends InstrumentedPreferenceF } public void highlightPreferenceIfNeeded() { - if (isAdded() && !mPreferenceHighlighted &&!TextUtils.isEmpty(mPreferenceKey)) { - getView().postDelayed(new Runnable() { - @Override - public void run() { - highlightPreference(mPreferenceKey); - } - }, DELAY_HIGHLIGHT_DURATION_MILLIS); + if (!isAdded()) { + return; + } + if (mAdapter != null) { + mAdapter.requestHighlight(getView(), getListView()); } } @@ -340,24 +331,6 @@ public abstract class SettingsPreferenceFragment extends InstrumentedPreferenceF return mEmptyView; } - /** - * Return a valid ListView position or -1 if none is found - */ - private int canUseListViewForHighLighting(String key) { - if (getListView() == null) { - return -1; - } - - RecyclerView listView = getListView(); - RecyclerView.Adapter adapter = listView.getAdapter(); - - if (adapter != null && adapter instanceof PreferenceGroupAdapter) { - return findListPositionFromKey((PreferenceGroupAdapter) adapter, key); - } - - return -1; - } - @Override public RecyclerView.LayoutManager onCreateLayoutManager() { mLayoutManager = new LinearLayoutManager(getContext()); @@ -366,7 +339,9 @@ public abstract class SettingsPreferenceFragment extends InstrumentedPreferenceF @Override protected RecyclerView.Adapter onCreateAdapter(PreferenceScreen preferenceScreen) { - mAdapter = new HighlightablePreferenceGroupAdapter(preferenceScreen); + mAdapter = new HighlightablePreferenceGroupAdapter(preferenceScreen, + getArguments().getString(SettingsActivity.EXTRA_FRAGMENT_ARG_KEY), + mPreferenceHighlighted); return mAdapter; } @@ -375,7 +350,7 @@ public abstract class SettingsPreferenceFragment extends InstrumentedPreferenceF } protected void cacheRemoveAllPrefs(PreferenceGroup group) { - mPreferenceCache = new ArrayMap(); + mPreferenceCache = new ArrayMap<>(); final int N = group.getPreferenceCount(); for (int i = 0; i < N; i++) { Preference p = group.getPreference(i); @@ -401,29 +376,6 @@ public abstract class SettingsPreferenceFragment extends InstrumentedPreferenceF return mPreferenceCache != null ? mPreferenceCache.size() : 0; } - private void highlightPreference(String key) { - final int position = canUseListViewForHighLighting(key); - if (position < 0) { - return; - } - - mPreferenceHighlighted = true; - mLayoutManager.scrollToPosition(position); - mAdapter.highlight(position); - } - - private int findListPositionFromKey(PreferenceGroupAdapter adapter, String key) { - final int count = adapter.getItemCount(); - for (int n = 0; n < count; n++) { - final Preference preference = adapter.getItem(n); - final String preferenceKey = preference.getKey(); - if (preferenceKey != null && preferenceKey.equals(key)) { - return n; - } - } - return -1; - } - protected boolean removePreference(String key) { return removePreference(getPreferenceScreen(), key); } @@ -692,11 +644,11 @@ public abstract class SettingsPreferenceFragment extends InstrumentedPreferenceF } protected boolean hasNextButton() { - return ((ButtonBarHandler)getActivity()).hasNextButton(); + return ((ButtonBarHandler) getActivity()).hasNextButton(); } protected Button getNextButton() { - return ((ButtonBarHandler)getActivity()).getNextButton(); + return ((ButtonBarHandler) getActivity()).getNextButton(); } public void finish() { @@ -741,45 +693,10 @@ public abstract class SettingsPreferenceFragment extends InstrumentedPreferenceF } else { Log.w(TAG, "Parent isn't SettingsActivity nor PreferenceActivity, thus there's no way to " - + "launch the given Fragment (name: " + fragmentClass - + ", requestCode: " + requestCode + ")"); + + "launch the given Fragment (name: " + fragmentClass + + ", requestCode: " + requestCode + ")"); return false; } } - public static class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter { - - @VisibleForTesting(otherwise=VisibleForTesting.NONE) - int initialHighlightedPosition = -1; - - private int mHighlightPosition = -1; - - public HighlightablePreferenceGroupAdapter(PreferenceGroup preferenceGroup) { - super(preferenceGroup); - } - - public void highlight(int position) { - mHighlightPosition = position; - initialHighlightedPosition = position; - notifyDataSetChanged(); - } - - @Override - public void onBindViewHolder(PreferenceViewHolder holder, int position) { - super.onBindViewHolder(holder, position); - if (position == mHighlightPosition) { - View v = holder.itemView; - v.post(() -> { - if (v.getBackground() != null) { - final int centerX = v.getWidth() / 2; - final int centerY = v.getHeight() / 2; - v.getBackground().setHotspot(centerX, centerY); - } - v.setPressed(true); - v.setPressed(false); - mHighlightPosition = -1; - }); - } - } - } } diff --git a/src/com/android/settings/users/UserSettings.java b/src/com/android/settings/users/UserSettings.java index fcb8aef0b47..c8c4c25e035 100644 --- a/src/com/android/settings/users/UserSettings.java +++ b/src/com/android/settings/users/UserSettings.java @@ -309,7 +309,7 @@ public class UserSettings extends SettingsPreferenceFragment public void onDestroy() { super.onDestroy(); - if (!mUserCaps.mEnabled) { + if (mUserCaps == null || !mUserCaps.mEnabled) { return; } diff --git a/src/com/android/settings/widget/HighlightablePreferenceGroupAdapter.java b/src/com/android/settings/widget/HighlightablePreferenceGroupAdapter.java new file mode 100644 index 00000000000..e1999efef85 --- /dev/null +++ b/src/com/android/settings/widget/HighlightablePreferenceGroupAdapter.java @@ -0,0 +1,102 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.widget; + +import android.content.Context; +import android.support.annotation.VisibleForTesting; +import android.support.v7.preference.PreferenceGroup; +import android.support.v7.preference.PreferenceGroupAdapter; +import android.support.v7.preference.PreferenceViewHolder; +import android.support.v7.widget.RecyclerView; +import android.text.TextUtils; +import android.util.TypedValue; +import android.view.View; + +import com.android.settings.R; + +public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter { + + @VisibleForTesting + static final long DELAY_HIGHLIGHT_DURATION_MILLIS = 600L; + private static final long HIGHLIGHT_DURATION = 5000L; + + private final int mHighlightColor; + private final int mNormalBackgroundRes; + private final String mHighlightKey; + + private boolean mHighlightRequested; + private int mHighlightPosition = RecyclerView.NO_POSITION; + + public HighlightablePreferenceGroupAdapter(PreferenceGroup preferenceGroup, String key, + boolean highlightRequested) { + super(preferenceGroup); + mHighlightKey = key; + mHighlightRequested = highlightRequested; + final Context context = preferenceGroup.getContext(); + final TypedValue outValue = new TypedValue(); + context.getTheme().resolveAttribute(android.R.attr.selectableItemBackground, + outValue, true /* resolveRefs */); + mNormalBackgroundRes = outValue.resourceId; + mHighlightColor = context.getColor(R.color.preference_highligh_color); + } + + @Override + public void onBindViewHolder(PreferenceViewHolder holder, int position) { + super.onBindViewHolder(holder, position); + updateBackground(holder, position); + } + + @VisibleForTesting + void updateBackground(PreferenceViewHolder holder, int position) { + View v = holder.itemView; + if (position == mHighlightPosition) { + v.setBackgroundColor(mHighlightColor); + v.setTag(R.id.preference_highlighted, true); + v.postDelayed(() -> { + mHighlightPosition = RecyclerView.NO_POSITION; + removeHighlightBackground(v); + }, HIGHLIGHT_DURATION); + } else if (Boolean.TRUE.equals(v.getTag(R.id.preference_highlighted))) { + removeHighlightBackground(v); + } + } + + public void requestHighlight(View root, RecyclerView recyclerView) { + if (mHighlightRequested || recyclerView == null || TextUtils.isEmpty(mHighlightKey)) { + return; + } + root.postDelayed(() -> { + final int position = getPreferenceAdapterPosition(mHighlightKey); + if (position < 0) { + return; + } + mHighlightRequested = true; + recyclerView.getLayoutManager().scrollToPosition(position); + mHighlightPosition = position; + notifyItemChanged(position); + }, DELAY_HIGHLIGHT_DURATION_MILLIS); + } + + public boolean isHighlightRequested() { + return mHighlightRequested; + } + + private void removeHighlightBackground(View v) { + v.setBackgroundResource(mNormalBackgroundRes); + v.setTag(R.id.preference_highlighted, false); + } +} diff --git a/tests/robotests/src/com/android/settings/widget/HighlightablePreferenceGroupAdapterTest.java b/tests/robotests/src/com/android/settings/widget/HighlightablePreferenceGroupAdapterTest.java new file mode 100644 index 00000000000..e2fb6c16bf6 --- /dev/null +++ b/tests/robotests/src/com/android/settings/widget/HighlightablePreferenceGroupAdapterTest.java @@ -0,0 +1,127 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.widget; + + +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + +import android.content.Context; +import android.graphics.drawable.ColorDrawable; +import android.support.v7.preference.PreferenceCategory; +import android.support.v7.preference.PreferenceViewHolder; +import android.support.v7.widget.RecyclerView; +import android.view.View; + +import com.android.settings.R; +import com.android.settings.TestConfig; +import com.android.settings.testutils.SettingsRobolectricTestRunner; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.util.ReflectionHelpers; + +@RunWith(SettingsRobolectricTestRunner.class) +@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) +public class HighlightablePreferenceGroupAdapterTest { + + private static final String TEST_KEY = "key"; + + @Mock + private View mRoot; + @Mock + private PreferenceCategory mPreferenceCatetory; + private Context mContext; + private HighlightablePreferenceGroupAdapter mAdapter; + private PreferenceViewHolder mViewHolder; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mContext = RuntimeEnvironment.application; + when(mPreferenceCatetory.getContext()).thenReturn(mContext); + mAdapter = new HighlightablePreferenceGroupAdapter(mPreferenceCatetory, TEST_KEY, + false /* highlighted*/); + mViewHolder = PreferenceViewHolder.createInstanceForTests( + View.inflate(mContext, R.layout.app_preference_item, null)); + } + + @Test + public void requestHighlight_hasKey_notHighlightedBefore_shouldRequest() { + mAdapter.requestHighlight(mRoot, mock(RecyclerView.class)); + + verify(mRoot).postDelayed(any(), + eq(HighlightablePreferenceGroupAdapter.DELAY_HIGHLIGHT_DURATION_MILLIS)); + } + + @Test + public void requestHighlight_noKey_highlightedBefore_noRecyclerView_shouldNotRequest() { + ReflectionHelpers.setField(mAdapter, "mHighlightKey", null); + ReflectionHelpers.setField(mAdapter, "mHighlightRequested", false); + mAdapter.requestHighlight(mRoot, mock(RecyclerView.class)); + + ReflectionHelpers.setField(mAdapter, "mHighlightKey", TEST_KEY); + ReflectionHelpers.setField(mAdapter, "mHighlightRequested", true); + mAdapter.requestHighlight(mRoot, mock(RecyclerView.class)); + + ReflectionHelpers.setField(mAdapter, "mHighlightKey", TEST_KEY); + ReflectionHelpers.setField(mAdapter, "mHighlightRequested", false); + mAdapter.requestHighlight(mRoot, null /* recyclerView */); + + verifyZeroInteractions(mRoot); + } + + @Test + public void updateBackground_notHighlightedRow_shouldNotSetHighlightedTag() { + ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10); + + mAdapter.updateBackground(mViewHolder, 0); + + assertThat(mViewHolder.itemView.getTag(R.id.preference_highlighted)).isNull(); + } + + @Test + public void updateBackground_highlight_shouldChangeBackgroundAndSetHighlightedTag() { + ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10); + + mAdapter.updateBackground(mViewHolder, 10); + assertThat(mViewHolder.itemView.getBackground()).isInstanceOf(ColorDrawable.class); + assertThat(mViewHolder.itemView.getTag(R.id.preference_highlighted)).isEqualTo(true); + } + + @Test + public void updateBackground_reuseHightlightedRowForNormalRow_shouldResetBackgroundAndTag() { + ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10); + mViewHolder.itemView.setTag(R.id.preference_highlighted, true); + + mAdapter.updateBackground(mViewHolder, 0); + + assertThat(mViewHolder.itemView.getBackground()).isNotInstanceOf(ColorDrawable.class); + assertThat(mViewHolder.itemView.getTag(R.id.preference_highlighted)).isEqualTo(false); + } + +} diff --git a/tests/unit/src/com/android/settings/SettingsPreferenceFragmentTest.java b/tests/unit/src/com/android/settings/SettingsPreferenceFragmentTest.java deleted file mode 100644 index 0e12e797365..00000000000 --- a/tests/unit/src/com/android/settings/SettingsPreferenceFragmentTest.java +++ /dev/null @@ -1,70 +0,0 @@ -package com.android.settings; - -import static com.google.common.truth.Truth.assertThat; - -import android.app.Instrumentation; -import android.content.Context; -import android.content.Intent; -import android.os.Bundle; -import android.support.test.InstrumentationRegistry; -import android.support.test.filters.SmallTest; -import android.support.test.runner.AndroidJUnit4; -import android.support.v7.preference.Preference; -import android.support.v7.preference.PreferenceGroupAdapter; - -import com.android.settings.accessibility.AccessibilitySettings; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; - -@RunWith(AndroidJUnit4.class) -@SmallTest -public class SettingsPreferenceFragmentTest { - - private Instrumentation mInstrumentation; - private Context mTargetContext; - - @Before - public void setUp() throws Exception { - mInstrumentation = InstrumentationRegistry.getInstrumentation(); - mTargetContext = mInstrumentation.getTargetContext(); - } - - @Test - public void testHighlightCaptions() throws InterruptedException { - final String prefKey = "captioning_preference_screen"; - Bundle args = new Bundle(); - args.putString(SettingsActivity.EXTRA_FRAGMENT_ARG_KEY, prefKey); - - Intent intent = new Intent(Intent.ACTION_MAIN); - intent.setClass(mTargetContext, SubSettings.class); - intent.putExtra(SettingsActivity.EXTRA_SHOW_FRAGMENT, - "com.android.settings.accessibility.AccessibilitySettings"); - intent.putExtra(SettingsActivity.EXTRA_SHOW_FRAGMENT_ARGUMENTS, args); - - SettingsActivity activity = (SettingsActivity) mInstrumentation.startActivitySync(intent); - AccessibilitySettings fragment = (AccessibilitySettings) - activity.getFragmentManager().getFragments().get(0); - - // Allow time for highlight from post-delay. - Thread.sleep(SettingsPreferenceFragment.DELAY_HIGHLIGHT_DURATION_MILLIS); - if (!fragment.mPreferenceHighlighted) { - Thread.sleep(SettingsPreferenceFragment.DELAY_HIGHLIGHT_DURATION_MILLIS); - } - - int prefPosition = -1; - PreferenceGroupAdapter adapter = (PreferenceGroupAdapter) - fragment.getListView().getAdapter(); - for (int n = 0, count = adapter.getItemCount(); n < count; n++) { - final Preference preference = adapter.getItem(n); - final String preferenceKey = preference.getKey(); - if (preferenceKey.equals(prefKey)) { - prefPosition = n; - break; - } - } - - assertThat(fragment.mAdapter.initialHighlightedPosition).isEqualTo(prefPosition); - } -}