Update InputDecorator et al by HansMuller · Pull Request #13734 · flutter/flutter

HansMuller · 2017-12-21T19:21:05Z

Updates the appearance of InputDecorator which updates the appearance of most TextFields.

There's a short video of the new InputDecorator in action on YouTube. The screenshot below shows some of the new InputDecorator configurations.

InputDecorator "container"

In the descriptions of the new input decorator's features that follow the term 'container' is often used to refer to the areas highlighted in the screenshots below:

New Features

This version of InputDecorator is largely API compatible with the current one. Some new visual elements have been added and the internal layout has changed a bit.

The prefixIcon and suffixIcon appear at the leading or trailing edge of the input. Unlike the icon, they appear within the decorator's container.

In the screenshot below the suffixIcon appears on the right (because the prevailing text direction is LTR):

This screenshot shows an input decorator with icon and contentPadding that eliminates the default horizontal 12dps padding with the decorator's container.
The container part of the decorator can be filled with filled: true and the color can be specified with fillColor: color.
What was called the "divider" is now a configurable border. Specifying border: const OutlineInputBorder() draw's an outline around the decorator's container, border: const UnderlineInputBorder() underlines the input as before, and border: null doesn't draw anything (same as hideDivider: true in the previous version).
When the decorator is outlined, the floating label animates into a gap in the border.

The screenshots below show the decorator's label when the text field lacks the focus, and the floating label when it gains the focus.
When the errorText first appears it "shakes" left and right to get the user's attention.
The default padding of the decorator's content can be overridden with contentPadding: edgeInsets. For example to generate a layout that's similar to the current Input decorator: contentPadding: const EdgeInsets.only(bottom: 8.0, top: 16.0).

Incompatible Changes

The hideDivider property has been replaced by border: null.
The layout of the InputDecorator's elements has changed. The "inline" elements, like the input child, the labelText and hintText, are now padded 12 dps on the left and right if filled is true. The vertical padding is also different: previously there was 16dps above the floating label, and 8dps below the inline elements, now its 12dps above and below.
The labelText's location is a little different. It's vertically centered within the input decorator's container when isFocus is true or isEmpty is false. When the labelText is "floating"
it appears within a gap within the InputBorderType.outline or just a little lower than before for InputBorderType.underline.

Hixie · 2017-12-21T22:31:47Z

trivial nit: vertically align the subexpressions (i.e. indent by 3 more)

Hixie · 2017-12-21T22:33:35Z

convert into a TODO() or fix or remove as appropriate

Hixie · 2017-12-21T22:34:41Z

Hixie · 2017-12-21T22:35:12Z

Hixie · 2017-12-21T22:35:49Z

should we just fill if fillColor is non-null, and not fill if it's null?

Most apps will not want to actually specify a fill color, they'll just the default one that's based on the theme's brightness.

(that would work better in a world where there's a way to configure the fill color in the theme explicitly)

I agree. What might work best here is an InputDecoratorTheme which would be part of ThemeData like IconTheme and textTheme. I will add that in a follow-up PR.

Hixie · 2017-12-21T22:36:31Z

it'd be mildly more efficient to do hashValues(..., hashValues(...))

Hixie · 2017-12-21T22:38:17Z

looks like you don't actually use the textDirection in _precache?

Hixie · 2017-12-21T22:38:26Z

Hixie · 2017-12-21T22:51:41Z

My main feedback is that instead of an enum for the borderType, I would just define an abstract subclass of ShapeBorder (similar to what you have), and then have people provide instances of subclasses of that subclass as the borderType (call it just shape, probably, to match what @amirh is doing). That way you get two benefits: one, your shape border class gets split into two simpler ones since now each one only has to deal with one border style, and two, people get to build their own crazy border styles if they want to.

Hixie · 2017-12-21T22:52:31Z

you should also check the animation is the same

Hixie · 2017-12-21T22:54:31Z

i think this whole method can just assert false, not reachable

Hixie · 2017-12-21T22:54:54Z

if you skip the default case, then the analyzer will catch cases where you have an unrecognized one for you

Hixie · 2017-12-21T22:55:10Z

TBD->todo or fix

Hixie · 2017-12-21T22:55:20Z

->TODO or fix

Hixie · 2017-12-21T23:05:56Z

I think the gapAnimation should probably be an argument to the paint method. You don't use it anywhere else as far as I can tell, and passing in a Listenable to a class without a lifecycle is a little scary (is it going to listen to it? i don't know!).

