Status Update
Comments
cl...@google.com <cl...@google.com> #2
ja...@google.com <ja...@google.com> #3
Thanks for the report!
fa...@google.com <fa...@google.com> #4
The release notes documentation has been edited to clarify this change in behavior for line height.
To support non-standard text sizes, we encourage users to follow the Material design system and use a different style = LocalTextStyle.current.copy(lineHeight = TextUnit.Unspecified)
, or create a custom Typography
entirely.
si...@google.com <si...@google.com> #6
In my case, I have multiple font sizes in the same Text
(using SpanStyle
in AnnotatedString
). There are legitimate reasons for this. For example, when combining Chinese and English (phonetic) together (for language-learning purposes).
an...@google.com <an...@google.com> #7
ap...@google.com <ap...@google.com> #8
Branch: androidx-master-dev
commit 528739f5a4f9e6b397aa206de080953dd5e15a90
Author: haoyu <haoyuchang@google.com>
Date: Fri Sep 18 01:35:55 2020
Introducing TextOverflow.None
1. Support TextOverflow.None option. When overflow is None, Text/CoreText won't handle overflow anymore,
and it will report its actual size to LayoutNode.
2. Text/CoreText will always report its actual width/height to LayoutNode if it's smaller than the
minWidht/minHeight specified by constraints. This behavior helps LayoutNode to handle the cases where text
is too small.
Bug: 169217230
Bug: 158830170
Test: ./gradlew test
RelNote: "TextOverflow.None is introduced. When overflow is None, Text won't handle overflow anymore, and it will report its actual size to LayoutNode."
Change-Id: I175c9163a70ed35e4390b10848f143ed30ed2bf3
M compose/foundation/foundation-text/src/androidAndroidTest/kotlin/androidx/compose/foundation/text/TextFieldDelegateIntegrationTest.kt
M compose/foundation/foundation-text/src/androidAndroidTest/kotlin/androidx/compose/foundation/text/selection/SelectionContainerTest.kt
M compose/foundation/foundation-text/src/commonMain/kotlin/androidx/compose/foundation/text/CoreText.kt
M compose/foundation/foundation-text/src/commonMain/kotlin/androidx/compose/foundation/text/TextFieldDelegate.kt
M compose/foundation/foundation-text/src/test/kotlin/androidx/compose/foundation/text/TextFieldDelegateTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/LazyForIndexedTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Text.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/ListItem.kt
M compose/ui/ui-text/api/current.txt
M compose/ui/ui-text/api/public_plus_experimental_current.txt
M compose/ui/ui-text/api/restricted_current.txt
M compose/ui/ui-text/src/androidAndroidTest/kotlin/androidx/compose/ui/text/TextDelegateIntegrationTest.kt
M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/TextDelegate.kt
M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/TextLayoutHelper.kt
M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/style/TextOverflow.kt
si...@google.com <si...@google.com> #9
One question
When we made this change, as far as I understand somebody was passing us a min height is it correct?
si...@google.com <si...@google.com> #10
one option we had was to revert the CL since it causes issues that we did not foresee.
- For example in the devrel chat Chris Banes was surprised that all the text they had was now centered
- it looks like the layout system aligns things by default to center but not to start (which on its own imho a little off)
- If a text contains paragraphs in RTL and LTR then they wont be correctly aligned. (not for this ticket, but a related note, I am very not confident about the layout direction, textdirection, text align changes we made.)
Let's revert the change. and think again.
si...@google.com <si...@google.com> #12
But Text is incorrectly reporting its size when we have not enough size available.
Text was not incorrectly reporting its size, it was complying with the given max/min widths. so they were correct from the Text perspective.
an...@google.com <an...@google.com> #14
the original issue was that the text was saying that it is complying with the constraints - clamping the size by maxHeight - but it fact was drawing outside its reported size. so it said it will use 20.dp, but was drawing in a 30.dp area. This was very unpredictable. Now text at least clips by the bounds it reported. It is already better than before. And technically we can consider this bug as fixed by introducing clipping.
But what I don't like about text clipping is that it is inconsistent with everything else in Compose. No other composables clip on their own. If you want clip you add clip modifier. Imagine such situation:
Button(
modifier = Modifier
.preferredHeight(36.dp),
elevation = ButtonConstants.defaultElevation(0.dp),
onClick = { },
) {
Text("SEND", fontSize = 20.sp)
Image(Icons.Default.Add, modifier = Modifier.size(60.dp))
}
See the attached screenshot with how it currently works. The Button is said to have 36.dp. Button also introduces inner padding. The available scape is not enough to correctly fill both Text and Image, but they react differently: Text clips, but Image fills the real requested content size and as there is not enough space in the parent - parent automatically centres the Image - the default behavior Mihai build in the layout system
si...@google.com <si...@google.com> #15
Thanks Andrey.
The clipping issue
- is something that I am aware of (the other components not clipping). As
- you also mentioned it is another problem.
- Let me create an issue for it.
- On Text side I am happy not to clip but it comes with problems of expectations.
- TextOverflow currently have Clip, maybe it should have, and we should rely on modifiers (at the time we didnt have that)
- or TextOverflow should still have None, where Text says ok, i won't clip it is up to developer (which I like better).
- therefore we would be giving an option to developer to have more control on the clipping behavior.
The centering issue:
- Question:
- When Image is given a larger width, does it scale? Does it comply?
- Specific to the change we made before:
- We said Text should either comply or not comply for both width and height. If it accepts width to be larger (changing this cause trouble), then it should accept height to be larger.
- Mihai mentioned that Text is different in the sense that there is a different relation with the width and height (width in height out). Therefore height can be special.
I wonder if this issue also requires a configuration. We changed the behavior and hit to a wall, if we had a configuration (through a modifier or a config option) it would be possible for a developer to enable the behavior you mentioned: Text will not comply with the rules, it will fit its box.
Lets keep this ticket open.
si...@google.com <si...@google.com> #16
btw :)
Text("SEND", fontSize = 20.sp) Image(Icons.Default.Add, modifier = Modifier.size(60.dp))
Image gets a size. if Text had the same modifier Modifier.size(60.sp.toDp()) it would behave the same.
si...@google.com <si...@google.com> #17
some notes from discussion with Mihai
- the option mentioned in coment#15 can be TextOverflow.None
- TextOverflow.Clip might not be needed for Compose (it came from CSS originally)
- TextOverflow can be [None, Ellipsis] (maybe later Fade) which cuts the text but not the drawing.
- When TextOverflow.None, developers use clip modifier.
- Open question: When TextOverflow.None, Text reports the real size (definitely height, don't know width) (fot width: think about softwrap)
an...@google.com <an...@google.com> #18
Yes, I intentionally added the fixed size on Image as image is scaleable. Without the modifier it will just scale to fill the available amount. But text is not scaleable(eg will not auto decrease the font size to fill within the available area) so it was more fair to compare text with the Image with fixed size.
si...@google.com <si...@google.com> #19
for
Will close this ticket, and postpone the mentioned ticket.
Description
Button(
modifier = Modifier
.padding(horizontal = 16.dp)
.preferredHeight(36.dp),
elevation = 0.dp,
onClick = onMessageSent,
disabledContentColor = MaterialTheme.colors.onSurface.copy(alpha = 0.38f),
) {
Text(stringResource(id = R.string.send), modifier = Modifier.padding(horizontal = 16.dp))
}
Making the Text bigger than the Button will misalign it vertically (moving it down).
The workaround is to set the padding to 0:
padding = InnerPadding(0.dp)
And make sure the height of the contents doesn't exceed the Button's.