Issues with end padding in ListItem from material3 [305342674]

Fixed

Bug

Status Update

No update yet.

Description

sp...@gmail.com

created issue #1

Oct 14, 2023 03:18PM

Jetpack Compose version: 1.6.0-alpha07
Jetpack Compose material3 version: 1.2.0-alpha09
Jetpack Compose component used: ListItem
Android Studio Build: Android Studio Iguana | 2023.2.1 Canary 7
Kotlin version: 1.9.10

I was trying to use the ListItem composable from material3 and noticed a couple of issues with trailingContent leaving more space than expected at the end.

First issue

After checking the specs, I believe that ListItemEndPadding is set to the wrong value. It should be 16.dp not 24.dp.

Second issue

If, for example, I try to recreate the "One-line lists" and use a CheckBox in the trailing content lambda, ListItem will add a 16.dp** padding to the end. Unfortunately, a clickable CheckBox uses minimumInteractiveComponentSize(), and the reserved space will increase from 24.dp (CheckBox size) to 48.dp, adding 12.dp to the already existing 16.dp from ListItem.

Right now I think it's not possible to recreate the examples from the specs that have a "Secondary Action area" like a CheckBox, IconButton, or anything small with a minimum touch target size added. Text "100+" examples are different since they don't need to be clickable.

** Assuming First issue is fixed at some point and the end padding applied is 16.dp and not 24.dp.

Steps to reproduce

Run a composable with this content:

// Result with markers in the "output_with_8dp_markers.png" file attached
Column {
    ListItem(
        headlineContent = { Text(text = "Text") },
        trailingContent = { Text(text = "100+") },
    )
    HorizontalDivider()
    ListItem(
        headlineContent = { Text(text = "Checkbox") },
        trailingContent = { Checkbox(checked = true, onCheckedChange = { }) },
    )
    HorizontalDivider()
    ListItem(
        headlineContent = { Text(text = "Checkbox (null onChange)") },
        trailingContent = { Checkbox(checked = true, onCheckedChange = null) },
    )
    HorizontalDivider()
    ListItem(
        headlineContent = { Text(text = "Icon") },
        trailingContent = {
            IconButton(onClick = {}) {
                Icon(Icons.AutoMirrored.Default.Send, contentDescription = "Send this")
            }
        },
    )
}

Instead of 16.dp, we get:

The "Text" example leaves 24.dp between the "100+" and the end.
The "Checkbox" example leaves 36.dp.
The "Checkbox [null onChange]" example (if we don't want to use the Checkbox as a "Secondary Action") leaves 24.dp.
The "Icon" example leaves 36.dp.

Apologies if this is working as expected. All the ListItem examples use Text as the trailingContent and I think an example with CheckBox/IconButton could be very helpful, because I don't see what the correct approach is when using other types of content. Thank you.

output_with_8dp_markers.png

20 KB

View

Download

Comments

sg...@google.com <sg...@google.com> Oct 16, 2023 09:25PM

Reassigned to ma...@google.com.

al...@gmail.com <al...@gmail.com> #2Nov 30, 2023 11:56PM

Project: platform/frameworks/support
Branch: androidx-main
Author: Louis Pullen-Freilich <lpf@google.com>
Link: https://android-review.googlesource.com/3328184

Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling

Expand for full commit details

Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling 
 
These APIs allow overscroll to have events dispatched to it by one component, and rendered in a separate component. 
 
Fixes: b/266550551 
Fixes: b/204650733 
Fixes: b/255554340 
Fixes: b/229537244 
Test: OverscrollTest 
Relnote: "Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling APIs - these APIs create a wrapped instance of the provided overscroll effect that doesn't draw / handle events respectively, which allows for rendering  overscroll in a separate component from the component that is dispatching events. For example, disabling drawing the overscroll inside a lazy list, and then drawing the overscroll separately on top / elsewhere." 
Change-Id: Idbb3d91546b49c1987a041f959bce4b2b09a9f61

Files:

M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/OverscrollDemo.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/OverscrollSample.kt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/OverscrollTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Overscroll.kt

Hash: f64e25b7a473c757d080521e7dd97b3f6670f60d
Date: Fri Nov 01 18:43:56 2024

ap...@google.com <ap...@google.com> #3Jan 2, 2024 05:58PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.compose.foundation:foundation:1.8.0-alpha06
androidx.compose.foundation:foundation-android:1.8.0-alpha06
androidx.compose.foundation:foundation-jvmstubs:1.8.0-alpha06
androidx.compose.foundation:foundation-linuxx64stubs:1.8.0-alpha06

ma...@google.com <ma...@google.com> #4Jan 2, 2024 06:00PM

Marked as fixed.

Thanks for pointing out the spec discrepancy! This has been fixed.

As for the minimumInteractiveComponentSize() issue, a workaround is to use Modifier.offset on the icon, but we will explore options to make this common use case more intuitive.

pr...@google.com <pr...@google.com> #5Jan 16, 2024 06:24PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.compose.material3:material3:1.2.0-beta02
androidx.compose.material3:material3-android:1.2.0-beta02
androidx.compose.material3:material3-desktop:1.2.0-beta02

od...@gmail.com <od...@gmail.com> #6Feb 5, 2024 04:11PM

Can we ask why this was closed? We are finding that the trailing padding has issues even after the applied fix and the workaround posted here. Note the following reproducible code and screenshots.

@Preview(widthDp = 403)
@Composable
private fun ListItemTrailingPaddingIssue() {
    ListItem(
        overlineContent = {
            Text("Overline content")
        },
        headlineContent = {
            Text("Headline content reaches to trailing content")
        },
        supportingContent = {
            Text("Supporting content")
        },
        trailingContent = {
            IconButton(
                // We've applied the modifier "workaround". However, note the large padding left between the leading
                // and trailing content because the offset doesn't change or move the actual padding of
                // the trailing content
                // https://issuetracker.google.com/issues/305342674#comment4
                modifier = Modifier.offset(
                    y = (-12).dp,
                    x = 12.dp,
                ),
                onClick = {},
            ) {
                Icon(
                    imageVector = Icons.Default.Delete,
                    contentDescription = "Delete button",
                )
            }
        },
    )
}

@Preview(widthDp = 403)
@Composable
private fun ListItemWithoutTrailingPaddingIssue() {
    ListItem(
        overlineContent = {
            Text("Overline content")
        },
        headlineContent = {
            Text("Headline content reaches to trailing content")
        },
        supportingContent = {
            Text("Supporting content")
        },
        trailingContent = {
            Icon(
                imageVector = Icons.Default.Delete,
                contentDescription = "Delete button",
            )
        },
    )
}

TrailingPaddingIssue.png

1002 KB