Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 582f4a96 authored by Tsung-Mao Fang's avatar Tsung-Mao Fang
Browse files

Fix search highlight issues 

This cl fixes two kinds of issues.
1) Highlight two different preferences while clicking a search result.
2) Preference only is  highlighted one time.

For the first problem, we don't allow to reuse view holder in the
highlight scenario, and then allow it later.

The root cause of second problem is scrolling to target preference
needs x milliseconds. Thus, we should wait a few milliseconds, and
then start to highlight preference.

Test: Test some search results, and see the correct behavior
Fix: 186060148
Fix: 186010165
Fix: 187886982
Change-Id: I7df3e34efe39ee386fe9ce91d7d6c52cf390e2e7
parent 15d75cf5

src/com/android/settings/widget/HighlightablePreferenceGroupAdapter.java

+20 −9
Original line number Original line Diff line number Diff line
@@ -121,10 +121,10 @@ public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter 
                && (mHighlightKey != null

                && (mHighlightKey != null

                && TextUtils.equals(mHighlightKey, getItem(position).getKey()))) {

                && TextUtils.equals(mHighlightKey, getItem(position).getKey()))) {

            // This position should be highlighted. If it's highlighted before - skip animation.

            // This position should be highlighted. If it's highlighted before - skip animation.

            addHighlightBackground(v, !mFadeInAnimated);

            addHighlightBackground(holder, !mFadeInAnimated);

        } else if (Boolean.TRUE.equals(v.getTag(R.id.preference_highlighted))) {

        } else if (Boolean.TRUE.equals(v.getTag(R.id.preference_highlighted))) {

            // View with highlight is reused for a view that should not have highlight

            // View with highlight is reused for a view that should not have highlight

            removeHighlightBackground(v, false /* animate */);

            removeHighlightBackground(holder, false /* animate */);

        }

        }

    }

    }
@@ -141,20 +141,26 @@ public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter 
            return;

            return;

        }

        }





        // Collapse app bar after 300 milliseconds.

        if (appBarLayout != null) {

        if (appBarLayout != null) {

            root.postDelayed(() -> {

            root.postDelayed(() -> {

                appBarLayout.setExpanded(false, true);

                appBarLayout.setExpanded(false, true);

            }, DELAY_COLLAPSE_DURATION_MILLIS);

            }, DELAY_COLLAPSE_DURATION_MILLIS);

        }

        }





        // Scroll to correct position after 600 milliseconds.

        root.postDelayed(() -> {

        root.postDelayed(() -> {

            mHighlightRequested = true;

            mHighlightRequested = true;

            // Remove the animator to avoid a RecyclerView crash.

            // Remove the animator to avoid a RecyclerView crash.

            recyclerView.setItemAnimator(null);

            recyclerView.setItemAnimator(null);

            recyclerView.smoothScrollToPosition(position);

            recyclerView.smoothScrollToPosition(position);

            mHighlightPosition = position;

            mHighlightPosition = position;

            notifyItemChanged(position);

        }, DELAY_HIGHLIGHT_DURATION_MILLIS);

        }, DELAY_HIGHLIGHT_DURATION_MILLIS);



        // Highlight preference after 900 milliseconds.

        root.postDelayed(() -> {

            notifyItemChanged(position);

        }, DELAY_COLLAPSE_DURATION_MILLIS + DELAY_HIGHLIGHT_DURATION_MILLIS);

    }

    }





    public boolean isHighlightRequested() {

    public boolean isHighlightRequested() {
@@ -162,19 +168,21 @@ public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter 
    }

    }





    @VisibleForTesting

    @VisibleForTesting

    void requestRemoveHighlightDelayed(View v) {

    void requestRemoveHighlightDelayed(PreferenceViewHolder holder) {

        final View v = holder.itemView;

        v.postDelayed(() -> {

        v.postDelayed(() -> {

            mHighlightPosition = RecyclerView.NO_POSITION;

            mHighlightPosition = RecyclerView.NO_POSITION;

            removeHighlightBackground(v, true /* animate */);

            removeHighlightBackground(holder, true /* animate */);

        }, HIGHLIGHT_DURATION);

        }, HIGHLIGHT_DURATION);

    }

    }





    private void addHighlightBackground(View v, boolean animate) {

    private void addHighlightBackground(PreferenceViewHolder holder, boolean animate) {

        final View v = holder.itemView;

        v.setTag(R.id.preference_highlighted, true);

        v.setTag(R.id.preference_highlighted, true);

        if (!animate) {

        if (!animate) {

            v.setBackgroundColor(mHighlightColor);

            v.setBackgroundColor(mHighlightColor);

            Log.d(TAG, "AddHighlight: Not animation requested - setting highlight background");

            Log.d(TAG, "AddHighlight: Not animation requested - setting highlight background");

            requestRemoveHighlightDelayed(v);

            requestRemoveHighlightDelayed(holder);

            return;

            return;

        }

        }

        mFadeInAnimated = true;

        mFadeInAnimated = true;
@@ -189,10 +197,12 @@ public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter 
        fadeInLoop.setRepeatCount(4);

        fadeInLoop.setRepeatCount(4);

        fadeInLoop.start();

        fadeInLoop.start();

        Log.d(TAG, "AddHighlight: starting fade in animation");

        Log.d(TAG, "AddHighlight: starting fade in animation");

        requestRemoveHighlightDelayed(v);

        holder.setIsRecyclable(false);

        requestRemoveHighlightDelayed(holder);

    }

    }





    private void removeHighlightBackground(View v, boolean animate) {

    private void removeHighlightBackground(PreferenceViewHolder holder, boolean animate) {

        final View v = holder.itemView;

        if (!animate) {

        if (!animate) {

            v.setTag(R.id.preference_highlighted, false);

            v.setTag(R.id.preference_highlighted, false);

            v.setBackgroundResource(mNormalBackgroundRes);

            v.setBackgroundResource(mNormalBackgroundRes);
@@ -220,6 +230,7 @@ public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter 
                // Animation complete - the background is now white. Change to mNormalBackgroundRes

                // Animation complete - the background is now white. Change to mNormalBackgroundRes

                // so it is white and has ripple on touch.

                // so it is white and has ripple on touch.

                v.setBackgroundResource(mNormalBackgroundRes);

                v.setBackgroundResource(mNormalBackgroundRes);

                holder.setIsRecyclable(true);

            }

            }

        });

        });

        colorAnimation.start();

        colorAnimation.start();

