Merge pull request #213 from Orange-OpenSource/accessibility-fix-cont…

…rols-displayed-in-list-items 173 - accessibility bug - manage list items controls activation by adding a toggleable on the parent
Orange-OpenSource · Jul 6, 2022 · 17ee030 · 17ee030
2 parents d216da6 + 8e3f5ad
commit 17ee030
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 38 deletions.
diff --git a/demo/src/main/java/com/orange/ods/demo/ui/components/chips/VariantChip.kt b/demo/src/main/java/com/orange/ods/demo/ui/components/chips/VariantChip.kt
@@ -18,7 +18,8 @@ import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.size
 import androidx.compose.foundation.rememberScrollState
-import androidx.compose.foundation.selection.toggleable
+import androidx.compose.foundation.selection.selectable
+import androidx.compose.foundation.selection.selectableGroup
 import androidx.compose.foundation.verticalScroll
 import androidx.compose.material.ExperimentalMaterialApi
 import androidx.compose.material.rememberBottomSheetScaffoldState
@@ -127,7 +128,9 @@ private fun VariantChip(customizationState: ChipCustomizationState) {
     val chipLabel = stringResource(id = R.string.component_chip)
 
     if (customizationState.isChoiceChip) {
-        FlowRow(modifier = Modifier.fillMaxWidth()) {
+        FlowRow(modifier = Modifier
+            .fillMaxWidth()
+            .selectableGroup()) {
             for (index in 1..4) {
                 ChoiceChip(index, customizationState)
             }
@@ -166,10 +169,10 @@ private fun ChoiceChip(index: Int, customizationState: ChipCustomizationState) {
     val selected = customizationState.choiceChipIndexSelected.value == index
     val onClick: () -> Unit = { customizationState.selectChoiceChip(index) }
     OdsChip(
-        modifier = Modifier.toggleable(
-            value = selected,
+        modifier = Modifier.selectable(
+            selected = selected,
             role = Role.RadioButton,
-            onValueChange = { onClick() }
+            onClick = onClick
         ),
         text = "${getChipText(chipType = customizationState.chipType.value)} $index",
         onClick = onClick,

diff --git a/demo/src/main/java/com/orange/ods/demo/ui/components/radiobuttons/ComponentRadioButtons.kt b/demo/src/main/java/com/orange/ods/demo/ui/components/radiobuttons/ComponentRadioButtons.kt
@@ -10,10 +10,13 @@
 
 package com.orange.ods.demo.ui.components.radiobuttons
 
+import androidx.compose.foundation.layout.Column
+import androidx.compose.foundation.selection.selectableGroup
 import androidx.compose.material.ExperimentalMaterialApi
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
+import androidx.compose.ui.Modifier
 import com.orange.ods.demo.R
 import com.orange.ods.demo.ui.utilities.composable.RadioButtonListItem
 import com.orange.ods.demo.ui.utilities.composable.Title
@@ -32,25 +35,26 @@ fun ComponentRadioButtonsContent() {
 @Composable
 private fun RadioButtons(enabled: Boolean) {
     val selectedRadio = remember { mutableStateOf(R.string.component_element_item1) }
-
-    RadioButtonListItem(
-        labelRes = R.string.component_element_item1,
-        selectedRadio = selectedRadio,
-        currentRadio = R.string.component_element_item1,
-        enabled = enabled
-    )
-
-    RadioButtonListItem(
-        labelRes = R.string.component_element_item2,
-        selectedRadio = selectedRadio,
-        currentRadio = R.string.component_element_item2,
-        enabled = enabled
-    )
-
-    RadioButtonListItem(
-        labelRes = R.string.component_element_item3,
-        selectedRadio = selectedRadio,
-        currentRadio = R.string.component_element_item3,
-        enabled = enabled
-    )
+    Column(modifier = Modifier.selectableGroup()) {
+        RadioButtonListItem(
+            labelRes = R.string.component_element_item1,
+            selectedRadio = selectedRadio,
+            currentRadio = R.string.component_element_item1,
+            enabled = enabled
+        )
+
+        RadioButtonListItem(
+            labelRes = R.string.component_element_item2,
+            selectedRadio = selectedRadio,
+            currentRadio = R.string.component_element_item2,
+            enabled = enabled
+        )
+
+        RadioButtonListItem(
+            labelRes = R.string.component_element_item3,
+            selectedRadio = selectedRadio,
+            currentRadio = R.string.component_element_item3,
+            enabled = enabled
+        )
+    }
 }
diff --git a/demo/src/main/java/com/orange/ods/demo/ui/utilities/composable/SelectionControlListItems.kt b/demo/src/main/java/com/orange/ods/demo/ui/utilities/composable/SelectionControlListItems.kt
@@ -11,12 +11,14 @@
 package com.orange.ods.demo.ui.utilities.composable
 
 import androidx.annotation.StringRes
-import androidx.compose.foundation.clickable
+import androidx.compose.foundation.selection.selectable
+import androidx.compose.foundation.selection.toggleable
 import androidx.compose.material.ExperimentalMaterialApi
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.MutableState
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.stringResource
+import androidx.compose.ui.semantics.Role
 import com.orange.ods.compose.component.control.OdsCheckbox
 import com.orange.ods.compose.component.control.OdsRadioButton
 import com.orange.ods.compose.component.control.OdsSwitch
@@ -26,12 +28,17 @@ import com.orange.ods.compose.component.list.OdsListItem
 @Composable
 fun CheckboxListItem(@StringRes labelRes: Int, checked: MutableState<Boolean>, enabled: Boolean = true) {
     OdsListItem(
-        modifier = if (enabled) Modifier.clickable { checked.value = !checked.value } else Modifier,
+        modifier = Modifier.toggleable(
+            value = checked.value,
+            role = Role.Checkbox,
+            enabled = enabled,
+            onValueChange = { checked.value = !checked.value }
+        ),
         text = stringResource(id = labelRes),
         trailing = {
             OdsCheckbox(
                 checked = checked.value,
-                onCheckedChange = { checked.value = it },
+                onCheckedChange = null,
                 enabled = enabled
             )
         }
@@ -41,17 +48,22 @@ fun CheckboxListItem(@StringRes labelRes: Int, checked: MutableState<Boolean>, e
 @ExperimentalMaterialApi
 @Composable
 fun <T> RadioButtonListItem(@StringRes labelRes: Int, selectedRadio: MutableState<T>, currentRadio: T, onClick: () -> Unit = {}, enabled: Boolean = true) {
-    val clickAction = {
-        selectedRadio.value = currentRadio
-        onClick.invoke()
-    }
+    val selected = selectedRadio.value == currentRadio
     OdsListItem(
-        modifier = if (enabled) Modifier.clickable { clickAction() } else Modifier,
+        modifier = Modifier.selectable(
+            selected = selected,
+            role = Role.RadioButton,
+            enabled = enabled,
+            onClick = {
+                selectedRadio.value = currentRadio
+                onClick()
+            }
+        ),
         text = stringResource(id = labelRes),
         trailing = {
             OdsRadioButton(
-                selected = selectedRadio.value == currentRadio,
-                onClick = { clickAction() },
+                selected = selected,
+                onClick = null,
                 enabled = enabled
             )
         }
@@ -62,12 +74,19 @@ fun <T> RadioButtonListItem(@StringRes labelRes: Int, selectedRadio: MutableStat
 @Composable
 fun SwitchListItem(@StringRes labelRes: Int, checked: MutableState<Boolean>, enabled: Boolean = true) {
     OdsListItem(
-        modifier = if (enabled) Modifier.clickable { checked.value = !checked.value } else Modifier,
+        modifier = Modifier.toggleable(
+            value = checked.value,
+            role = Role.Switch,
+            enabled = enabled,
+            onValueChange = {
+                checked.value = !checked.value
+            }
+        ),
         text = stringResource(id = labelRes),
         trailing = {
             OdsSwitch(
                 checked = checked.value,
-                onCheckedChange = { checked.value = it },
+                onCheckedChange = null,
                 enabled = enabled
             )
         }