Fixed
Status Update
No update yet.
Comments
so...@google.com
so...@google.com
Hi Yinglei,
I remember I ran into the same issue in our demos. Looking at the implementation of the Modifier.clickable, I can see that we provide the corresponding semantics property and set info.enabled in the a11y delegate.
Do you have an idea why it's not picked up by Talkback then?
yi...@google.com
In compose, when the button (and other widgets) is disabled, the actions are not added to semantics:
val semanticModifier = Modifier.semantics(mergeDescendants = true) {
if (enabled) {
//
onClick(action = { onClick(); true }, label = onClickLabel)
if (onLongClick != null) {
onLongClick(action = { onLongClick(); true }, label = onLongClickLabel)
}
} else {
disabled()
}
}
However, we didn't add clickable semantics property to compose. In android, we have a clickable property. when disabled, we don't add the click accessibility action (we still have the onclick listener though for non accessibility), but we still set the clickable to true.
in talkback, we only notify disabled when the node is actionable. In android, since we have clickable set the node is actionable. but for compose, clickable is not set, so talkback doesn't announce it is disabled because the node is not considered actionable.
I see three possible solutions:
1. add clickable semantics property in compose.
2. in compose, even if disabled, we still add the click semantics action, and we set clickable to true when convert to accessibility node info, but don't add the click action in accessibility node info. when we perform the action, we check whether it is disabled in compose.
3. I think talkback should be fixed: the current compose pattern seems very reasonable for developers. What we really want is to announce disabled for roles known as controls, because user might expect to click them. so this is a perfect place to use the control_filter in talkback which is based on roles. If a node passes the control filter, we always announce disabled status if disabled.
I personably prefer 3 or 2.
Phil, I remember the enabled/clickable/click action change once caused regression. Could you let me know what regression it caused? If there are other complications, we might need to use approach 1. Thanks!
val semanticModifier = Modifier.semantics(mergeDescendants = true) {
if (enabled) {
//
onClick(action = { onClick(); true }, label = onClickLabel)
if (onLongClick != null) {
onLongClick(action = { onLongClick(); true }, label = onLongClickLabel)
}
} else {
disabled()
}
}
However, we didn't add clickable semantics property to compose. In android, we have a clickable property. when disabled, we don't add the click accessibility action (we still have the onclick listener though for non accessibility), but we still set the clickable to true.
in talkback, we only notify disabled when the node is actionable. In android, since we have clickable set the node is actionable. but for compose, clickable is not set, so talkback doesn't announce it is disabled because the node is not considered actionable.
I see three possible solutions:
1. add clickable semantics property in compose.
2. in compose, even if disabled, we still add the click semantics action, and we set clickable to true when convert to accessibility node info, but don't add the click action in accessibility node info. when we perform the action, we check whether it is disabled in compose.
3. I think talkback should be fixed: the current compose pattern seems very reasonable for developers. What we really want is to announce disabled for roles known as controls, because user might expect to click them. so this is a perfect place to use the control_filter in talkback which is based on roles. If a node passes the control filter, we always announce disabled status if disabled.
I personably prefer 3 or 2.
Phil, I remember the enabled/clickable/click action change once caused regression. Could you let me know what regression it caused? If there are other complications, we might need to use approach 1. Thanks!
pw...@google.com
Yep. We had exactly this issue when we tried to eliminate the situation where a node is clickable but doesn't support action_click. It turned out that when a button is disabled, it gets into that state and wasn't announced properly.
(2) would match what's in View I think. (3) does make sense, but as you say would require a bit more consideration.
I would probably go with (2) since that seems quick, and consider (3) as a longer-term, more correct fix that would also permit restoring the idea of making clickable just a shorthand for the action list containing a click.
(2) would match what's in View I think. (3) does make sense, but as you say would require a bit more consideration.
I would probably go with (2) since that seems quick, and consider (3) as a longer-term, more correct fix that would also permit restoring the idea of making clickable just a shorthand for the action list containing a click.
cb...@google.com
> we didn't add clickable semantics property to compose. In android, we have a clickable property.
Consistency between compose and android-UI-toolkit seems essential, since talkback cannot handle many variations.
> talkback ... announce disabled for roles known as controls
This seems like a reasonable default. But I suspect there might be unknown roles that are intended to be clickable, which would require a clickable-attribute.
Consistency between compose and android-UI-toolkit seems essential, since talkback cannot handle many variations.
> talkback ... announce disabled for roles known as controls
This seems like a reasonable default. But I suspect there might be unknown roles that are intended to be clickable, which would require a clickable-attribute.
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit 1e6d8aa505d2efde5f61ff3c45325ab533840d32
Author: yingleiw <yingleiw@google.com>
Date: Fri Dec 04 14:49:25 2020
provide SemanticsActions when a SemanticsNode is marked as disabled
In android, we have a clickable property. when disabled, we don't
add the click accessibility action (we still have the onclick
listener though for non accessibility), but we still set the
clickable to true.
In talkback, we only notify disabled when the node is actionable.
In android, since we have clickable set the node is actionable.
but for compose, clickable is not set, so talkback doesn't announce
it is disabled because the node is not considered actionable.
This cl change it to always add semantics actions, even when the node is
disabled and convert to corresponding android accessibility properties.
For more details on different solutions considered, please see the bug.
Relnote: Add Toggle to foundation Strings.kt
Fix:
Test: tested with demo app and talkback. Added a disabled toggle button
in the demo app to make sure disabled nodes can't be toggled. Added an
integration test to make sure disabled nodes can't be toggled.
Unit tests added.
Change-Id: I4a5b74e3ed9bc3b1acd09af221331ef6ab51b9fe
M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ClickableTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ToggleableTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Clickable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Strings.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/selection/Toggleable.kt
M compose/material/material/integration-tests/material-demos/src/main/java/androidx/compose/material/demos/ButtonDemo.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/CheckboxUiTest.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/SwitchTest.kt
M compose/ui/ui-test/src/androidAndroidTest/kotlin/androidx/compose/ui/test/ErrorMessagesTest.kt
M compose/ui/ui/api/current.txt
M compose/ui/ui/api/public_plus_experimental_current.txt
M compose/ui/ui/api/restricted_current.txt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/AndroidAccessibilityTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/AndroidComposeViewAccessibilityDelegateCompatTest.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeViewAccessibilityDelegateCompat.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/semantics/SemanticsProperties.kt
https://android-review.googlesource.com/1518413
Branch: androidx-master-dev
commit 1e6d8aa505d2efde5f61ff3c45325ab533840d32
Author: yingleiw <yingleiw@google.com>
Date: Fri Dec 04 14:49:25 2020
provide SemanticsActions when a SemanticsNode is marked as disabled
In android, we have a clickable property. when disabled, we don't
add the click accessibility action (we still have the onclick
listener though for non accessibility), but we still set the
clickable to true.
In talkback, we only notify disabled when the node is actionable.
In android, since we have clickable set the node is actionable.
but for compose, clickable is not set, so talkback doesn't announce
it is disabled because the node is not considered actionable.
This cl change it to always add semantics actions, even when the node is
disabled and convert to corresponding android accessibility properties.
For more details on different solutions considered, please see the bug.
Relnote: Add Toggle to foundation Strings.kt
Fix:
Test: tested with demo app and talkback. Added a disabled toggle button
in the demo app to make sure disabled nodes can't be toggled. Added an
integration test to make sure disabled nodes can't be toggled.
Unit tests added.
Change-Id: I4a5b74e3ed9bc3b1acd09af221331ef6ab51b9fe
M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ClickableTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ToggleableTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Clickable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Strings.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/selection/Toggleable.kt
M compose/material/material/integration-tests/material-demos/src/main/java/androidx/compose/material/demos/ButtonDemo.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/CheckboxUiTest.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/SwitchTest.kt
M compose/ui/ui-test/src/androidAndroidTest/kotlin/androidx/compose/ui/test/ErrorMessagesTest.kt
M compose/ui/ui/api/current.txt
M compose/ui/ui/api/public_plus_experimental_current.txt
M compose/ui/ui/api/restricted_current.txt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/AndroidAccessibilityTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/AndroidComposeViewAccessibilityDelegateCompatTest.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeViewAccessibilityDelegateCompat.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/semantics/SemanticsProperties.kt
Description
Jetpack Compose release version: 1.0.0-alpha06
TalkBack does not currently read out "disabled" for buttons that are disabled.
For example, I'd expect this button to read out as "Save, Button, disabled", but it currently is read as simple "Save" (I know the button class name issue is being worked on separately):
As an example from the View world, this does read as "Save, button, disabled":
This appears to be because TalkBack only reads "disabled" on nodes that
Buttononly adds anonClicksemantic if it is enabled, it is never both "actionable" and disabled right now.