Yes, that seems reasonable.

I replaced gapAnimation with gapPercentage, which is just the animation's value.

Hixie · 2017-12-21T23:06:58Z

can you elaborate on why it's important that they be circular?

The label text's gap can extend into the corners (even both the top left and top right corners) and I didn't want to bother with the more complicated problem of finding how far the gap penetrated into an elliptical corner. It didn't seem to be an important feature to support now. I suppose the summary rationale is: sloth.

abarth · 2017-12-22T05:52:39Z

Those outline animations are beautiful!

HansMuller · 2018-01-03T00:07:00Z

UnderlineInputBorder and OutlineInputBorder and their abstract superclass InputBorder (a ShapeBorder) are public classes that can be used to configure InputDecoration.border. I've removed the borderType parameter and InputBorderType enum.

new TextField(
  decoration: new InputDecoration(
    border: const OutlineInputBorder(),
    labelText: 'Hello World',
  ),
)

Hixie · 2018-01-03T21:31:30Z

I think we should add the following to this class, to avoid needing to hard-code any knowledge about InputBorder's subclasses in any of the other classes, thus allowing people to implement custom InputBorders that are crazier than currently allowed, while also simplifying the rest of the code:

add an abstract paint() method that has the arguments that OutlineInputBorder has.

add a property that returns an enum that describes how the padding/baseline should be applied (also covers the isOutlineBorder logic in the paint method).

We should also document in the class docs for InputBorder all the ways that this class differs from ShapeBorder, in particular:

That it always has a single side style, borderSide.

That copies of the class can be made, applying a different style, using copyWith.

That the paint method has additional arguments.

What the enum property described above is about.

InputBorder now has an abstract isOutline getter that's used by the renderer to configure the layout. The renderer tests this boolean instead of testing the border's type.

I updated the abstract methods and docs as you suggested.

Hixie · 2018-01-03T21:32:05Z

for completeness, also add borderRadius and gapPad

Hixie · 2018-01-03T21:32:09Z

gapPadding?

Yes, that's better.

Hixie · 2018-01-03T21:33:45Z

we just say "logical pixels", not "dps", in the Flutter documentation

~~where is the default of 4px implemented?~~ oh, i see, in the getter.

For efficiency it'd be better if that was done in the constructor.

why must the radii be circular?

This came up before: #13734 (comment)

I added a comment with the explanation.

Hixie · 2018-01-03T21:34:38Z

Hixie · 2018-01-03T21:34:57Z

use a switch

Hixie · 2018-01-03T21:35:44Z

assert circular

Now checked at the top of CircularInputBorder.paint().

Hixie · 2018-01-03T21:38:09Z

TODO should be actionable

This code isn't used so for now it's assert(false, 'not implemented').

Hixie · 2018-01-03T21:38:26Z

prefer switch instead of if when checking an enum value

jonasbark · 2018-01-09T17:54:09Z

+      final double t = decoration.floatingLabelProgress;
+      // The center of the outline border label ends up a little below the
+      // center of the top border line.
+      final double floatingY = decoration.border.isOutline ? -labelHeight * 0.25 : contentPadding.top;


this line throws an exception if border: null and a labelText is used

jup, can confirm.
https://gist.github.com/BenSower/e4bda931033a0fa306722e3da3b02fc9

Sorry to have missed this. I've filed and issue and will fix:
#14165

googlebot added the cla: yes label Dec 21, 2017