Skip to content

Conversation

@abarth
Copy link
Contributor

@abarth abarth commented Mar 31, 2017

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 #7031

@abarth
Copy link
Contributor Author

abarth commented Mar 31, 2017

This patch is stacked on #9074, so you probably want to just look at the second commit independently.

Also, I need to add/update a bunch of docs, but this patch should otherwise be ready for review.

/cc @Hixie

@abarth abarth force-pushed the text_controller3 branch from e064a3c to 3530239 Compare March 31, 2017 09:45
Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor

@Hixie Hixie Mar 31, 2017

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.

Copy link
Contributor

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...

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this actually work?

Copy link
Contributor Author

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.

@Hixie
Copy link
Contributor

Hixie commented Mar 31, 2017

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.

@abarth
Copy link
Contributor Author

abarth commented Mar 31, 2017

I updated the docs, but I squashed the commit before seeing your note. :(

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
@abarth abarth force-pushed the text_controller3 branch from f225e8e to abd6da3 Compare April 1, 2017 05:02
@abarth abarth merged commit ae89948 into flutter:master Apr 2, 2017
@abarth abarth deleted the text_controller3 branch April 2, 2017 00:30
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Need to rationalise the names of the *Input* and *Field* classes

3 participants