Obsolete 48dp height in ListHeader [251166127]

Fixed

Bug

Status Update

No update yet.

Description

gu...@google.com

created issue #1

Oct 5, 2022 03:19PM

After discussing Figma designs with UX, I got the following:

The 48DP fixed height header is now obsolete as SysUI allows for 3 lines and just pushes the content down

They are referring to the height defined in: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:wear/compose/compose-material/src/main/java/androidx/wear/compose/material/ListHeader.kt;l=53?q=ListHeader

So I believe there should be code changes to not have the height hardcoded?

Full discussion: https://www.figma.com/file/G6QEiBHkZ9TeKyBNUybJao?node-id=50:20539#279549982

Component used: ListHeader

Version used: 1.0.0

Devices/Android versions reproduced on: N/A

Comments

jn...@google.com <jn...@google.com> #2Oct 6, 2022 09:00AM

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

jn...@google.com <jn...@google.com> #3Oct 17, 2022 05:10PM

Assigned to ma...@google.com.

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

jn...@google.com <jn...@google.com> #4Nov 7, 2022 10:26AM

Reassigned to vi...@google.com.

Should we be looking to change this as part of the WM3? Do we need to change either the WM2 spec/guidance or existing code?

pe...@google.com <pe...@google.com> #5Dec 19, 2023 11:48AM

These tickets were part of Compose UX backlog hotlist

Adding these to wear_ux_sprint_inbox so they can be reassessed and verify if UX action is needed or if obsolete

st...@google.com <st...@google.com> #6Jan 25, 2024 04:54PM

Following this https://chat.google.com/room/AAAAYRnp4-Y/8xkRlG-EsbE in Compose Friends, this ticket should be treated as an accessibility issue if UX confirm that ListHeader should show up to 3 lines of text. Currently, that would fail to expand to show 3 lines for the largest font size (please ignore the clipping at the top as that's just a result of the demo app being used for testing):

Before with ListHeader using Modifier.height(48.dp) - only room for 2 lines: https://screenshot.googleplex.com/5zi9RqeAwoTQaPK
After with ListHeader using defaultMinSize(minHeight = 48.dp).height(IntrinsicSize.Min) - expands to 3 lines: https://screenshot.googleplex.com/9L43x8dcqTtJ6VZ

st...@google.com <st...@google.com> #7Jan 25, 2024 04:57PM

Note that if we change ListHeader to use Modifier.defaultMinSize(minHeight = 48.dp).height(IntrinsicSize.Min), then according to my testing, that allows developers to override the height in their own code, which is what the chat in comment 6 was originally about.

st...@google.com <st...@google.com> #8Jan 25, 2024 05:03PM

FWIW I notice that we have already made exactly this change in our Material3 codebase following a UX code review (see aosp/2715054)

st...@google.com <st...@google.com> #9Jan 26, 2024 10:16AM

@vincenttam Please will you confirm if we should make this change to expand the ListHeader to accommodate contents in Material 2.5 - thanks.

vi...@google.com <vi...@google.com> #10Jan 26, 2024 08:05PM

Yes, confirming from UX that we should update to ensure the ListHeader can accommodate contents for up to 3 lines of text (and behaviour is to push the content below further down as a result)

We made this update on SysUI for accessibility reasons, as Steve has noted in comment#6, to ensure scalability for larger text sizes and also localized strings

st...@google.com <st...@google.com> Jan 29, 2024 10:04AM

Reassigned to st...@google.com.

ap...@google.com <ap...@google.com> #11Jan 30, 2024 11:41AM

Project: platform/frameworks/support
Branch: androidx-main

commit 150d8bdb16a8d55375780e8092c9fd8aae4ea5aa
Author: stevebower <stevebower@google.com>
Date: Mon Jan 29 17:43:27 2024

Update ListHeader so that its height adjusts to accommodate its contents when large font sizes are used.

Test: Added multi-line example to integration demos for testing
Bug: 251166127
Relnote: "We have updated the ListHeader to support height adjustments when contents need extra height to accommodate large font sizes."
Change-Id: I7290ce63e80f480bf67da630013b875aa0005ee2

M wear/compose/compose-material/src/main/java/androidx/wear/compose/material/ListHeader.kt
M wear/compose/integration-tests/demos/src/main/java/androidx/wear/compose/integration/demos/MaterialDemos.kt

https://android-review.googlesource.com/2933031

st...@google.com <st...@google.com> Feb 1, 2024 12:02PM

Marked as fixed.