Skip to content

Commit

Permalink
Fixes InputDecorator to work with textScaleFactor, fixes Material Des…
Browse files Browse the repository at this point in the history 
…ign differences. (#12595)

Fixes InputDecorator to work with textScaleFactor, fixes Material Design differences.

There were a number of differences with the Material Design spec, including
several different padding values and underline thickness.  This corrects
that so that the decorator is in line with the Material Design spec now.

Also, the decorator properly handles changes to the textScaleFactor, where
before it would not re-layout when needed, painting the cursor and
underline incorrectly.

The decorator also now properly animates helper, error, and hint text when
the textScaleFactor or input decoration properties change.

Helper text is now properly displayed in dense mode, as the spec shows.
Before this change, it was never displayed in dense mode.

Fixes #12485
  • Loading branch information
@gspencergoog
gspencergoog committed Oct 19, 2017
1 parent 7e09649 commit 67cf791
Show file tree
Hide file tree
Showing 5 changed files with 414 additions and 88 deletions.
29 changes: 22 additions & 7 deletions dev/manual_tests/lib/card_collection.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:math';

import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart' show debugDumpRenderTree;

Expand Down Expand Up @@ -31,6 +33,11 @@ class CardCollectionState extends State<CardCollection> {

static const double kCardMargins = 8.0;
static const double kFixedCardHeight = 100.0;
static const List<double> _cardHeights = const <double>[
48.0, 63.0, 85.0, 146.0, 60.0, 55.0, 84.0, 96.0, 50.0,
48.0, 63.0, 85.0, 146.0, 60.0, 55.0, 84.0, 96.0, 50.0,
48.0, 63.0, 85.0, 146.0, 60.0, 55.0, 84.0, 96.0, 50.0,
];

MaterialColor _primaryColor = Colors.deepPurple;
List<CardModel> _cardModels;
Expand All @@ -41,15 +48,22 @@ class CardCollectionState extends State<CardCollection> {
bool _sunshine = false;
bool _varyFontSizes = false;

void _updateCardSizes() {
if (_fixedSizeCards)
return;
_cardModels = new List<CardModel>.generate(
_cardModels.length,
(int i) {
_cardModels[i].height = _editable ? max(_cardHeights[i], 60.0) : _cardHeights[i];
return _cardModels[i];
}
);
}

void _initVariableSizedCardModels() {
final List<double> cardHeights = <double>[
48.0, 63.0, 82.0, 146.0, 60.0, 55.0, 84.0, 96.0, 50.0,
48.0, 63.0, 82.0, 146.0, 60.0, 55.0, 84.0, 96.0, 50.0,
48.0, 63.0, 82.0, 146.0, 60.0, 55.0, 84.0, 96.0, 50.0,
];
_cardModels = new List<CardModel>.generate(
cardHeights.length,
(int i) => new CardModel(i, cardHeights[i])
_cardHeights.length,
(int i) => new CardModel(i, _editable ? max(_cardHeights[i], 60.0) : _cardHeights[i])
);
}

Expand Down Expand Up @@ -126,6 +140,7 @@ class CardCollectionState extends State<CardCollection> {
void _toggleEditable() {
setState(() {
_editable = !_editable;
_updateCardSizes();
});
}

Expand Down
127 changes: 74 additions & 53 deletions packages/flutter/lib/src/material/input_decorator.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,12 @@ class InputDecorator extends StatelessWidget {
/// The widget below this widget in the tree.
final Widget child;

static const double _kBottomBorderHeight = 1.0;
static const double _kDensePadding = 4.0;
static const double _kNormalPadding = 8.0;
static const double _kDenseTopPadding = 8.0;
static const double _kNormalTopPadding = 16.0;

@override
void debugFillProperties(DiagnosticPropertiesBuilder description) {
super.debugFillProperties(description);
Expand All @@ -392,32 +398,23 @@ class InputDecorator extends StatelessWidget {
return themeData.hintColor;
}

Widget _buildContent(Color borderColor, double topPadding, bool isDense, Widget inputChild) {
final double bottomPadding = isDense ? 8.0 : 1.0;
const double bottomBorder = 2.0;
final double bottomHeight = isDense ? 14.0 : 18.0;

final EdgeInsets padding = new EdgeInsets.only(top: topPadding, bottom: bottomPadding);
final EdgeInsets margin = new EdgeInsets.only(bottom: bottomHeight - (bottomPadding + bottomBorder));

Widget _buildContent(Color borderColor, double topPadding, bool isDense, Widget inputChild, double subTextHeight) {
if (decoration.hideDivider) {
return new Container(
margin: margin + const EdgeInsets.only(bottom: bottomBorder),
padding: padding,
padding: new EdgeInsets.only(top: topPadding, bottom: _kNormalPadding),
child: inputChild,
);
}

return new AnimatedContainer(
margin: margin,
padding: padding,
padding: new EdgeInsets.only(top: topPadding, bottom: _kNormalPadding - _kBottomBorderHeight),
duration: _kTransitionDuration,
curve: _kTransitionCurve,
decoration: new BoxDecoration(
border: new Border(
bottom: new BorderSide(
color: borderColor,
width: bottomBorder,
width: _kBottomBorderHeight,
),
),
),
Expand All @@ -429,6 +426,7 @@ class InputDecorator extends StatelessWidget {
Widget build(BuildContext context) {
assert(debugCheckHasMaterial(context));
final ThemeData themeData = Theme.of(context);
final double textScaleFactor = MediaQuery.of(context, nullOk: true)?.textScaleFactor ?? 1.0;

final bool isDense = decoration.isDense;
final bool isCollapsed = decoration.isCollapsed;
Expand All @@ -439,34 +437,37 @@ class InputDecorator extends StatelessWidget {
final String hintText = decoration.hintText;
final String errorText = decoration.errorText;

// If we're not focused, there's no value, and labelText was provided,
// then the label appears where the hint would. And we will not show
// the hintText.
final bool hasInlineLabel = !isFocused && labelText != null && isEmpty;

final Color activeColor = _getActiveColor(themeData);

final TextStyle baseStyle = this.baseStyle ?? themeData.textTheme.subhead;
final TextStyle hintStyle = decoration.hintStyle ?? baseStyle.copyWith(color: themeData.hintColor);
final TextStyle subtextStyle = errorText != null
? decoration.errorStyle ?? themeData.textTheme.caption.copyWith(color: themeData.errorColor)
: decoration.helperStyle ?? themeData.textTheme.caption.copyWith(color: themeData.hintColor);

final Color activeColor = _getActiveColor(themeData);
final double entryTextHeight = baseStyle.fontSize * textScaleFactor;
final double subTextHeight = subtextStyle.fontSize * textScaleFactor;

double topPadding = isCollapsed ? 0.0 : (isDense ? 12.0 : 16.0);
double topPadding = isCollapsed ? 0.0 : (isDense ? _kDenseTopPadding : _kNormalTopPadding);

final List<Widget> stackChildren = <Widget>[];

// If we're not focused, there's no value, and labelText was provided,
// then the label appears where the hint would. And we will not show
// the hintText.
final bool hasInlineLabel = !isFocused && labelText != null && isEmpty;

if (labelText != null) {
assert(!isCollapsed);
final TextStyle labelStyle = hasInlineLabel ?
hintStyle : (decoration.labelStyle ?? themeData.textTheme.caption.copyWith(color: activeColor));

final double topPaddingIncrement = themeData.textTheme.caption.fontSize + (isDense ? 4.0 : 8.0);
double top = topPadding;
if (hasInlineLabel)
top += topPaddingIncrement + baseStyle.fontSize - labelStyle.fontSize;
final TextStyle floatingLabelStyle = decoration.labelStyle ?? themeData.textTheme.caption.copyWith(color: activeColor);
final TextStyle labelStyle = hasInlineLabel ? hintStyle : floatingLabelStyle;
final double labelTextHeight = floatingLabelStyle.fontSize * textScaleFactor;

final double topPaddingIncrement = labelTextHeight + (isDense ? _kDensePadding : _kNormalPadding);
stackChildren.add(
new AnimatedPositionedDirectional(
start: 0.0,
top: top,
top: topPadding + (hasInlineLabel ? topPaddingIncrement : 0.0),
duration: _kTransitionDuration,
curve: _kTransitionCurve,
child: new _AnimatedLabel(
Expand All @@ -483,10 +484,12 @@ class InputDecorator extends StatelessWidget {

if (hintText != null) {
stackChildren.add(
new Positioned(
left: 0.0,
right: 0.0,
top: topPadding + baseStyle.fontSize - hintStyle.fontSize,
new AnimatedPositionedDirectional(
start: 0.0,
end: 0.0,
top: topPadding,
duration: _kTransitionDuration,
curve: _kTransitionCurve,
child: new AnimatedOpacity(
opacity: (isEmpty && !hasInlineLabel) ? 1.0 : 0.0,
duration: _kTransitionDuration,
Expand Down Expand Up @@ -534,33 +537,43 @@ class InputDecorator extends StatelessWidget {
inputChild = new Row(children: rowContents);
}

// The inputChild and the helper/error text need to be in a column so that if the inputChild is
// a multiline input or a non-text widget, it lays out with the helper/error text below the
// inputChild.
final List<Widget> columnChildren = <Widget>[];
if (isCollapsed) {
stackChildren.add(inputChild);
columnChildren.add(inputChild);
} else {
final Color borderColor = errorText == null ? activeColor : themeData.errorColor;
stackChildren.add(_buildContent(borderColor, topPadding, isDense, inputChild));
columnChildren.add(_buildContent(borderColor, topPadding, isDense, inputChild, subTextHeight));
}

if (!isDense && (errorText != null || helperText != null)) {
if (errorText != null || helperText != null) {
assert(!isCollapsed);
final TextStyle captionStyle = themeData.textTheme.caption;
final TextStyle subtextStyle = errorText != null
? decoration.errorStyle ?? captionStyle.copyWith(color: themeData.errorColor)
: decoration.helperStyle ?? captionStyle.copyWith(color: themeData.hintColor);

stackChildren.add(new Positioned(
left: 0.0,
right: 0.0,
bottom: 0.0,
child: new Text(
errorText ?? helperText,
style: subtextStyle,
textAlign: textAlign,
overflow: TextOverflow.ellipsis,
final double linePadding = _kBottomBorderHeight + (isDense ? _kDensePadding : _kNormalPadding);
columnChildren.add(
new AnimatedContainer(
padding: new EdgeInsets.only(top: linePadding),
duration: _kTransitionDuration,
curve: _kTransitionCurve,
child: new Text(
errorText ?? helperText,
style: subtextStyle,
textAlign: textAlign,
overflow: TextOverflow.ellipsis,
),
),
));
);
}

stackChildren.add(
new Column(
mainAxisSize: MainAxisSize.min,
crossAxisAlignment: CrossAxisAlignment.stretch,
children: columnChildren,
),
);

final Widget stack = new Stack(
fit: StackFit.passthrough,
children: stackChildren
Expand All @@ -569,17 +582,19 @@ class InputDecorator extends StatelessWidget {
if (decoration.icon != null) {
assert(!isCollapsed);
final double iconSize = isDense ? 18.0 : 24.0;
final double iconTop = topPadding + (baseStyle.fontSize - iconSize) / 2.0;
final double iconTop = topPadding + (entryTextHeight - iconSize) / 2.0;
return new Row(
crossAxisAlignment: CrossAxisAlignment.start,
children: <Widget>[
new Container(
new AnimatedContainer(
margin: new EdgeInsets.only(top: iconTop),
duration: _kTransitionDuration,
curve: _kTransitionCurve,
width: isDense ? 40.0 : 48.0,
child: IconTheme.merge(
data: new IconThemeData(
color: isFocused ? activeColor : Colors.black45,
size: isDense ? 18.0 : 24.0,
size: iconSize,
),
child: decoration.icon,
),
Expand All @@ -605,11 +620,15 @@ class _AnimatedLabel extends ImplicitlyAnimatedWidget {
@required this.style,
Curve curve: Curves.linear,
@required Duration duration,
this.textAlign,
this.overflow,
}) : assert(style != null),
super(key: key, curve: curve, duration: duration);

final String text;
final TextStyle style;
final TextAlign textAlign;
final TextOverflow overflow;

@override
_AnimatedLabelState createState() => new _AnimatedLabelState();
Expand Down Expand Up @@ -646,6 +665,8 @@ class _AnimatedLabelState extends AnimatedWidgetBaseState<_AnimatedLabel> {
child: new Text(
widget.text,
style: style,
textAlign: widget.textAlign,
overflow: widget.overflow,
),
);
}
Expand Down
1 change: 1 addition & 0 deletions packages/flutter/lib/src/painting/text_painter.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ class TextPainter {
return;
_textScaleFactor = value;
_paragraph = null;
_layoutTemplate = null;
_needsLayout = true;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/lib/src/rendering/object.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1964,7 +1964,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
// yet (that is, our layer might not have been detached yet), because the
// same node that skipped us in layout is above us in the tree (obviously)
// and therefore may not have had a chance to paint yet (since the tree
// paints in reverse order). In particular this will happen if they are have
// paints in reverse order). In particular this will happen if they have
// a different layer, because there's a repaint boundary between us.
if (_needsLayout)
return;
Expand Down
Loading

0 comments on commit 67cf791

Please sign in to comment.