Status Update
Comments
ae...@google.com <ae...@google.com> #3
Thanks for the report!
lp...@google.com <lp...@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.
ae...@google.com <ae...@google.com> #5
an...@google.com <an...@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).
ae...@google.com <ae...@google.com> #7
Hmm. I notice when attempting to remove data
keyword from ProgressBarRangeInfo
, one of the ProgressSemanticsTest
fails because the test matchers use ==
. When data
is removed, then ==
silently has the same meaning as ===
in Kotlin.
So it is hard to statically determine whether there are any uses of == with a data class
, and a systematic removal may introduce bugs which may not all be caught by tests. I guess the safe way to remove data
is to make sure we also add equals
to all of them at the same time. (As I understand it, the other methods: hashCode
, toString
, componentN
, and copy
would all cause a compile error if something relies on them, so we don't need to worry about those as much.)
si...@google.com <si...@google.com> #8
hashcode sounds extra dangerous.
ToString is a good utility while debugging our code, and developers
debugging our code.since all 3 methods can be automatically created using
android studio, i dont see a reason/value to create equals but not hashcode
and tostring.
On Mon, Feb 1, 2021, 16:34 aelias <buganizer-system+aelias@google.com>
wrote:
ae...@google.com <ae...@google.com> #9
Makes sense. Also, equals
/hashCode
/toString
are the three methods on Any
, so overriding them doesn't affect our public API at all (they don't change any lines in current.txt
).
ae...@google.com <ae...@google.com> #10
OK, I did a survey of all the data classes and uploaded a patch to remove a bunch at aosp/1569820 . In the end, I decided the majority of the data classes looked pretty appropriate to use (things like IntRect
) and I left them as is, so I changed my mind about the lint rule, it would be too cumbersome to ban it. The API review process can reasonably take care of it in the future.
ap...@google.com <ap...@google.com> #11
Branch: androidx-main
commit 9b3a65a1038c2da421e0dda0781d2189563e2c6f
Author: Alexandre Elias <aelias@google.com>
Date: Mon Feb 01 21:42:52 2021
Replace data classes with equals/hashCode/toString
This replaces "data" with the Android Studio autogenerated
implementations of equals/hashCode/toString in several classes, in order
to minimize public API surface and improve extensibility.
Note that the majority of the uses of "data" are in dead-simple classes
in graphics, geometry and layout like "IntRect", and I decided it was
appropriate to keep using "data" for those.
Bug: 178659281
Test: Existing tests
Relnote: "Destructuring and copy() methods have been removed from
several classes where they were rarely used."
Change-Id: I267021d3a45314acc9a169f6bbdfbfb4295a448c
M compose/animation/animation-core/api/current.txt
M compose/animation/animation-core/api/public_plus_experimental_current.txt
M compose/animation/animation-core/api/restricted_current.txt
M compose/animation/animation-core/src/commonMain/kotlin/androidx/compose/animation/core/DynamicTargetAnimation.kt
M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/BorderStroke.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/animation/FlingConfig.kt
M compose/material/material/api/current.txt
M compose/material/material/api/public_plus_experimental_current.txt
M compose/material/material/api/restricted_current.txt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/Swipeable.kt
M compose/ui/ui-graphics/api/current.txt
M compose/ui/ui-graphics/api/public_plus_experimental_current.txt
M compose/ui/ui-graphics/api/restricted_current.txt
M compose/ui/ui-graphics/src/commonMain/kotlin/androidx/compose/ui/graphics/Shadow.kt
M compose/ui/ui-graphics/src/commonMain/kotlin/androidx/compose/ui/graphics/drawscope/DrawScope.kt
M compose/ui/ui-graphics/src/commonMain/kotlin/androidx/compose/ui/graphics/painter/ColorPainter.kt
M compose/ui/ui-graphics/src/commonMain/kotlin/androidx/compose/ui/graphics/painter/ImagePainter.kt
M compose/ui/ui/api/current.txt
M compose/ui/ui/api/public_plus_experimental_current.txt
M compose/ui/ui/api/restricted_current.txt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/autofill/Autofill.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/semantics/SemanticsProperties.kt
Description
With the below command, I see Compose has 46 data classes in our public API. This commits us to supporting APIs like equals() and component1(). It means among other things that changing the order of non-constructor fields in the class would be a breaking API change. We should do a cleanup pass and remove "data" except in the few cases where destructuring style is part of our documented sample code.
frameworks/support/compose$ git grep 'method public.*component1();' -- */current.txt