Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix onClick A11y #2040

Merged
merged 5 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -133,7 +135,7 @@ internal fun Button(
enabled: Boolean = true,
) {
val interactionSource = remember { MutableInteractionSource() }
Button(
MaterialButton(
onClick = onClick,
modifier = modifier
.size(buttonSize.tapTargetSize)
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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")
}
}

Expand All @@ -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")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
yschimke marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
yschimke marked this conversation as resolved.
Show resolved Hide resolved
// not possible to define action for OnDoubleClick
.assertTextEquals("Primary label, Secondary label")
}
}

Expand Down
Loading