Fix the a11y focus problem in a long list page
While a page is scrolling to the target position, the target can't request a11y focus, and the highlight blink animation at the moment is also invisible. The patch is to skip the unseen highlight animation and focus operation when onBindViewHolder() is called before scrolling. Therefore, when the scrolling stops, the scroll listener will notify the item changed and onBindViewHolder() will be called again to do the operations. Also decrease the delay time of the scrolling start to make the a11y smoother. Bug: 318459003 Test: manual, robotest Change-Id: Ie55b2edeb7e1feaaadb5e670f39f07f6f17b92b9
This commit is contained in:
@@ -40,6 +40,7 @@ import androidx.recyclerview.widget.RecyclerView.ViewHolder;
|
|||||||
|
|
||||||
import com.android.settings.R;
|
import com.android.settings.R;
|
||||||
import com.android.settings.SettingsPreferenceFragment;
|
import com.android.settings.SettingsPreferenceFragment;
|
||||||
|
import com.android.settings.accessibility.AccessibilityUtil;
|
||||||
|
|
||||||
import com.google.android.material.appbar.AppBarLayout;
|
import com.google.android.material.appbar.AppBarLayout;
|
||||||
|
|
||||||
@@ -50,6 +51,8 @@ public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter
|
|||||||
static final long DELAY_COLLAPSE_DURATION_MILLIS = 300L;
|
static final long DELAY_COLLAPSE_DURATION_MILLIS = 300L;
|
||||||
@VisibleForTesting
|
@VisibleForTesting
|
||||||
static final long DELAY_HIGHLIGHT_DURATION_MILLIS = 600L;
|
static final long DELAY_HIGHLIGHT_DURATION_MILLIS = 600L;
|
||||||
|
@VisibleForTesting
|
||||||
|
static final long DELAY_HIGHLIGHT_DURATION_MILLIS_A11Y = 300L;
|
||||||
private static final long HIGHLIGHT_DURATION = 15000L;
|
private static final long HIGHLIGHT_DURATION = 15000L;
|
||||||
private static final long HIGHLIGHT_FADE_OUT_DURATION = 500L;
|
private static final long HIGHLIGHT_FADE_OUT_DURATION = 500L;
|
||||||
private static final long HIGHLIGHT_FADE_IN_DURATION = 200L;
|
private static final long HIGHLIGHT_FADE_IN_DURATION = 200L;
|
||||||
@@ -59,6 +62,7 @@ public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter
|
|||||||
@VisibleForTesting
|
@VisibleForTesting
|
||||||
boolean mFadeInAnimated;
|
boolean mFadeInAnimated;
|
||||||
|
|
||||||
|
private final Context mContext;
|
||||||
private final int mNormalBackgroundRes;
|
private final int mNormalBackgroundRes;
|
||||||
private final String mHighlightKey;
|
private final String mHighlightKey;
|
||||||
private boolean mHighlightRequested;
|
private boolean mHighlightRequested;
|
||||||
@@ -102,12 +106,12 @@ public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter
|
|||||||
super(preferenceGroup);
|
super(preferenceGroup);
|
||||||
mHighlightKey = key;
|
mHighlightKey = key;
|
||||||
mHighlightRequested = highlightRequested;
|
mHighlightRequested = highlightRequested;
|
||||||
final Context context = preferenceGroup.getContext();
|
mContext = preferenceGroup.getContext();
|
||||||
final TypedValue outValue = new TypedValue();
|
final TypedValue outValue = new TypedValue();
|
||||||
context.getTheme().resolveAttribute(android.R.attr.selectableItemBackground,
|
mContext.getTheme().resolveAttribute(android.R.attr.selectableItemBackground,
|
||||||
outValue, true /* resolveRefs */);
|
outValue, true /* resolveRefs */);
|
||||||
mNormalBackgroundRes = outValue.resourceId;
|
mNormalBackgroundRes = outValue.resourceId;
|
||||||
mHighlightColor = context.getColor(R.color.preference_highlight_color);
|
mHighlightColor = mContext.getColor(R.color.preference_highlight_color);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@@ -121,12 +125,11 @@ public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter
|
|||||||
View v = holder.itemView;
|
View v = holder.itemView;
|
||||||
if (position == mHighlightPosition
|
if (position == mHighlightPosition
|
||||||
&& (mHighlightKey != null
|
&& (mHighlightKey != null
|
||||||
&& TextUtils.equals(mHighlightKey, getItem(position).getKey()))) {
|
&& TextUtils.equals(mHighlightKey, getItem(position).getKey()))
|
||||||
|
&& v.isShown()) {
|
||||||
// This position should be highlighted. If it's highlighted before - skip animation.
|
// This position should be highlighted. If it's highlighted before - skip animation.
|
||||||
|
v.requestAccessibilityFocus();
|
||||||
addHighlightBackground(holder, !mFadeInAnimated);
|
addHighlightBackground(holder, !mFadeInAnimated);
|
||||||
if (v != null) {
|
|
||||||
v.requestAccessibilityFocus();
|
|
||||||
}
|
|
||||||
} 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(holder, false /* animate */);
|
removeHighlightBackground(holder, false /* animate */);
|
||||||
@@ -157,13 +160,14 @@ public class HighlightablePreferenceGroupAdapter extends PreferenceGroupAdapter
|
|||||||
|
|
||||||
// Remove the animator as early as possible to avoid a RecyclerView crash.
|
// Remove the animator as early as possible to avoid a RecyclerView crash.
|
||||||
recyclerView.setItemAnimator(null);
|
recyclerView.setItemAnimator(null);
|
||||||
// Scroll to correct position after 600 milliseconds.
|
// Scroll to correct position after a short delay.
|
||||||
root.postDelayed(() -> {
|
root.postDelayed(() -> {
|
||||||
if (ensureHighlightPosition()) {
|
if (ensureHighlightPosition()) {
|
||||||
recyclerView.smoothScrollToPosition(mHighlightPosition);
|
recyclerView.smoothScrollToPosition(mHighlightPosition);
|
||||||
highlightAndFocusTargetItem(recyclerView, mHighlightPosition);
|
highlightAndFocusTargetItem(recyclerView, mHighlightPosition);
|
||||||
}
|
}
|
||||||
}, DELAY_HIGHLIGHT_DURATION_MILLIS);
|
}, AccessibilityUtil.isTouchExploreEnabled(mContext)
|
||||||
|
? DELAY_HIGHLIGHT_DURATION_MILLIS_A11Y : DELAY_HIGHLIGHT_DURATION_MILLIS);
|
||||||
}
|
}
|
||||||
|
|
||||||
private void highlightAndFocusTargetItem(RecyclerView recyclerView, int highlightPosition) {
|
private void highlightAndFocusTargetItem(RecyclerView recyclerView, int highlightPosition) {
|
||||||
|
@@ -33,6 +33,7 @@ import android.content.Context;
|
|||||||
import android.graphics.drawable.ColorDrawable;
|
import android.graphics.drawable.ColorDrawable;
|
||||||
import android.os.Bundle;
|
import android.os.Bundle;
|
||||||
import android.view.View;
|
import android.view.View;
|
||||||
|
import android.view.accessibility.AccessibilityManager;
|
||||||
|
|
||||||
import androidx.preference.Preference;
|
import androidx.preference.Preference;
|
||||||
import androidx.preference.PreferenceCategory;
|
import androidx.preference.PreferenceCategory;
|
||||||
@@ -54,6 +55,8 @@ import org.mockito.MockitoAnnotations;
|
|||||||
import org.robolectric.RobolectricTestRunner;
|
import org.robolectric.RobolectricTestRunner;
|
||||||
import org.robolectric.RuntimeEnvironment;
|
import org.robolectric.RuntimeEnvironment;
|
||||||
import org.robolectric.annotation.Config;
|
import org.robolectric.annotation.Config;
|
||||||
|
import org.robolectric.shadow.api.Shadow;
|
||||||
|
import org.robolectric.shadows.ShadowAccessibilityManager;
|
||||||
import org.robolectric.util.ReflectionHelpers;
|
import org.robolectric.util.ReflectionHelpers;
|
||||||
|
|
||||||
@RunWith(RobolectricTestRunner.class)
|
@RunWith(RobolectricTestRunner.class)
|
||||||
@@ -67,7 +70,7 @@ public class HighlightablePreferenceGroupAdapterTest {
|
|||||||
@Mock
|
@Mock
|
||||||
private View mRoot;
|
private View mRoot;
|
||||||
@Mock
|
@Mock
|
||||||
private PreferenceCategory mPreferenceCatetory;
|
private PreferenceCategory mPreferenceCategory;
|
||||||
@Mock
|
@Mock
|
||||||
private SettingsPreferenceFragment mFragment;
|
private SettingsPreferenceFragment mFragment;
|
||||||
|
|
||||||
@@ -82,8 +85,8 @@ public class HighlightablePreferenceGroupAdapterTest {
|
|||||||
mContext = RuntimeEnvironment.application;
|
mContext = RuntimeEnvironment.application;
|
||||||
mPreference = new Preference(mContext);
|
mPreference = new Preference(mContext);
|
||||||
mPreference.setKey(TEST_KEY);
|
mPreference.setKey(TEST_KEY);
|
||||||
when(mPreferenceCatetory.getContext()).thenReturn(mContext);
|
when(mPreferenceCategory.getContext()).thenReturn(mContext);
|
||||||
mAdapter = spy(new HighlightablePreferenceGroupAdapter(mPreferenceCatetory, TEST_KEY,
|
mAdapter = spy(new HighlightablePreferenceGroupAdapter(mPreferenceCategory, TEST_KEY,
|
||||||
false /* highlighted*/));
|
false /* highlighted*/));
|
||||||
when(mAdapter.getItem(anyInt())).thenReturn(mPreference);
|
when(mAdapter.getItem(anyInt())).thenReturn(mPreference);
|
||||||
mViewHolder = PreferenceViewHolder.createInstanceForTests(
|
mViewHolder = PreferenceViewHolder.createInstanceForTests(
|
||||||
@@ -101,6 +104,18 @@ public class HighlightablePreferenceGroupAdapterTest {
|
|||||||
eq(HighlightablePreferenceGroupAdapter.DELAY_HIGHLIGHT_DURATION_MILLIS));
|
eq(HighlightablePreferenceGroupAdapter.DELAY_HIGHLIGHT_DURATION_MILLIS));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void requestHighlight_enableTouchExploration_shouldHaveA11yHighlightDelay() {
|
||||||
|
ShadowAccessibilityManager am = Shadow.extract(AccessibilityManager.getInstance(mContext));
|
||||||
|
am.setTouchExplorationEnabled(true);
|
||||||
|
when(mAdapter.getPreferenceAdapterPosition(anyString())).thenReturn(1);
|
||||||
|
mAdapter.requestHighlight(mRoot, mock(RecyclerView.class), mock(AppBarLayout.class));
|
||||||
|
|
||||||
|
// DELAY_HIGHLIGHT_DURATION_MILLIS_A11Y = DELAY_COLLAPSE_DURATION_MILLIS
|
||||||
|
verify(mRoot, times(2)).postDelayed(any(),
|
||||||
|
eq(HighlightablePreferenceGroupAdapter.DELAY_HIGHLIGHT_DURATION_MILLIS_A11Y));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void requestHighlight_noKey_highlightedBefore_noRecyclerView_shouldNotRequest() {
|
public void requestHighlight_noKey_highlightedBefore_noRecyclerView_shouldNotRequest() {
|
||||||
ReflectionHelpers.setField(mAdapter, "mHighlightKey", null);
|
ReflectionHelpers.setField(mAdapter, "mHighlightKey", null);
|
||||||
@@ -178,12 +193,24 @@ public class HighlightablePreferenceGroupAdapterTest {
|
|||||||
assertThat(mViewHolder.itemView.getTag(R.id.preference_highlighted)).isNull();
|
assertThat(mViewHolder.itemView.getTag(R.id.preference_highlighted)).isNull();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void updateBackground_itemViewIsInvisible_shouldNotSetHighlightedTag() {
|
||||||
|
ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10);
|
||||||
|
ReflectionHelpers.setField(mViewHolder, "itemView", spy(mViewHolder.itemView));
|
||||||
|
when(mViewHolder.itemView.isShown()).thenReturn(false);
|
||||||
|
|
||||||
|
mAdapter.updateBackground(mViewHolder, 0);
|
||||||
|
|
||||||
|
assertThat(mViewHolder.itemView.getTag(R.id.preference_highlighted)).isNull();
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* When background is being updated, we also request the a11y focus on the preference
|
* When background is being updated, we also request the a11y focus on the preference
|
||||||
*/
|
*/
|
||||||
@Test
|
@Test
|
||||||
public void updateBackground_shouldRequestAccessibilityFocus() {
|
public void updateBackground_shouldRequestAccessibilityFocus() {
|
||||||
View viewItem = mock(View.class);
|
View viewItem = mock(View.class);
|
||||||
|
when(viewItem.isShown()).thenReturn(true);
|
||||||
mViewHolder = PreferenceViewHolder.createInstanceForTests(viewItem);
|
mViewHolder = PreferenceViewHolder.createInstanceForTests(viewItem);
|
||||||
ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10);
|
ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10);
|
||||||
|
|
||||||
@@ -195,6 +222,8 @@ public class HighlightablePreferenceGroupAdapterTest {
|
|||||||
@Test
|
@Test
|
||||||
public void updateBackground_highlight_shouldAnimateBackgroundAndSetHighlightedTag() {
|
public void updateBackground_highlight_shouldAnimateBackgroundAndSetHighlightedTag() {
|
||||||
ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10);
|
ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10);
|
||||||
|
ReflectionHelpers.setField(mViewHolder, "itemView", spy(mViewHolder.itemView));
|
||||||
|
when(mViewHolder.itemView.isShown()).thenReturn(true);
|
||||||
assertThat(mAdapter.mFadeInAnimated).isFalse();
|
assertThat(mAdapter.mFadeInAnimated).isFalse();
|
||||||
|
|
||||||
mAdapter.updateBackground(mViewHolder, 10);
|
mAdapter.updateBackground(mViewHolder, 10);
|
||||||
@@ -205,10 +234,22 @@ public class HighlightablePreferenceGroupAdapterTest {
|
|||||||
verify(mAdapter).requestRemoveHighlightDelayed(mViewHolder);
|
verify(mAdapter).requestRemoveHighlightDelayed(mViewHolder);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void updateBackground_highlight_itemViewIsInvisible_shouldNotAnimate() {
|
||||||
|
ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10);
|
||||||
|
ReflectionHelpers.setField(mViewHolder, "itemView", spy(mViewHolder.itemView));
|
||||||
|
when(mViewHolder.itemView.isShown()).thenReturn(false);
|
||||||
|
|
||||||
|
mAdapter.updateBackground(mViewHolder, 10);
|
||||||
|
|
||||||
|
assertThat(mAdapter.mFadeInAnimated).isFalse();
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void updateBackgroundTwice_highlight_shouldAnimateOnce() {
|
public void updateBackgroundTwice_highlight_shouldAnimateOnce() {
|
||||||
ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10);
|
ReflectionHelpers.setField(mAdapter, "mHighlightPosition", 10);
|
||||||
ReflectionHelpers.setField(mViewHolder, "itemView", spy(mViewHolder.itemView));
|
ReflectionHelpers.setField(mViewHolder, "itemView", spy(mViewHolder.itemView));
|
||||||
|
when(mViewHolder.itemView.isShown()).thenReturn(true);
|
||||||
assertThat(mAdapter.mFadeInAnimated).isFalse();
|
assertThat(mAdapter.mFadeInAnimated).isFalse();
|
||||||
mAdapter.updateBackground(mViewHolder, 10);
|
mAdapter.updateBackground(mViewHolder, 10);
|
||||||
// mFadeInAnimated change from false to true - indicating background change is scheduled
|
// mFadeInAnimated change from false to true - indicating background change is scheduled
|
||||||
|
Reference in New Issue
Block a user