Improve checkbox and switch accessibility

ooni · Nov 4, 2024 · 7344f2b · 7344f2b
1 parent 4791bf6
commit 7344f2b
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 61 deletions.
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/ui/dashboard/TestDescriptorLabel.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/ui/dashboard/TestDescriptorLabel.kt
@@ -25,7 +25,7 @@ fun TestDescriptorLabel(
 ) {
     Row(
         verticalAlignment = Alignment.CenterVertically,
-        modifier = modifier.padding(bottom = 2.dp),
+        modifier = modifier,
     ) {
         Icon(
             painter = painterResource(descriptor.icon ?: Res.drawable.ooni_empty_state),

diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/ui/descriptor/DescriptorScreen.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/ui/descriptor/DescriptorScreen.kt
@@ -1,7 +1,6 @@
 package org.ooni.probe.ui.descriptor
 
 import androidx.compose.foundation.background
-import androidx.compose.foundation.clickable
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Row
@@ -13,6 +12,8 @@ import androidx.compose.foundation.layout.navigationBars
 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.triStateToggleable
 import androidx.compose.foundation.verticalScroll
 import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.automirrored.filled.ArrowBack
@@ -33,6 +34,7 @@ import androidx.compose.runtime.Composable
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.SolidColor
+import androidx.compose.ui.semantics.Role
 import androidx.compose.ui.text.font.FontWeight
 import androidx.compose.ui.unit.dp
 import ooniprobe.composeapp.generated.resources.AddDescriptor_AutoRun
@@ -140,17 +142,19 @@ fun DescriptorScreen(
                     verticalAlignment = Alignment.CenterVertically,
                     modifier = Modifier
                         .fillMaxWidth()
-                        .padding(start = 8.dp)
-                        .clickable { onEvent(DescriptorViewModel.Event.AllChecked) },
+                        .triStateToggleable(
+                            state = state.allState,
+                            onClick = { onEvent(DescriptorViewModel.Event.AllChecked) },
+                            role = Role.Checkbox,
+                        )
+                        .padding(horizontal = 8.dp, vertical = 12.dp),
                 ) {
                     TriStateCheckbox(
                         state = state.allState,
-                        onClick = { onEvent(DescriptorViewModel.Event.AllChecked) },
-                    )
-                    Text(
-                        stringResource(Res.string.AddDescriptor_AutoRun),
-                        modifier = Modifier.padding(start = 16.dp),
+                        onClick = null,
+                        modifier = Modifier.padding(start = 16.dp, end = 24.dp),
                     )
+                    Text(stringResource(Res.string.AddDescriptor_AutoRun))
                 }
 
                 when (OrganizationConfig.testDisplayMode) {
@@ -287,26 +291,26 @@ private fun TestItems(
             verticalAlignment = Alignment.CenterVertically,
             modifier = Modifier
                 .fillMaxWidth()
-                .clickable {
-                    onEvent(
-                        DescriptorViewModel.Event.TestChecked(test, !netTestItem.isSelected),
-                    )
-                }
-                .padding(start = 32.dp, top = 4.dp),
+                .padding(start = 32.dp)
+                .toggleable(
+                    value = netTestItem.isSelected,
+                    onValueChange = { onEvent(DescriptorViewModel.Event.TestChecked(test, it)) },
+                    role = Role.Checkbox,
+                )
+                .padding(horizontal = 16.dp)
+                .padding(vertical = 12.dp),
         ) {
             Checkbox(
                 checked = netTestItem.isSelected,
-                onCheckedChange = {
-                    onEvent(DescriptorViewModel.Event.TestChecked(test, it))
-                },
+                onCheckedChange = null,
             )
             Icon(
                 painterResource(
                     test.test.iconRes ?: descriptor.icon ?: Res.drawable.ooni_empty_state,
                 ),
                 contentDescription = null,
                 modifier = Modifier
-                    .padding(start = 16.dp, end = 8.dp)
+                    .padding(start = 24.dp, end = 8.dp)
                     .size(24.dp),
             )
             Text(

diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/ui/descriptor/add/AddDescriptorScreen.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/ui/descriptor/add/AddDescriptorScreen.kt
@@ -1,7 +1,6 @@
 package org.ooni.probe.ui.descriptor.add
 
 import androidx.compose.foundation.background
-import androidx.compose.foundation.clickable
 import androidx.compose.foundation.layout.Arrangement
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
@@ -10,6 +9,8 @@ import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.lazy.LazyColumn
 import androidx.compose.foundation.lazy.items
+import androidx.compose.foundation.selection.toggleable
+import androidx.compose.foundation.selection.triStateToggleable
 import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.filled.Close
 import androidx.compose.material3.CircularProgressIndicator
@@ -23,6 +24,7 @@ import androidx.compose.material3.TriStateCheckbox
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.semantics.Role
 import androidx.compose.ui.state.ToggleableState
 import androidx.compose.ui.text.font.FontWeight
 import androidx.compose.ui.unit.dp
@@ -84,33 +86,45 @@ fun AddDescriptorScreen(
                 )
                 Row(
                     verticalAlignment = Alignment.CenterVertically,
-                    modifier = Modifier.fillMaxWidth().clickable {
-                        onEvent(AddDescriptorViewModel.Event.AutoUpdateChanged(!state.autoUpdate))
-                    },
+                    modifier = Modifier.fillMaxWidth()
+                        .toggleable(
+                            value = state.autoUpdate,
+                            onValueChange = {
+                                onEvent(AddDescriptorViewModel.Event.AutoUpdateChanged(it))
+                            },
+                            role = Role.Switch,
+                        )
+                        .padding(vertical = 8.dp),
                 ) {
                     Text(
                         stringResource(Res.string.AddDescriptor_AutoUpdate),
                         modifier = Modifier.weight(1f),
                     )
                     Switch(
                         checked = state.autoUpdate,
-                        onCheckedChange = {
-                            onEvent(AddDescriptorViewModel.Event.AutoUpdateChanged(it))
-                        },
+                        onCheckedChange = null,
                     )
                 }
                 Row(
                     verticalAlignment = Alignment.CenterVertically,
-                    modifier = Modifier.fillMaxWidth().clickable {
-                        onEvent(AddDescriptorViewModel.Event.AutoRunChanged(state.allTestsSelected() != ToggleableState.On))
-                    },
+                    modifier = Modifier.fillMaxWidth()
+                        .triStateToggleable(
+                            state = state.allTestsSelected(),
+                            onClick = {
+                                onEvent(
+                                    AddDescriptorViewModel.Event.AutoRunChanged(
+                                        state.allTestsSelected() != ToggleableState.On,
+                                    ),
+                                )
+                            },
+                        )
+                        .padding(start = 16.dp)
+                        .padding(vertical = 12.dp),
                 ) {
                     TriStateCheckbox(
                         state = state.allTestsSelected(),
-                        onClick = {
-                            onEvent(AddDescriptorViewModel.Event.AutoRunChanged(state.allTestsSelected() != ToggleableState.On))
-                        },
-                        modifier = Modifier.padding(end = 16.dp),
+                        onClick = null,
+                        modifier = Modifier.padding(end = 24.dp),
                     )
                     Text(
                         stringResource(Res.string.AddDescriptor_AutoRun),
@@ -121,9 +135,7 @@ fun AddDescriptorScreen(
                     items(state.selectableItems) { selectableItem ->
                         TestItem(selectableItem, onChecked = { _ ->
                             onEvent(
-                                AddDescriptorViewModel.Event.SelectableItemClicked(
-                                    selectableItem,
-                                ),
+                                AddDescriptorViewModel.Event.SelectableItemClicked(selectableItem),
                             )
                         })
                     }

diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/ui/run/RunScreen.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/ui/run/RunScreen.kt
@@ -1,7 +1,6 @@
 package org.ooni.probe.ui.run
 
 import androidx.compose.foundation.background
-import androidx.compose.foundation.clickable
 import androidx.compose.foundation.layout.Arrangement
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
@@ -18,6 +17,7 @@ import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.lazy.LazyColumn
 import androidx.compose.foundation.lazy.LazyListScope
 import androidx.compose.foundation.lazy.items
+import androidx.compose.foundation.selection.toggleable
 import androidx.compose.foundation.selection.triStateToggleable
 import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.automirrored.filled.ArrowBack
@@ -131,8 +131,7 @@ fun RunScreen(
                     end = 16.dp,
                     // Insets + Run tests button
                     bottom =
-                        WindowInsets.navigationBars.asPaddingValues().calculateBottomPadding() +
-                            64.dp,
+                        WindowInsets.navigationBars.asPaddingValues().calculateBottomPadding() + 64.dp,
                 ),
                 modifier = Modifier.fillMaxSize()
                     .testTag("Run-DescriptorsList"),
@@ -268,17 +267,17 @@ private fun DescriptorItem(
     Row(
         verticalAlignment = Alignment.CenterVertically,
         modifier = Modifier.fillMaxWidth()
-            .padding(vertical = 4.dp)
             .triStateToggleable(
                 state = descriptorItem.state,
                 onClick = { onChecked(descriptorItem.state != ToggleableState.On) },
                 role = Role.Checkbox,
-            ),
+            )
+            .padding(horizontal = 16.dp),
     ) {
         TriStateCheckbox(
             state = descriptorItem.state,
             onClick = null,
-            modifier = Modifier.padding(start = 16.dp, end = 20.dp),
+            modifier = Modifier.padding(end = 24.dp),
         )
         TestDescriptorLabel(descriptor)
         IconButton(onClick = { onDropdownToggled() }) {
@@ -312,12 +311,17 @@ fun TestItem(
         verticalAlignment = Alignment.CenterVertically,
         modifier = Modifier.fillMaxWidth()
             .padding(start = 32.dp)
-            .clickable { onChecked(!testItem.isSelected) },
+            .toggleable(
+                value = testItem.isSelected,
+                onValueChange = { onChecked(it) },
+                role = Role.Checkbox,
+            )
+            .padding(horizontal = 16.dp, vertical = 10.dp),
     ) {
         Checkbox(
             checked = testItem.isSelected,
-            onCheckedChange = { onChecked(it) },
-            modifier = Modifier.padding(end = 16.dp),
+            onCheckedChange = null,
+            modifier = Modifier.padding(end = 24.dp),
         )
         Text(
             if (test.test is TestType.Experimental) {

diff --git a/...seApp/src/commonMain/kotlin/org/ooni/probe/ui/settings/category/SettingsCategoryScreen.kt b/...seApp/src/commonMain/kotlin/org/ooni/probe/ui/settings/category/SettingsCategoryScreen.kt
@@ -11,6 +11,7 @@ import androidx.compose.foundation.layout.navigationBars
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.width
 import androidx.compose.foundation.rememberScrollState
+import androidx.compose.foundation.selection.toggleable
 import androidx.compose.foundation.text.KeyboardOptions
 import androidx.compose.foundation.verticalScroll
 import androidx.compose.material.icons.Icons
@@ -33,6 +34,7 @@ import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.alpha
 import androidx.compose.ui.platform.testTag
+import androidx.compose.ui.semantics.Role
 import androidx.compose.ui.text.input.KeyboardType
 import androidx.compose.ui.unit.dp
 import androidx.compose.ui.unit.sp
@@ -178,19 +180,19 @@ fun SwitchSettingsView(
         trailingContent = {
             Switch(
                 checked = checked,
-                onCheckedChange = { newValue -> onCheckedChange(key, newValue) },
+                onCheckedChange = null,
                 enabled = enabled,
             )
         },
         modifier = Modifier
             .alpha(if (enabled) 1f else 0.5f)
-            .run {
-                if (enabled) {
-                    clickable { onCheckedChange(key, !checked) }
-                } else {
-                    this
-                }
-            },
+            .toggleable(
+                value = checked,
+                onValueChange = { onCheckedChange(key, it) },
+                enabled = enabled,
+                role = Role.Switch,
+            )
+            .padding(vertical = 2.dp),
     )
 }
 

diff --git a/...oseApp/src/ooniMain/kotlin/org/ooni/probe/ui/descriptor/InstalledDescriptorActionsView.kt b/...oseApp/src/ooniMain/kotlin/org/ooni/probe/ui/descriptor/InstalledDescriptorActionsView.kt
@@ -1,9 +1,9 @@
 package org.ooni.probe.ui.descriptor
 
-import androidx.compose.foundation.clickable
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.padding
+import androidx.compose.foundation.selection.toggleable
 import androidx.compose.material3.AlertDialog
 import androidx.compose.material3.Button
 import androidx.compose.material3.ButtonDefaults
@@ -20,6 +20,7 @@ import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.Color
+import androidx.compose.ui.semantics.Role
 import androidx.compose.ui.unit.dp
 import ooniprobe.composeapp.generated.resources.AddDescriptor_AutoUpdate
 import ooniprobe.composeapp.generated.resources.Dashboard_Runv2_Overview_PreviousRevisions
@@ -69,7 +70,7 @@ fun InstalledDescriptorActionsView(
         )
     }
 
-    Column(modifier = modifier) {
+    Column(modifier = modifier.padding(top = 16.dp)) {
         descriptor.revisions?.let { revisions ->
             if (revisions.isNotEmpty()) {
                 Text(text = stringResource(Res.string.Dashboard_Runv2_Overview_PreviousRevisions))
@@ -108,19 +109,22 @@ fun ConfigureUpdates(
 ) {
     Row(
         verticalAlignment = Alignment.CenterVertically,
-        modifier = Modifier.padding(horizontal = 16.dp).clickable {
-            onEvent(DescriptorViewModel.Event.AutoUpdateChanged(!autoUpdate))
-        },
+        modifier = Modifier
+            .padding(bottom = 8.dp)
+            .toggleable(
+                value = autoUpdate,
+                onValueChange = { onEvent(DescriptorViewModel.Event.AutoUpdateChanged(it)) },
+                role = Role.Switch,
+            )
+            .padding(horizontal = 16.dp, vertical = 8.dp),
     ) {
         Text(
             stringResource(Res.string.AddDescriptor_AutoUpdate),
             modifier = Modifier.weight(1f),
         )
         Switch(
             checked = autoUpdate,
-            onCheckedChange = {
-                onEvent(DescriptorViewModel.Event.AutoUpdateChanged(it))
-            },
+            onCheckedChange = null,
         )
     }
 }