Fix onClick A11y (#2040) (#2046)

(cherry picked from commit 8851577)
google · Feb 20, 2024 · acd7e36 · acd7e36
1 parent eea8e87
commit acd7e36
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 23 deletions.
diff --git a/compose-material/src/main/java/com/google/android/horologist/compose/material/Button.kt b/compose-material/src/main/java/com/google/android/horologist/compose/material/Button.kt
@@ -36,9 +36,10 @@ import androidx.compose.ui.semantics.Role
 import androidx.compose.ui.semantics.clearAndSetSemantics
 import androidx.compose.ui.semantics.contentDescription
 import androidx.compose.ui.semantics.disabled
+import androidx.compose.ui.semantics.onClick
+import androidx.compose.ui.semantics.onLongClick
 import androidx.compose.ui.semantics.role
 import androidx.compose.ui.unit.Dp
-import androidx.wear.compose.material.Button
 import androidx.wear.compose.material.ButtonColors
 import androidx.wear.compose.material.ButtonDefaults
 import androidx.wear.compose.material.ButtonDefaults.DefaultButtonSize
@@ -51,6 +52,7 @@ import com.google.android.horologist.annotations.ExperimentalHorologistApi
 import com.google.android.horologist.images.base.paintable.DrawableResPaintable
 import com.google.android.horologist.images.base.paintable.ImageVectorPaintable
 import com.google.android.horologist.images.base.paintable.PaintableIcon
+import androidx.wear.compose.material.Button as MaterialButton
 
 /**
  * This component is an alternative to [Button], providing the following:
@@ -133,7 +135,7 @@ internal fun Button(
     enabled: Boolean = true,
 ) {
     val interactionSource = remember { MutableInteractionSource() }
-    Button(
+    MaterialButton(
         onClick = onClick,
         modifier = modifier
             .size(buttonSize.tapTargetSize)
@@ -143,6 +145,16 @@ internal fun Button(
                 if (!enabled) {
                     disabled()
                 }
+                this.onClick(action = {
+                    onClick()
+                    true
+                })
+                if (onLongClick != null) {
+                    this.onLongClick(action = {
+                        onLongClick()
+                        true
+                    })
+                }
             },
         enabled = enabled,
         colors = colors,

diff --git a/compose-material/src/main/java/com/google/android/horologist/compose/material/Card.kt b/compose-material/src/main/java/com/google/android/horologist/compose/material/Card.kt
@@ -36,6 +36,7 @@ import androidx.compose.ui.unit.dp
 import androidx.wear.compose.material.CardDefaults
 import androidx.wear.compose.material.MaterialTheme
 import com.google.android.horologist.annotations.ExperimentalHorologistApi
+import androidx.wear.compose.material.Card as MaterialCard
 
 /**
  * This component is an alternative to [Card], adding support for long and double-clicks.
@@ -56,7 +57,7 @@ public fun Card(
     content: @Composable () -> Unit,
 ) {
     val interactionSource = remember { MutableInteractionSource() }
-    androidx.wear.compose.material.Card(
+    MaterialCard(
         onClick = onClick,
         modifier = modifier,
         backgroundPainter = backgroundPainter,

diff --git a/compose-material/src/main/java/com/google/android/horologist/compose/material/Chip.kt b/compose-material/src/main/java/com/google/android/horologist/compose/material/Chip.kt
@@ -50,6 +50,8 @@ import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.semantics.Role
 import androidx.compose.ui.semantics.clearAndSetSemantics
 import androidx.compose.ui.semantics.disabled
+import androidx.compose.ui.semantics.onClick
+import androidx.compose.ui.semantics.onLongClick
 import androidx.compose.ui.semantics.role
 import androidx.compose.ui.semantics.text
 import androidx.compose.ui.text.buildAnnotatedString
@@ -248,6 +250,16 @@ public fun Chip(
                 if (!enabled) {
                     disabled()
                 }
+                this.onClick(action = {
+                    onClick()
+                    true
+                })
+                if (onLongClick != null) {
+                    this.onLongClick(action = {
+                        onLongClick()
+                        true
+                    })
+                }
             },
         secondaryLabel = secondaryLabelParam,
         icon = icon,

diff --git a/...e-material/src/test/java/com/google/android/horologist/compose/material/ButtonA11yTest.kt b/...e-material/src/test/java/com/google/android/horologist/compose/material/ButtonA11yTest.kt
@@ -22,11 +22,14 @@ import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.filled.Check
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
-import androidx.compose.ui.test.onRoot
-import androidx.compose.ui.test.printToString
+import androidx.compose.ui.semantics.SemanticsActions
+import androidx.compose.ui.semantics.SemanticsProperties
+import androidx.compose.ui.test.SemanticsMatcher.Companion.keyIsDefined
+import androidx.compose.ui.test.assert
+import androidx.compose.ui.test.assertContentDescriptionEquals
+import androidx.compose.ui.test.assertHasClickAction
 import com.google.android.horologist.screenshots.ScreenshotBaseTest
 import com.google.android.horologist.screenshots.ScreenshotTestRule.Companion.screenshotTestRuleParams
-import org.junit.Assert
 import org.junit.Test
 
 class ButtonA11yTest : ScreenshotBaseTest(
@@ -44,17 +47,16 @@ class ButtonA11yTest : ScreenshotBaseTest(
                     imageVector = Icons.Default.Check,
                     contentDescription = "contentDescription",
                     onClick = { },
+                    onLongClick = {},
                 )
             }
         }
 
         screenshotTestRule.interact {
-            val logEntries = onRoot().printToString()
-                .split("\n")
-                .map { it.trim() }
-
-            Assert.assertEquals(1, logEntries.filter { it.startsWith("Role") }.size)
-            Assert.assertEquals(1, logEntries.filter { it.startsWith("ContentDescription") }.size)
+            onNode(keyIsDefined(SemanticsProperties.Role))
+                .assertHasClickAction()
+                .assert(keyIsDefined(SemanticsActions.OnLongClick))
+                .assertContentDescriptionEquals("contentDescription")
         }
     }
 
@@ -72,12 +74,9 @@ class ButtonA11yTest : ScreenshotBaseTest(
         }
 
         screenshotTestRule.interact {
-            val logEntries = onRoot().printToString()
-                .split("\n")
-                .map { it.trim() }
-
-            Assert.assertEquals(1, logEntries.filter { it.startsWith("Role") }.size)
-            Assert.assertEquals(1, logEntries.filter { it.startsWith("ContentDescription") }.size)
+            onNode(keyIsDefined(SemanticsProperties.Role))
+                .assertHasClickAction()
+                .assertContentDescriptionEquals("contentDescription")
         }
     }
 }
diff --git a/...ose-material/src/test/java/com/google/android/horologist/compose/material/CardA11yTest.kt b/...ose-material/src/test/java/com/google/android/horologist/compose/material/CardA11yTest.kt
@@ -23,8 +23,9 @@ import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.semantics.SemanticsProperties
-import androidx.compose.ui.test.SemanticsMatcher
-import androidx.compose.ui.test.assertCountEquals
+import androidx.compose.ui.test.SemanticsMatcher.Companion.keyIsDefined
+import androidx.compose.ui.test.assertHasClickAction
+import androidx.compose.ui.test.assertTextEquals
 import androidx.wear.compose.material.Text
 import com.google.android.horologist.screenshots.ScreenshotBaseTest
 import com.google.android.horologist.screenshots.ScreenshotTestRule.Companion.screenshotTestRuleParams
@@ -60,7 +61,11 @@ class CardA11yTest : ScreenshotBaseTest(
         }
 
         screenshotTestRule.interact {
-            onAllNodes(SemanticsMatcher.keyIsDefined(SemanticsProperties.Role)).assertCountEquals(1)
+            onNode(keyIsDefined(SemanticsProperties.Role))
+                .assertTextEquals("Click me!")
+                .assertHasClickAction()
+            // Not set by combinedClickable
+//                .assert(keyIsDefined(SemanticsActions.OnLongClick))
         }
     }
 

diff --git a/...ose-material/src/test/java/com/google/android/horologist/compose/material/ChipA11yTest.kt b/...ose-material/src/test/java/com/google/android/horologist/compose/material/ChipA11yTest.kt
@@ -22,9 +22,12 @@ import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.filled.Image
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.semantics.SemanticsActions
 import androidx.compose.ui.semantics.SemanticsProperties
 import androidx.compose.ui.test.SemanticsMatcher.Companion.keyIsDefined
-import androidx.compose.ui.test.assertCountEquals
+import androidx.compose.ui.test.assert
+import androidx.compose.ui.test.assertHasClickAction
+import androidx.compose.ui.test.assertTextEquals
 import com.google.android.horologist.images.base.paintable.ImageVectorPaintable.Companion.asPaintable
 import com.google.android.horologist.screenshots.ScreenshotBaseTest
 import com.google.android.horologist.screenshots.ScreenshotTestRule
@@ -44,14 +47,20 @@ class ChipA11yTest : ScreenshotBaseTest(
                 Chip(
                     label = "Primary label",
                     onClick = { },
+                    onLongClick = {},
+                    onDoubleClick = {},
                     secondaryLabel = "Secondary label",
                     icon = Icons.Default.Image.asPaintable(),
                 )
             }
         }
 
         screenshotTestRule.interact {
-            onAllNodes(keyIsDefined(SemanticsProperties.Role)).assertCountEquals(1)
+            onNode(keyIsDefined(SemanticsProperties.Role))
+                .assertHasClickAction()
+                .assert(keyIsDefined(SemanticsActions.OnLongClick))
+                // not possible to define action for OnDoubleClick
+                .assertTextEquals("Primary label, Secondary label")
         }
     }