Accessibility: improve behavior of list items (#4626)

* a11y: add Modifier to improve accessibility of ListItems.

Remove duplication of onChange. As per the documentation, it has to be used only if the behavior is different than the onClick listener of the list item.
It also has the effect to read twice the action when the screen reader is one. See https://github.com/element-hq/element-x-android/pull/4047#discussion_r1888136571 for more details

a11y: remove contentDescription on List item icon, else the text is read twice.

* Ensure that if the ListItem is not enabled, the trailing/leading content is also not enabled.

* Update screenshots

* Fix lint crash.

---------

Co-authored-by: ElementBot <android@element.io>

libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/list/CheckboxListItem.kt

@@ -29,7 +29,11 @@ fun CheckboxListItem(
         modifier = modifier,
         headlineContent = { Text(headline) },
         supportingContent = supportingText?.let { @Composable { Text(it) } },
-        leadingContent = ListItemContent.Checkbox(checked, null, enabled, compact = compactLayout),
+        leadingContent = ListItemContent.Checkbox(
+            checked = checked,
+            enabled = enabled,
+            compact = compactLayout,
+        ),
         trailingContent = trailingContent,
         style = style,
         enabled = enabled,

libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/list/ListItemContent.kt

@@ -37,26 +37,22 @@ sealed interface ListItemContent {
     /**
      * Default Switch content for [ListItem].
      * @param checked The current state of the switch.
-     * @param onChange Callback when the switch is toggled: it should only be set to override the default click behaviour in the [ListItem].
      * @param enabled Whether the switch is enabled or not.
      */
     data class Switch(
         val checked: Boolean,
-        val onChange: ((Boolean) -> Unit)? = null,
         val enabled: Boolean = true
     ) : ListItemContent
 
     /**
      * Default Checkbox content for [ListItem].
      * @param checked The current state of the checkbox.
-     * @param onChange Callback when the checkbox is toggled: it should only be set to override the default click behaviour in the [ListItem].
      * @param enabled Whether the checkbox is enabled or not.
      * @param compact Reduces the size of the component to make the wrapping [ListItem] smaller.
      * This is especially useful when the [ListItem] is used inside a Dialog. `false` by default.
      */
     data class Checkbox(
         val checked: Boolean,
-        val onChange: ((Boolean) -> Unit)? = null,
         val enabled: Boolean = true,
         val compact: Boolean = false
     ) : ListItemContent
@@ -64,14 +60,12 @@ sealed interface ListItemContent {
     /**
      * Default RadioButton content for [ListItem].
      * @param selected The current state of the radio button.
-     * @param onClick Callback when the radio button is toggled: it should only be set to override the default click behaviour in the [ListItem].
      * @param enabled Whether the radio button is enabled or not.
      * @param compact Reduces the size of the component to make the wrapping [ListItem] smaller.
      * This is especially useful when the [ListItem] is used inside a Dialog. `false` by default.
      */
     data class RadioButton(
         val selected: Boolean,
-        val onClick: (() -> Unit)? = null,
         val enabled: Boolean = true,
         val compact: Boolean = false
     ) : ListItemContent
@@ -99,24 +93,24 @@ sealed interface ListItemContent {
     data class Counter(val count: Int) : ListItemContent
 
     @Composable
-    fun View() {
+    fun View(isItemEnabled: Boolean) {
         when (this) {
             is Switch -> SwitchComponent(
                 checked = checked,
-                onCheckedChange = onChange,
-                enabled = enabled
+                onCheckedChange = null,
+                enabled = enabled && isItemEnabled,
             )
             is Checkbox -> CheckboxComponent(
                 modifier = if (compact) Modifier.size(maxCompactSize) else Modifier,
                 checked = checked,
-                onCheckedChange = onChange,
-                enabled = enabled
+                onCheckedChange = null,
+                enabled = enabled && isItemEnabled,
             )
             is RadioButton -> RadioButtonComponent(
                 modifier = if (compact) Modifier.size(maxCompactSize) else Modifier,
                 selected = selected,
-                onClick = onClick,
-                enabled = enabled
+                onClick = null,
+                enabled = enabled && isItemEnabled,
             )
             is Icon -> {
                 IconComponent(

libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/list/RadioButtonListItem.kt

@@ -29,7 +29,11 @@ fun RadioButtonListItem(
         modifier = modifier,
         headlineContent = { Text(headline) },
         supportingContent = supportingText?.let { @Composable { Text(it) } },
-        leadingContent = ListItemContent.RadioButton(selected, null, enabled, compact = compactLayout),
+        leadingContent = ListItemContent.RadioButton(
+            selected = selected,
+            enabled = enabled,
+            compact = compactLayout,
+        ),
         trailingContent = trailingContent,
         style = style,
         enabled = enabled,

libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/list/SwitchListItem.kt

@@ -29,7 +29,10 @@ fun SwitchListItem(
         headlineContent = { Text(headline) },
         supportingContent = supportingText?.let { @Composable { Text(it) } },
         leadingContent = leadingContent,
-        trailingContent = ListItemContent.Switch(value, null, enabled),
+        trailingContent = ListItemContent.Switch(
+            checked = value,
+            enabled = enabled,
+        ),
         style = style,
         enabled = enabled,
         onClick = { onChange(!value) },

libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/preferences/PreferenceCheckbox.kt

@@ -65,6 +65,7 @@ fun PreferenceCheckbox(
             checked = isChecked,
             enabled = enabled,
         ),
+        enabled = enabled,
     )
 }
 

libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/ListItem.kt

@@ -9,6 +9,8 @@ package io.element.android.libraries.designsystem.theme.components
 
 import androidx.compose.foundation.clickable
 import androidx.compose.foundation.layout.Column
+import androidx.compose.foundation.selection.selectable
+import androidx.compose.foundation.selection.toggleable
 import androidx.compose.material3.ListItemColors
 import androidx.compose.material3.ListItemDefaults
 import androidx.compose.material3.LocalContentColor
@@ -16,12 +18,9 @@ import androidx.compose.material3.LocalTextStyle
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.CompositionLocalProvider
 import androidx.compose.runtime.Immutable
-import androidx.compose.runtime.getValue
-import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.remember
-import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.Color
+import androidx.compose.ui.semantics.Role
 import androidx.compose.ui.text.style.TextOverflow
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.dp
@@ -135,7 +134,7 @@ fun ListItem(
             CompositionLocalProvider(
                 LocalContentColor provides leadingContentColor,
             ) {
-                content.View()
+                content.View(isItemEnabled = enabled)
             }
         }
     }
@@ -145,7 +144,7 @@ fun ListItem(
                 LocalTextStyle provides ElementTheme.typography.fontBodyMdRegular,
                 LocalContentColor provides trailingContentColor,
             ) {
-                content.View()
+                content.View(isItemEnabled = enabled)
             }
         }
     }
@@ -158,7 +157,12 @@ fun ListItem(
                 .then(modifier)
         } else {
             modifier
-        },
+        }
+            .withAccessibilityModifier(
+                content = trailingContent ?: leadingContent,
+                enabled = enabled || alwaysClickable,
+                onClick = onClick,
+            ),
         overlineContent = null,
         supportingContent = decoratedSupportingContent,
         leadingContent = decoratedLeadingContent,
@@ -169,6 +173,45 @@ fun ListItem(
     )
 }
 
+private fun Modifier.withAccessibilityModifier(
+    content: ListItemContent?,
+    enabled: Boolean,
+    onClick: (() -> Unit)?,
+): Modifier = then(
+    when (content) {
+        is ListItemContent.Checkbox -> {
+            Modifier.toggleable(
+                value = content.checked,
+                role = Role.Checkbox,
+                enabled = content.enabled && enabled,
+                onValueChange = { onClick?.invoke() }
+            )
+        }
+        is ListItemContent.Switch -> {
+            Modifier.toggleable(
+                value = content.checked,
+                role = Role.Switch,
+                enabled = content.enabled && enabled,
+                onValueChange = { onClick?.invoke() }
+            )
+        }
+        is ListItemContent.RadioButton -> {
+            Modifier.selectable(
+                selected = content.selected,
+                role = Role.RadioButton,
+                enabled = content.enabled && enabled,
+                onClick = { onClick?.invoke() }
+            )
+        }
+        ListItemContent.Badge,
+        is ListItemContent.Custom,
+        is ListItemContent.Icon,
+        is ListItemContent.Text,
+        is ListItemContent.Counter,
+        null -> Modifier
+    }
+)
+
 /**
  * The style to use for a [ListItem].
  */
@@ -546,20 +589,17 @@ private object PreviewItems {
 
     @Composable
     fun checkbox(): ListItemContent {
-        var checked by remember { mutableStateOf(false) }
-        return ListItemContent.Checkbox(checked = checked, onChange = { checked = !checked })
+        return ListItemContent.Checkbox(checked = false)
     }
 
     @Composable
     fun radioButton(): ListItemContent {
-        var checked by remember { mutableStateOf(false) }
-        return ListItemContent.RadioButton(selected = checked, onClick = { checked = !checked })
+        return ListItemContent.RadioButton(selected = false)
     }
 
     @Composable
     fun switch(): ListItemContent {
-        var checked by remember { mutableStateOf(false) }
-        return ListItemContent.Switch(checked = checked, onChange = { checked = !checked })
+        return ListItemContent.Switch(checked = false)
     }
 
     @Composable

libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/ListSupportingText.kt

@@ -164,7 +164,7 @@ internal fun ListSupportingTextSmallPaddingPreview() {
 internal fun ListSupportingTextLargePaddingPreview() {
     ElementThemedPreview {
         Column {
-            ListItem(headlineContent = { Text("A title") }, leadingContent = ListItemContent.Switch(checked = true, onChange = {}))
+            ListItem(headlineContent = { Text("A title") }, leadingContent = ListItemContent.Switch(checked = true))
             ListSupportingText(
                 text = "Supporting line text lorem ipsum dolor sit amet, consectetur. Read more",
                 contentPadding = ListSupportingTextDefaults.Padding.LargeLeadingContent,