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

Commit 3ec7ebbf authored by Yuri Lin's avatar Yuri Lin
Browse files

Handle all BundleHeader interactions via ExpandableNotificationRow

Having a click handler on BundleHeader but actual long-click handling on the ExpandableNotificationRow led to both of these nodes being focusable accessibility nodes, which in turn leads to confusing behavior where it's not clear which node is selected.

This change also removes all semantics from the BundleHeader so that it does not receive any focus, and in exchange explicitly sets the content description (to a string combining the bundle title and the number of notifications) when initializing ExpandableNotificationRow's accessibility node info.

Fixes: 425576022
Test: performed click, long-click, and each action (expand/collapse/dismiss) on the bundle header using both TalkBack and Voice Access
Flag: com.android.systemui.notification_bundle_ui
Change-Id: I0a5ba13bcb0dd2f24d01e76a144df73a44c20d6d
parent 8fb1bc7f
Loading
Loading
Loading
Loading
+6 −42
Original line number Diff line number Diff line
@@ -21,7 +21,6 @@ import androidx.compose.animation.core.FastOutSlowInEasing
import androidx.compose.animation.core.LinearEasing
import androidx.compose.animation.core.tween
import androidx.compose.foundation.Image
import androidx.compose.foundation.combinedClickable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxSize
@@ -44,11 +43,8 @@ import androidx.compose.ui.layout.ContentScale
import androidx.compose.ui.layout.Layout
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.collapse
import androidx.compose.ui.semantics.clearAndSetSemantics
import androidx.compose.ui.semantics.contentDescription
import androidx.compose.ui.semantics.dismiss
import androidx.compose.ui.semantics.expand
import androidx.compose.ui.semantics.onLongClick
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.unit.Dp
@@ -87,12 +83,7 @@ object BundleHeader {

@OptIn(ExperimentalMaterial3ExpressiveApi::class)
@Composable
fun BundleHeader(
    viewModel: BundleHeaderViewModel,
    modifier: Modifier = Modifier,
    onHeaderClicked: () -> Unit = {},
    onA11yDismissAction: () -> Unit = {}, // only for dismissing via accessibility action
) {
fun BundleHeader(viewModel: BundleHeaderViewModel, modifier: Modifier = Modifier) {
    val state =
        rememberMutableSceneTransitionLayoutState(
            initialScene = BundleHeader.Scenes.Collapsed,
@@ -142,39 +133,12 @@ fun BundleHeader(

    Box(modifier) {
        Background(background = viewModel.backgroundDrawable, modifier = Modifier.matchParentSize())
        fun toggle() {
            viewModel.onHeaderClicked()
            onHeaderClicked()
        }
        SceneTransitionLayout(
            state = state,
            modifier =
                Modifier.combinedClickable(
                        onClick = { toggle() },
                        interactionSource = null,
                        indication = null,
                    )
                    .semantics {
                        when (state.currentScene) {
                            BundleHeader.Scenes.Collapsed ->
                                expand {
                                    toggle()
                                    true
                                }
                            BundleHeader.Scenes.Expanded ->
                                collapse {
                                    toggle()
                                    true
                                }
                        }
                        dismiss {
                            onA11yDismissAction()
                            true
                        }
                        // Do nothing. This is here to indicate that the BundleHeader is long
                        // clickable; the actual long click is handled by ExpandableNotificationRow.
                        onLongClick(action = null)
                    },
            // The BundleHeader is clickable, but clicks are handled at the level of the
            // ExpandableNotificationRow. We clear all semantics here so that accessibility focus
            // remains on the same element as handles the clicks and actions.
            modifier = Modifier.clearAndSetSemantics {},
        ) {
            scene(BundleHeader.Scenes.Collapsed) {
                BundleHeaderContent(viewModel, collapsed = true)
+3 −0
Original line number Diff line number Diff line
@@ -2204,6 +2204,9 @@
        other {# notifications}
    }</string>

    <!-- Template for making a content description of a notification bundle header, combining the bundle name (e.g. "News") with the number of notifications in it. -->
    <string name="notification_bundle_header_joined_description"><xliff:g id="name" example="Promotions">%1$s</xliff:g>, <xliff:g id="num_notifications" example="2 notifications">%2$s</xliff:g></string>

    <!-- Notification Inline controls: button to dismiss the blocking helper [CHAR_LIMIT=20] -->
    <string name="inline_done_button">Done</string>

+12 −9
Original line number Diff line number Diff line
@@ -49,12 +49,12 @@ import com.android.systemui.statusbar.notification.row.ui.viewmodel.BundleHeader
import com.android.systemui.statusbar.notification.stack.NotificationListContainer
import com.android.systemui.util.time.SystemClock
import dagger.Lazy
import javax.inject.Inject
import javax.inject.Provider
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import javax.inject.Inject
import javax.inject.Provider

/** Class that handles inflating BundleEntry view and controller, for use by NodeSpecBuilder. */
@SysUISingleton
@@ -125,6 +125,7 @@ constructor(
        row.addOnAttachStateChangeListener(
            object : View.OnAttachStateChangeListener {
                override fun onViewAttachedToWindow(v: View) {}

                override fun onViewDetachedFromWindow(v: View) {
                    scope.cancel()
                    row.removeOnAttachStateChangeListener(this)
@@ -204,14 +205,16 @@ private fun HeaderComposeViewContent(
            )
        DisposableEffect(viewModel) {
            row.setBundleHeaderViewModel(viewModel)
            onDispose { row.setBundleHeaderViewModel(null) }
            row.setOnClickListener {
                viewModel.onHeaderClicked()
                row.expandNotification()
            }
        BundleHeader(
            viewModel,
            onHeaderClicked = { row.expandNotification() },
            // to be used only for dismissal coming from an accessibility action.
            onA11yDismissAction = { row.performDismiss(true) },
        )
            onDispose {
                row.setOnClickListener(null)
                row.setBundleHeaderViewModel(null)
            }
        }
        BundleHeader(viewModel)
    }
}

+7 −0
Original line number Diff line number Diff line
@@ -4379,6 +4379,13 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
                info.addAction(action);
            }
        }

        if (isBundle()) {
            // Set a content description explicitly for the bundle header because we can't
            // otherwise merge accessibility information across the ViewGroup/ComposeView boundary.
            NotificationChildrenContainer childrenContainer = getChildrenContainerNonNull();
            info.setContentDescription(childrenContainer.getBundleHeaderDescription());
        }
    }

    /** @return whether this row's expansion state can be toggled by an accessibility action. */
+8 −0
Original line number Diff line number Diff line
@@ -78,6 +78,14 @@ constructor(
                numberOfChildren ?: 0,
            )

    val headerContentDescription: String
        get() =
            context.resources.getString(
                R.string.notification_bundle_header_joined_description,
                context.resources.getString(titleText),
                numberOfChildrenContentDescription,
            )

    private var sceneTargetJob: Job? = null

    init {
Loading