Line Height improvements for Text [181155707]

Fixed

Feature Request

Status Update

No update yet.

Description

si...@google.com

created issue #1

Feb 24, 2021 09:10PM

created from a suggestion on slack channel

Have you considered changing the behavior of lineHeight to match web? Figma recently made this change (https://www.figma.com/blog/line-height-changes/) and it’s become difficult to match specs from our designers using views. I don’t have an opinion on which one is “correct”, but maybe this is a better default?

https://androidstudygroup.slack.com/archives/CJH03QASH/p1598645885188600?thread_ts=1598645885.188600&cid=CJH03QASH

will attach the conversation separately

Related to b/181049465

Comments

si...@google.com <si...@google.com> #2Feb 24, 2021 09:20PM

It is expected that BackHandler crashes in cases where no LocalOnBackPressedDispatcherOwner is provided.

Given that LocalOnBackPressedDispatcherOwner.current is nullable, you can certainly wrap your call to BackHandler in a null check if you'd like to specifically disable that functionality when in @Preview. Of course, you could just provide your own OnBackPressedDispatcherOwner by constructing an OnBackPressedDispatcher instance yourself as part of your @Preview.

Andrey - any thoughts on whether we should have @Preview automatically create and set a LocalOnBackPressedDispatcherOwner as we do for LocalViewModelStoreOwner? Who would be the correct assignee for that?

si...@google.com <si...@google.com> #3Feb 15, 2022 07:40PM

If you think you want to provide some fake implementation for previews you probably want to provide it somewhere here:

https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/ui/ui-tooling/src/androidMain/kotlin/androidx/compose/ui/tooling/preview/ComposeViewAdapter.kt;l=478;drc=38caeffe2a32b6135f231b83c54b109e30bcd474, and ask Diego for a review.
Note that in previews we do not really support ViewModels in reality. We provide a fake ViewModelStoreOwner, but this implementation just crashes when you try to create a ViewModel. It was a Sergey's idea as usually ViewModels are doing some network/database requests and we don't really support it in the preview mode.

si...@google.com <si...@google.com> #4Mar 14, 2022 04:55AM

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

commit 026203513c5a704abeaa34595e7a7e7ab06876f7
Author: Ian Lake <ilake@google.com>
Date: Mon Jun 14 11:47:14 2021

Add an ActivityResultRegistry to Compose Previews

An ActivityResultRegistry is provided by default
by ComponentActivity, but using APIs that depend on
it, like rememberLauncherForActivityResult() would
crash in @Preview. By adding a fake implementation,
developers can preview those Composables.

For cases where they call `launch()` (i.e., the
`startActivityForResult()` equivalent), the fake
throws an IllegalStateException to indicate that
actually starting an activity / permission request
in Preview is not supported.

Relnote: "Compose `@Preview` now provides a
`LocalActivityResultRegistryOwner` that allows you
to preview Composables that use APIs like
`rememberLauncherForActivityResult()` that depend
on that owner existing."
Test: new ComposeViewAdapterTest test passes
BUG: 185693006

Change-Id: Ib13d12f085065adb89cfb731179b9295029e06e9

M compose/ui/ui-tooling/src/androidAndroidTest/kotlin/androidx/compose/ui/tooling/SimpleComposablePreview.kt
M compose/ui/ui-tooling/src/androidAndroidTest/kotlin/androidx/compose/ui/tooling/preview/ComposeViewAdapterTest.kt
M compose/ui/ui-tooling/src/androidMain/kotlin/androidx/compose/ui/tooling/preview/ComposeViewAdapter.kt

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

Message last modified on Mar 25, 2022 05:46AM

si...@google.com <si...@google.com> #5Mar 26, 2022 06:03PM

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

commit 622947a991ba924d4896965f708be022bf1b35ba
Author: Ian Lake <ilake@google.com>
Date: Mon Jun 14 11:34:13 2021

Add a fake OnBackPressedDispatcher to Compose Previews

Rather than having APIs that depend on
LocalOnBackPressedDispatcher crash in @Preview,
provide a fake implementation by default to mirror
the capabilities that are available by default
in a ComponentActivity.

Relnote: "Compose `@Preview` now provides a
`LocalOnBackPressedDispatcherOwner` that allows you
to preview Composables that use APIs like `BackHandler`
that depend on that owner existing."
Test: new ComposeViewAdapterTest test passes
BUG: 185693006

Change-Id: Ia1c05ea3cadf2fb55cef9c4b8bae0898cfb35ba9

M compose/ui/ui-tooling/src/androidAndroidTest/kotlin/androidx/compose/ui/tooling/SimpleComposablePreview.kt
M compose/ui/ui-tooling/src/androidAndroidTest/kotlin/androidx/compose/ui/tooling/preview/ComposeViewAdapterTest.kt
M compose/ui/ui-tooling/src/androidMain/kotlin/androidx/compose/ui/tooling/preview/ComposeViewAdapter.kt

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

Message last modified on Mar 27, 2022 09:01PM

line_height_distribution.mp4

2 MB

Download

cs...@google.com <cs...@google.com> #6Mar 28, 2022 04:43PM

We've added default a LocalOnBackPressedDispatcherOwner and LocalActivityResultRegistryOwner to @Preview, so APIs that depend on these existing won't crash out of the box.

si...@google.com <si...@google.com> Apr 12, 2022 05:18AM

Reassigned to si...@google.com.

ap...@google.com <ap...@google.com> #7Apr 18, 2022 03:07PM

Thanks!

ap...@google.com <ap...@google.com> #8Apr 18, 2022 03:07PM

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

commit d0853b60893256b272ed9b3bd444b209de707118
Author: Siyamed Sinir <siyamed@google.com>
Date: Thu Apr 07 15:32:11 2022

Line height behavior and vertical alignment

This CL adds LineHeightBehavior to the TextStyle and
ParagraphStyle. LineHeightBehavior controls whether line height is
applied to the top of the first line and to the bottom of the last
line. It also defines the alignment of line in the space provided
by TextStyle(lineHeight).

go/compose-lineheight-distribution

RelNote: "Added LineHeightBehavior to the TextStyle and
ParagraphStyle. LineHeightBehavior controls whether line height is
applied to the top of the first line and to the bottom of the last
line. It also defines the alignment of line in the space provided
by TextStyle(lineHeight).

For example it is possible to get a behavior similar to what CSS
defines via LineHeightBehavior(
alignment = LineVerticalAlignment.Center,
trimFirstLineTop=false,
trimLastLineBottom = false).

trimFirstLineTop, trimLastLineBottom configurations works correctly
only when includeFontPadding is false."

Test: Added demo
Test: Added tests
Test: ./gradlew text:text:test
Test: ./gradlew text:text:cAT
Test: ./gradlew compose:ui:ui-text:test
Test: ./gradlew compose:ui:ui-text:cAT
Test: ./gradlew compose:foundation:foundation:test
Test: ./gradlew compose:foundation:foundation:cAT

Bug: 181155707
Change-Id: I973329a540ca9f5a6e225f1e5aaffeaa1ff9cc61

M compose/ui/ui-inspection/src/androidTest/java/androidx/compose/ui/inspection/inspector/ParameterFactoryTest.kt
A compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/text/TextDemoMetrics.kt
M compose/ui/ui-text/api/restricted_current.txt
M compose/ui/ui-text/src/test/java/androidx/compose/ui/text/TextStyleLayoutAttributesTest.kt
A compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/text/ComposeLineHeight.kt
A compose/ui/ui-text/src/androidAndroidTest/kotlin/androidx/compose/ui/text/ParagraphIntegrationLineHeightBehaviorTest.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/text/TextDemos.kt
M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/ParagraphStyle.kt
M text/text/src/main/java/androidx/compose/ui/text/android/TextLayout.kt
M compose/ui/ui-text/src/test/java/androidx/compose/ui/text/TextStyleTest.kt
A text/text/src/main/java/androidx/compose/ui/text/android/style/LineHeightBehaviorSpan.kt
M text/text/src/main/java/androidx/compose/ui/text/android/style/LineHeightSpan.kt
A text/text/src/androidTest/java/androidx/compose/ui/text/android/style/LineHeightBehaviorSpanTest.kt
A compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/style/LineHeightBehavior.kt
M compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/platform/AndroidParagraphHelper.android.kt
M compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/platform/AndroidParagraph.android.kt
M compose/ui/ui-text/api/public_plus_experimental_current.txt
M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/TextStyle.kt
A compose/ui/ui-text/src/test/java/androidx/compose/ui/text/style/LineHeightBehaviorTest.kt
M compose/ui/ui-text/api/current.txt
M compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/platform/extensions/SpannableExtensions.android.kt
M compose/ui/ui-text/src/test/java/androidx/compose/ui/text/ParagraphStyleTest.kt

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

ap...@google.com <ap...@google.com> #9Apr 18, 2022 03:53PM

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

commit 8f0b7683ecd7e786ba894e0833752ca2c763e5cd
Author: Siyamed Sinir <siyamed@google.com>
Date: Fri Apr 15 15:07:50 2022

Make LineHeightBehavior.Default the default

This CL assigns LineHeightBehavior.Default as the default line
height behavior for includeFontPadding = false case.

Test: Tested on Demo app
Test: ./gradlew text:text:test
Test: ./gradlew text:text:cAT
Test: ./gradlew compose:ui:ui-text:test
Test: ./gradlew compose:ui:ui-text:cAT

Bug: 181155707
Change-Id: I265f53c7c2a4dd135722a21f53d795b4361779c6

M text/text/src/main/java/androidx/compose/ui/text/android/style/LineHeightSpan.kt
M compose/ui/ui-text/src/androidAndroidTest/kotlin/androidx/compose/ui/text/platform/AndroidParagraphTest.kt
M compose/ui/ui-text/src/androidAndroidTest/kotlin/androidx/compose/ui/text/ParagraphFillBoundingBoxesTest.kt
M compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/platform/AndroidParagraphHelper.android.kt
M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/ParagraphStyle.kt
M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/TextStyle.kt
M compose/ui/ui-text/src/androidAndroidTest/kotlin/androidx/compose/ui/text/ParagraphIntegrationTest.kt
M compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/platform/extensions/SpannableExtensions.android.kt

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

si...@google.com <si...@google.com> #10Apr 18, 2022 06:30PM

Attaching the behavior with different configurations for future.

Mon Apr 18 2022 11:29:35 GMT-0700 (Pacific Daylight Time).png

1 MB

View

Download

si...@google.com <si...@google.com> Apr 18, 2022 08:43PM

Marked as fixed.

ap...@google.com <ap...@google.com> #11Apr 18, 2022 08:54PM

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

commit 4b23d9be8cfe80dcd00d065e9375ba5b58a8d495
Author: Siyamed Sinir <siyamed@google.com>
Date: Fri Apr 15 19:39:52 2022

Introduce LineHeightTrim

LineHeightBehavior worked with two booleans conceptually related to
each other: trimFirstLineTop, trimLastLineBottom

This CL unifies those two boolean values in a single enum named
LineHeightTrim.

LineHeightTrim have values of 4 states defined by two booleans:
- FirstLineTop
- LastLineBottom
- Both
- None

RelNote: "trimFirstLineTop, trimLastLineBottom attributes of
LineHeightBehavior changed into a single enum: LineHeightTrim.
LineHeightTrim have values of 4 states defined by two booleans:
FirstLineTop, LastLineBottom, Both and None"

Test: ./gradlew compose:ui:ui-text:test
Test: ./gradlew compose:ui:ui-text:cAT

Bug: 181155707
Change-Id: Ifc6a5912eab7a0e41ae6cd4045ea9cbdf3c0a146

M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/style/LineHeightBehavior.kt
M compose/ui/ui-text/src/androidAndroidTest/kotlin/androidx/compose/ui/text/ParagraphFillBoundingBoxesTest.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/text/ComposeLineHeight.kt
M compose/ui/ui-text/src/androidAndroidTest/kotlin/androidx/compose/ui/text/ParagraphIntegrationLineHeightBehaviorTest.kt
M compose/ui/ui-text/api/public_plus_experimental_current.txt
M compose/ui/ui-text/src/test/java/androidx/compose/ui/text/TextStyleTest.kt
M compose/ui/ui-text/src/test/java/androidx/compose/ui/text/style/LineHeightBehaviorTest.kt
M compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/platform/extensions/SpannableExtensions.android.kt

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

ap...@google.com <ap...@google.com> #12Apr 27, 2022 04:17PM

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

commit 5095a557ff762472327d950541786ae9828ad036
Author: Siyamed Sinir <siyamed@google.com>
Date: Mon Apr 25 15:35:47 2022

LineHeightBehavior API Review updates

Based on API review, this CL
- Renames LineHeightBehavior into LineHeightStyle
- Renames LineVerticalAlignment into LineHeightStyle.Alignment
- Renames LineHeightTrim into LineHeightStyle.Trim
- Removes default constructor values from LineHeightStyle

RelNote: "
- LineHeightBehavior is renamed as LineHeightStyle
- LineVerticalAlignment is renamed as LineHeightStyle.Alignment
- Renames LineHeightTrim is renamed as LineHeightStyle.Trim
- Default constructor values from LineHeightStyle is removed
"

Test: ./gradlew compose:ui:ui-text:test
Test: ./gradlew compose:ui:ui-text:cAT
Test: ./gradlew text:text:test
Test: ./gradlew text:text:cAT

Bug: 230347691
Bug: 181155707
Change-Id: I582bf09152d60b30362b3cce9bd60d57fc488fe7

M compose/ui/ui-text/api/restricted_current.txt
M compose/ui/ui-text/src/test/java/androidx/compose/ui/text/TextStyleLayoutAttributesTest.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/text/ComposeLineHeight.kt
M compose/ui/ui-text/src/androidAndroidTest/kotlin/androidx/compose/ui/text/ParagraphIntegrationLineHeightStyleTest.kt
M compose/ui/ui-inspection/src/main/java/androidx/compose/ui/inspection/inspector/ParameterFactory.kt
M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/ParagraphStyle.kt
M text/text/src/main/java/androidx/compose/ui/text/android/TextLayout.kt
M text/text/src/androidTest/java/androidx/compose/ui/text/android/TextLayoutTest.kt
M compose/ui/ui-text/src/test/java/androidx/compose/ui/text/TextStyleTest.kt
M text/text/src/androidTest/java/androidx/compose/ui/text/android/style/LineHeightStyleSpanTest.kt
D compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/style/LineHeightBehavior.kt
M compose/ui/ui-text/src/androidAndroidTest/kotlin/androidx/compose/ui/text/ParagraphFillBoundingBoxesTest.kt
M compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/platform/AndroidParagraphHelper.android.kt
A compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/style/LineHeightStyle.kt
A compose/ui/ui-text/src/test/java/androidx/compose/ui/text/style/LineHeightStyleTest.kt
M text/text/src/main/java/androidx/compose/ui/text/android/style/LineHeightStyleSpan.kt
M compose/ui/ui-text/api/public_plus_experimental_current.txt
M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/TextStyle.kt
D compose/ui/ui-text/src/test/java/androidx/compose/ui/text/style/LineHeightBehaviorTest.kt
M compose/ui/ui-text/api/current.txt
M compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/platform/extensions/SpannableExtensions.android.kt
M compose/ui/ui-text/src/test/java/androidx/compose/ui/text/ParagraphStyleTest.kt

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

ap...@google.com <ap...@google.com> #13Mar 14, 2023 06:49PM

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

commit 5ddf42b0308c9c64303dd378baae8b13aa1f5614
Author: Siyamed Sinir <siyamed@google.com>
Date: Mon Apr 18 11:09:47 2022

Use LineHeightStyle as default for includeFontPadding=true

LineHeightStyle(Alignment.Proportional, Trim.Both) is the current default for
includeFontPadding=false.

This CL makes it the default for includeFontPadding=true as well.

The previous behavior includeFontPadding=true should not change since
it was the same or similar as LineHeightStyle(proportional, trim
both).

The change required no update in regular tests. Treehugger will
validate screenshot tests.

Test: ./gradlew compose:ui:ui-text:test
Test: ./gradlew compose:ui:ui-text:cAT
Test: ./gradlew text:text:test
Test: ./gradlew text:text:cAT

Bug: 181155707
Change-Id: Ib22aaeef41d2b2b6888be7a295524a2baa43c615

M compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/platform/AndroidParagraphHelper.android.kt
M compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/platform/extensions/SpannableExtensions.android.kt
M text/text/src/androidTest/java/androidx/compose/ui/text/android/TextLayoutIntrinsicWidthTest.kt
D text/text/src/main/java/androidx/compose/ui/text/android/style/LineHeightSpan.kt
M text/text/src/main/java/androidx/compose/ui/text/android/style/LineHeightStyleSpan.kt

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

ju...@google.com <ju...@google.com> #14May 24, 2023 07:37PM

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

androidx.compose.ui:ui-text-android:1.5.0-beta01