From 8a71e7741a05846979ae237768f30ae5d833eb4f Mon Sep 17 00:00:00 2001 From: Material Design Team Date: Thu, 8 Feb 2024 17:02:01 +0000 Subject: [PATCH] [AppBarLayout] Use an accessibility delegate to add and perform actions This replaces the use of `ViewCompat#add/removeAccessibilityAction` Both are valid strategies, but each call to `remove` or `add` triggers an accessibility event. Since this check is done in layout (originally to fix a11y scroll state) it's sending a high number of events that create noise for accessibility services. To avoid this, we move this code to the delegate `onInitialize` and `performAction` methods. Instead of the view dynamically adding and removing actions to itself, the node is initialized with actions only when an a11y service sends a request with a new node (likely due to some UI change). The flow here would look like: 1. UI is scrolled/page is loaded 2. TalkBack gets a scroll event/content change event 3. TalkBack requests new snapshot of the screen 4. ABL populates the node with the actions For a simple scroll, this change reduces the events from ~40 to ~10. We also add the Truth library for clearer assertions. PiperOrigin-RevId: 605333170 --- .../android/material/appbar/AppBarLayout.java | 182 ++++++++---------- .../material/appbar/AppBarLayoutBaseTest.java | 29 ++- 2 files changed, 96 insertions(+), 115 deletions(-) diff --git a/lib/java/com/google/android/material/appbar/AppBarLayout.java b/lib/java/com/google/android/material/appbar/AppBarLayout.java index 22526512793..c27b08ec76d 100644 --- a/lib/java/com/google/android/material/appbar/AppBarLayout.java +++ b/lib/java/com/google/android/material/appbar/AppBarLayout.java @@ -38,6 +38,7 @@ import android.os.Build; import android.os.Build.VERSION; import android.os.Build.VERSION_CODES; +import android.os.Bundle; import android.os.Parcel; import android.os.Parcelable; import androidx.appcompat.content.res.AppCompatResources; @@ -68,8 +69,6 @@ import androidx.core.view.ViewCompat.NestedScrollType; import androidx.core.view.WindowInsetsCompat; import androidx.core.view.accessibility.AccessibilityNodeInfoCompat; -import androidx.core.view.accessibility.AccessibilityNodeInfoCompat.AccessibilityActionCompat; -import androidx.core.view.accessibility.AccessibilityViewCommand; import androidx.customview.view.AbsSavedState; import com.google.android.material.animation.AnimationUtils; import com.google.android.material.appbar.AppBarLayout.BaseBehavior.SavedState; @@ -1516,8 +1515,6 @@ public abstract static class BaseDragCallback { @Nullable private WeakReference lastNestedScrollingChildRef; private BaseDragCallback onDragCallback; - private boolean coordinatorLayoutA11yScrollable; - public BaseBehavior() {} public BaseBehavior(Context context, AttributeSet attrs) { @@ -1610,7 +1607,7 @@ public void onNestedScroll( if (dyUnconsumed == 0) { // The scrolling view may scroll to the top of its content without updating the actions, so // update here. - updateAccessibilityActions(coordinatorLayout, child); + addAccessibilityDelegateIfNeeded(coordinatorLayout, child); } } @@ -1870,30 +1867,12 @@ public boolean onLayoutChild( // Make sure we dispatch the offset update abl.onOffsetChanged(getTopAndBottomOffset()); - updateAccessibilityActions(parent, abl); + addAccessibilityDelegateIfNeeded(parent, abl); return handled; } - private void updateAccessibilityActions( + private void addAccessibilityDelegateIfNeeded( CoordinatorLayout coordinatorLayout, @NonNull T appBarLayout) { - ViewCompat.removeAccessibilityAction(coordinatorLayout, ACTION_SCROLL_FORWARD.getId()); - ViewCompat.removeAccessibilityAction(coordinatorLayout, ACTION_SCROLL_BACKWARD.getId()); - // Don't add a11y actions if the abl has no scroll range. - if (appBarLayout.getTotalScrollRange() == 0) { - return; - } - // Don't add actions if a child view doesn't have the behavior that will cause the abl to - // scroll. - View scrollingView = getChildWithScrollingBehavior(coordinatorLayout); - if (scrollingView == null) { - return; - } - - // Don't add actions if the children do not have scrolling flags. - if (!childrenHaveScrollFlags(appBarLayout)) { - return; - } - if (!ViewCompat.hasAccessibilityDelegate(coordinatorLayout)) { ViewCompat.setAccessibilityDelegate( coordinatorLayout, @@ -1902,14 +1881,87 @@ private void updateAccessibilityActions( public void onInitializeAccessibilityNodeInfo( View host, @NonNull AccessibilityNodeInfoCompat info) { super.onInitializeAccessibilityNodeInfo(host, info); - info.setScrollable(coordinatorLayoutA11yScrollable); info.setClassName(ScrollView.class.getName()); + if (appBarLayout.getTotalScrollRange() == 0) { + return; + } + View scrollingView = getChildWithScrollingBehavior(coordinatorLayout); + // Don't add actions if a child view doesn't have the behavior that will cause the + // ABL to scroll. + if (scrollingView == null) { + return; + } + + // Don't add actions if the children do not have scrolling flags. + if (!childrenHaveScrollFlags(appBarLayout)) { + return; + } + + if (getTopBottomOffsetForScrollingSibling() + != -appBarLayout.getTotalScrollRange()) { + // Add a collapsing action/forward if the view offset isn't the ABL scroll range. + // (The same offset means the view is completely collapsed). + info.addAction(ACTION_SCROLL_FORWARD); + info.setScrollable(true); + } + + // Don't add an expanding action if the sibling offset is 0, which would mean the + // ABL is completely expanded. + if (getTopBottomOffsetForScrollingSibling() != 0) { + if (scrollingView.canScrollVertically(-1)) { + final int dy = -appBarLayout.getDownNestedPreScrollRange(); + // Offset by non-zero. + if (dy != 0) { + info.addAction(ACTION_SCROLL_BACKWARD); + info.setScrollable(true); + } + } else { + info.addAction(ACTION_SCROLL_BACKWARD); + info.setScrollable(true); + } + } + } + + @Override + public boolean performAccessibilityAction(View host, int action, Bundle args) { + + if (action == AccessibilityNodeInfoCompat.ACTION_SCROLL_FORWARD) { + appBarLayout.setExpanded(false); + return true; + } else if (action == AccessibilityNodeInfoCompat.ACTION_SCROLL_BACKWARD) { + if (getTopBottomOffsetForScrollingSibling() != 0) { + View scrollingView = getChildWithScrollingBehavior(coordinatorLayout); + if (scrollingView.canScrollVertically(-1)) { + // Expanding action. If the view can scroll down, expand the app bar + // reflecting the logic + // in onNestedPreScroll. + final int dy = -appBarLayout.getDownNestedPreScrollRange(); + // Offset by non-zero. + if (dy != 0) { + onNestedPreScroll( + coordinatorLayout, + appBarLayout, + scrollingView, + 0, + dy, + new int[] {0, 0}, + ViewCompat.TYPE_NON_TOUCH); + return true; + } + } else { + // If the view can't scroll down, we are probably at the top of the + // scrolling content so expand completely. + appBarLayout.setExpanded(true); + return true; + } + } + } else { + return super.performAccessibilityAction(host, action, args); + } + return false; } }); } - - coordinatorLayoutA11yScrollable = - addAccessibilityScrollActions(coordinatorLayout, appBarLayout, scrollingView); } @Nullable @@ -1941,74 +1993,6 @@ private boolean childrenHaveScrollFlags(AppBarLayout appBarLayout) { return false; } - private boolean addAccessibilityScrollActions( - final CoordinatorLayout coordinatorLayout, - @NonNull final T appBarLayout, - @NonNull final View scrollingView) { - boolean a11yScrollable = false; - if (getTopBottomOffsetForScrollingSibling() != -appBarLayout.getTotalScrollRange()) { - // Add a collapsing action if the view offset isn't the abl scroll range. - // (The same offset means the view is completely collapsed). Collapse to minimum height. - addActionToExpand(coordinatorLayout, appBarLayout, ACTION_SCROLL_FORWARD, false); - a11yScrollable = true; - } - // Don't add an expanding action if the sibling offset is 0, which would mean the abl is - // completely expanded. - if (getTopBottomOffsetForScrollingSibling() != 0) { - if (scrollingView.canScrollVertically(-1)) { - // Expanding action. If the view can scroll down, expand the app bar reflecting the logic - // in onNestedPreScroll. - final int dy = -appBarLayout.getDownNestedPreScrollRange(); - // Offset by non-zero. - if (dy != 0) { - ViewCompat.replaceAccessibilityAction( - coordinatorLayout, - ACTION_SCROLL_BACKWARD, - null, - new AccessibilityViewCommand() { - @Override - public boolean perform(@NonNull View view, @Nullable CommandArguments arguments) { - onNestedPreScroll( - coordinatorLayout, - appBarLayout, - scrollingView, - 0, - dy, - new int[] {0, 0}, - ViewCompat.TYPE_NON_TOUCH); - return true; - } - }); - a11yScrollable = true; - } - } else { - // If the view can't scroll down, we are probably at the top of the scrolling content so - // expand completely. - addActionToExpand(coordinatorLayout, appBarLayout, ACTION_SCROLL_BACKWARD, true); - a11yScrollable = true; - } - } - return a11yScrollable; - } - - private void addActionToExpand( - CoordinatorLayout parent, - @NonNull final T appBarLayout, - @NonNull AccessibilityActionCompat action, - final boolean expand) { - ViewCompat.replaceAccessibilityAction( - parent, - action, - null, - new AccessibilityViewCommand() { - @Override - public boolean perform(@NonNull View view, @Nullable CommandArguments arguments) { - appBarLayout.setExpanded(expand); - return true; - } - }); - } - @Override boolean canDragView(T view) { if (onDragCallback != null) { @@ -2112,7 +2096,7 @@ int setHeaderTopBottomOffset( offsetDelta = 0; } - updateAccessibilityActions(coordinatorLayout, appBarLayout); + addAccessibilityDelegateIfNeeded(coordinatorLayout, appBarLayout); return consumed; } @@ -2410,8 +2394,6 @@ public boolean onDependentViewChanged( public void onDependentViewRemoved( @NonNull CoordinatorLayout parent, @NonNull View child, @NonNull View dependency) { if (dependency instanceof AppBarLayout) { - ViewCompat.removeAccessibilityAction(parent, ACTION_SCROLL_FORWARD.getId()); - ViewCompat.removeAccessibilityAction(parent, ACTION_SCROLL_BACKWARD.getId()); ViewCompat.setAccessibilityDelegate(parent, null); } } diff --git a/tests/javatests/com/google/android/material/appbar/AppBarLayoutBaseTest.java b/tests/javatests/com/google/android/material/appbar/AppBarLayoutBaseTest.java index 2d9efc1188d..995314b10f8 100644 --- a/tests/javatests/com/google/android/material/appbar/AppBarLayoutBaseTest.java +++ b/tests/javatests/com/google/android/material/appbar/AppBarLayoutBaseTest.java @@ -25,11 +25,8 @@ import static com.google.android.material.testutils.SwipeUtils.swipeUp; import static com.google.android.material.testutils.TestUtilsActions.setText; import static com.google.android.material.testutils.TestUtilsActions.setTitle; -import static org.hamcrest.CoreMatchers.equalTo; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; import android.graphics.Color; import android.os.Build; @@ -135,29 +132,31 @@ protected boolean matchesSafely(View view) { protected void assertAccessibilityHasScrollForwardAction(boolean hasScrollForward) { if (VERSION.SDK_INT >= 21) { + AccessibilityNodeInfoCompat info = AccessibilityNodeInfoCompat.obtain(); + ViewCompat.onInitializeAccessibilityNodeInfo(mCoordinatorLayout, info); assertThat( - AccessibilityUtils.hasAction( - mCoordinatorLayout, AccessibilityNodeInfoCompat.ACTION_SCROLL_FORWARD), - equalTo(hasScrollForward)); + AccessibilityUtils.hasAction( + info, + AccessibilityNodeInfoCompat.AccessibilityActionCompat.ACTION_SCROLL_FORWARD)) + .isEqualTo(hasScrollForward); } } protected void assertAccessibilityHasScrollBackwardAction(boolean hasScrollBackward) { if (VERSION.SDK_INT >= 21) { + AccessibilityNodeInfoCompat info = AccessibilityNodeInfoCompat.obtain(); + ViewCompat.onInitializeAccessibilityNodeInfo(mCoordinatorLayout, info); assertThat( - AccessibilityUtils.hasAction( - mCoordinatorLayout, AccessibilityNodeInfoCompat.ACTION_SCROLL_BACKWARD), - equalTo(hasScrollBackward)); + AccessibilityUtils.hasAction( + info, + AccessibilityNodeInfoCompat.AccessibilityActionCompat.ACTION_SCROLL_BACKWARD)) + .isEqualTo(hasScrollBackward); } } protected void assertAccessibilityScrollable(boolean isScrollable) { AccessibilityNodeInfoCompat info = AccessibilityNodeInfoCompat.obtain(); ViewCompat.onInitializeAccessibilityNodeInfo(mCoordinatorLayout, info); - if (isScrollable) { - assertTrue(info.isScrollable()); - } else { - assertFalse(info.isScrollable()); - } + assertThat(info.isScrollable()).isEqualTo(isScrollable); } }