Status Update
Comments
mm...@commonsware.com <mm...@commonsware.com> #2
Hi Nick!
TextField currently have no skinning, borders etc, and it let's the API users (i.e. FilledTextField) to define those. Even though hint (placeholder) is a common TextField feature, material had animations etc on that feature which made us believe that it highly depends on the design system to provide the hint, and foundation should ideally be independent of the design system.
Previously we had only 2 Textfield implementations, 1 on material, 1 in core. If that structured did hold, I would recommend against adding the placeholder to core.TextField. However now we have 3 of them :)
I added nona, matvei and lpf to the ticket to get their opinions.
lu...@gmail.com <lu...@gmail.com> #3
I agree with Siyamed's analysis here, the implementation of a placeholder is evidently something specific to a design system. If you aren't using the Material implementation of a text field, it seems reasonable for you to need to build your own placeholder implementation - I think there is too much complexity and room for customization with transitions etc for it to be possible to build a generic version that is flexible enough to support all use cases.
ch...@google.com <ch...@google.com> #4
Yes, I agree. Maybe we should add a sample to the documentation so that developers have something to begin with when they need to develop their own?
mm...@commonsware.com <mm...@commonsware.com> #5
Adding a sample makes sense to me. I thought we previously had it but right now I couldn't find it.
Nick does it sound good to you as well?
[Deleted User] <[Deleted User]> #6
I'm not sure I agree that it depends upon design system; I can see many users wanting a simple placeholder that appears when no text is entered and disappears otherwise. Correctly implementing could be complex to correctly synchronize alignment, lines height, cursor etc. I see the benefit of having this in a library function that we can update etc.
Also asking many developers to implement this or directly copy a sample seems to be an indication the we should provide it?
Would we consider a PlaceholderTextField
in foundation?
cl...@google.com <cl...@google.com>
ap...@google.com <ap...@google.com> #7
I'm strongly against PlusOneFeature components in foundaiton, so I don't think PlaceholderTextField is a good component to have.
However, Nick's concerns make total sense to me, so I imagine we could end up with some placeholder api in foundation, but I'm not sure what is it now.
Anastasia, let's keep an eye on what we have in MaterialTextFields and maybe it will make sense to extract some placeholder functionality to the foundation version
Description
Android Studio 3.5 Beta 1
Build #AI-191.6707.61.35.5677133, built on June 20, 2019
JRE: 1.8.0_202-release-1483-b39-5396753 amd64
JVM: OpenJDK 64-Bit Server VM by JetBrains s.r.o
Linux 5.0.0-25-generic
(specifically the one from the Compose repo)
Version of Gradle Plugin: 3.5.0-beta05
Version of Gradle: whatever frameworks/support/ui is using -- your setup is too damn complicated...
Version of Kotlin: 1.3.41
OS: Ubuntu 19.04
Steps to Reproduce:
Step #1: Define a @Model as a top-level class:
@Model
data class AgreementViewState(var terms: Boolean, var privacy: Boolean) {
val canProceed = terms && privacy
}
Step #2: Attempt to use that @Model in a @Composable:
@Composable
private fun Agreement() {
Padding(8.dp) {
val viewState = +model { AgreementViewState(true, false) }
Column(mainAxisAlignment = MainAxisAlignment.Start, crossAxisAlignment = CrossAxisAlignment.Start) {
Row(mainAxisAlignment = MainAxisAlignment.Start) {
Checkbox(
checked = viewState.terms,
onCheckedChange = { viewState.terms = it }
)
Text(text = "I agree to the terms and conditions")
}
Row(mainAxisAlignment = MainAxisAlignment.Start) {
Checkbox(
checked = viewState.privacy,
onCheckedChange = { viewState.privacy = it }
)
Text(text = "I agree to the privacy policy")
}
Text(text = if (viewState.canProceed) "You may proceed" else "Please indicate your agreement")
}
}
}
Expected Results: No crashes
Actual Results: A crash from generated code when the @Composable is called:
java.lang.IllegalArgumentException: Parameter specified as non-null is null: method kotlin.jvm.internal.Intrinsics.checkParameterIsNotNull, parameter r
at androidx.compose.frames.FramesKt._readable(Unknown Source:2)
at androidx.ui.foundation.demos.AgreementViewState.getTerms(Unknown Source:5)
at androidx.ui.foundation.demos.AgreementViewState.<init>(MainActivity.kt:230)
Note that this works:
@Model
data class AgreementViewState(var terms: Boolean, var privacy: Boolean) {
fun canProceed() = terms && privacy
}
...and this works:
@Model
data class AgreementViewState(var terms: Boolean, var privacy: Boolean) {
// fun canProceed() = terms && privacy
}
val AgreementViewState.canProceed
get() = terms && privacy
But, IMHO, the original form ought to work as well, as it seems to be the most natural form of a derived property in Kotlin.
Thanks for considering this!