-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Rationalize text input widgets #9119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
8dafe6d to
e064a3c
Compare
e064a3c to
3530239
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the Material mocks, the * should be red when the field doesn't have a value (even when the label is another colour). We didn't support this before, but it's something worth bearing in mind. Maybe a reason to take a Text here or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we factored this out into the InputDecoration class itself, then you could pass in entirely novel decorations, much as we do with DecoratedBox.
Actually that kind of seems like a primitive we should have. build(BuildContext context, Widget child). AnimatedBuilder does something like that, though it passes in a Listenable as well IIRC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually AnimatedBuilder's builder does have exactly that signature. The listenable is passed in out of band.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe AnimatedBuilder is exactly what we should use here, with InputDecoration just being a class that extends Listenable and exposes a method with that signature, and you pass in both the Decoration and the method, or something...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not quite so clean because there's some interaction between the InputDecoration data and the other parameters to the widget. For example, the baseStyle and the isFocused bit.
Also, you don't gain a ton by doing that because you can always pass null to TextField to remove the decoration and just wrap it yourself with whatever you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something's off with this code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that code was outdated. I've removed it since the point it's going after doesn't make sense anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this actually work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does something, but not much.
|
This looks great. LGTM. If you add or edit docs in this patch and want that reviewed too, add them as a separate commit and I'll be happy to review those too. |
3530239 to
104a3d1
Compare
104a3d1 to
6e1b10f
Compare
|
I updated the docs, but I squashed the commit before seeing your note. :( |
6e1b10f to
f225e8e
Compare
After this patch, there are three major text input widgets: * EditableText. This widget is a low-level editing control that interacts with the IME and displays a blinking cursor. * TextField. This widget is a Material Design text field, with all the bells and whistles. It is highly configurable and can be reduced down to a fairly simple control by setting its `decoration` property to null. * TextFormField. This widget is a FormField that wraps a TextField. This patch also replaces the InputValue data model for these widgets with a Listenable TextEditingController, which is much more flexible. Fixes flutter#7031
After this patch, there are three major text input widgets:
EditableText. This widget is a low-level editing control that
interacts with the IME and displays a blinking cursor.
TextField. This widget is a Material Design text field, with all the
bells and whistles. It is highly configurable and can be reduced down
to a fairly simple control by setting its
decorationproperty tonull.
TextFormField. This widget is a FormField that wraps a TextField.
This patch also replaces the InputValue data model for these widgets
with a Listenable TextEditingController, which is much more flexible.
Fixes #7031