tests/robotests/src/com/android/settings/widget/HighlightablePreferenceGroupAdapterTest.java

+3 −3
Original line number Original line Diff line number Diff line
@@ -184,7 +184,7 @@ public class HighlightablePreferenceGroupAdapterTest { 
        assertThat(mAdapter.mFadeInAnimated).isTrue();

        assertThat(mAdapter.mFadeInAnimated).isTrue();

        assertThat(mViewHolder.itemView.getBackground()).isInstanceOf(ColorDrawable.class);

        assertThat(mViewHolder.itemView.getBackground()).isInstanceOf(ColorDrawable.class);

        assertThat(mViewHolder.itemView.getTag(R.id.preference_highlighted)).isEqualTo(true);

        assertThat(mViewHolder.itemView.getTag(R.id.preference_highlighted)).isEqualTo(true);

        verify(mAdapter).requestRemoveHighlightDelayed(mViewHolder.itemView);

        verify(mAdapter).requestRemoveHighlightDelayed(mViewHolder);

    }

    }





    @Test

    @Test
@@ -197,14 +197,14 @@ public class HighlightablePreferenceGroupAdapterTest { 
        // through animation.

        // through animation.

        assertThat(mAdapter.mFadeInAnimated).isTrue();

        assertThat(mAdapter.mFadeInAnimated).isTrue();

        // remove highlight should be requested.

        // remove highlight should be requested.

        verify(mAdapter).requestRemoveHighlightDelayed(mViewHolder.itemView);

        verify(mAdapter).requestRemoveHighlightDelayed(mViewHolder);





        ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10);

        ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10);

        mAdapter.updateBackground(mViewHolder, 10);

        mAdapter.updateBackground(mViewHolder, 10);

        // only sets background color once - if it's animation this would be called many times

        // only sets background color once - if it's animation this would be called many times

        verify(mViewHolder.itemView).setBackgroundColor(mAdapter.mHighlightColor);

        verify(mViewHolder.itemView).setBackgroundColor(mAdapter.mHighlightColor);

        // remove highlight should be requested.

        // remove highlight should be requested.

        verify(mAdapter, times(2)).requestRemoveHighlightDelayed(mViewHolder.itemView);

        verify(mAdapter, times(2)).requestRemoveHighlightDelayed(mViewHolder);

    }

    }





    @Test

    @Test