From 7b5a769b3188e648ab22585a88d24ced290cd689 Mon Sep 17 00:00:00 2001 From: Gary Qian Date: Sat, 23 Feb 2019 03:35:37 -0800 Subject: [PATCH] Force line height in TextFields with strut (#27612) --- .../flutter/lib/src/cupertino/text_field.dart | 7 +- .../flutter/lib/src/material/text_field.dart | 5 + .../lib/src/material/text_form_field.dart | 2 + .../flutter/lib/src/painting/strut_style.dart | 87 +++++- .../flutter/lib/src/rendering/editable.dart | 12 + .../lib/src/widgets/editable_text.dart | 40 +++ .../test/cupertino/text_field_test.dart | 228 ++++++++++++++- .../test/material/input_decorator_test.dart | 33 +++ .../test/material/text_field_test.dart | 266 ++++++++++++++++++ .../widgets/editable_text_cursor_test.dart | 1 + .../test/widgets/text_golden_test.dart | 10 +- 11 files changed, 682 insertions(+), 9 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/text_field.dart b/packages/flutter/lib/src/cupertino/text_field.dart index 4e1f5d9a75..d17294c749 100644 --- a/packages/flutter/lib/src/cupertino/text_field.dart +++ b/packages/flutter/lib/src/cupertino/text_field.dart @@ -158,6 +158,7 @@ class CupertinoTextField extends StatefulWidget { this.textInputAction, this.textCapitalization = TextCapitalization.none, this.style, + this.strutStyle, this.textAlign = TextAlign.start, this.autofocus = false, this.obscureText = false, @@ -271,6 +272,9 @@ class CupertinoTextField extends StatefulWidget { /// Defaults to the standard iOS font style from [CupertinoTheme] if null. final TextStyle style; + /// {@macro flutter.widgets.editableText.strutStyle} + final StrutStyle strutStyle; + /// {@macro flutter.widgets.editableText.textAlign} final TextAlign textAlign; @@ -619,7 +623,7 @@ class _CupertinoTextFieldState extends State with AutomaticK formatters.add(LengthLimitingTextInputFormatter(widget.maxLength)); } final CupertinoThemeData themeData = CupertinoTheme.of(context); - final TextStyle textStyle = widget.style ?? themeData.textTheme.textStyle; + final TextStyle textStyle = themeData.textTheme.textStyle.merge(widget.style); final Brightness keyboardAppearance = widget.keyboardAppearance ?? themeData.brightness; final Widget paddedEditable = Padding( @@ -633,6 +637,7 @@ class _CupertinoTextFieldState extends State with AutomaticK textInputAction: widget.textInputAction, textCapitalization: widget.textCapitalization, style: textStyle, + strutStyle: widget.strutStyle, textAlign: widget.textAlign, autofocus: widget.autofocus, obscureText: widget.obscureText, diff --git a/packages/flutter/lib/src/material/text_field.dart b/packages/flutter/lib/src/material/text_field.dart index 52238f9be0..c079574876 100644 --- a/packages/flutter/lib/src/material/text_field.dart +++ b/packages/flutter/lib/src/material/text_field.dart @@ -124,6 +124,7 @@ class TextField extends StatefulWidget { this.textInputAction, this.textCapitalization = TextCapitalization.none, this.style, + this.strutStyle, this.textAlign = TextAlign.start, this.textDirection, this.autofocus = false, @@ -231,6 +232,9 @@ class TextField extends StatefulWidget { /// If null, defaults to the `subhead` text style from the current [Theme]. final TextStyle style; + /// {@macro flutter.widgets.editableText.strutStyle} + final StrutStyle strutStyle; + /// {@macro flutter.widgets.editableText.textAlign} final TextAlign textAlign; @@ -763,6 +767,7 @@ class _TextFieldState extends State with AutomaticKeepAliveClientMixi textInputAction: widget.textInputAction, textCapitalization: widget.textCapitalization, style: style, + strutStyle: widget.strutStyle, textAlign: widget.textAlign, textDirection: widget.textDirection, autofocus: widget.autofocus, diff --git a/packages/flutter/lib/src/material/text_form_field.dart b/packages/flutter/lib/src/material/text_form_field.dart index ebc246e8ae..3db75f9f48 100644 --- a/packages/flutter/lib/src/material/text_form_field.dart +++ b/packages/flutter/lib/src/material/text_form_field.dart @@ -80,6 +80,7 @@ class TextFormField extends FormField { TextCapitalization textCapitalization = TextCapitalization.none, TextInputAction textInputAction, TextStyle style, + StrutStyle strutStyle, TextDirection textDirection, TextAlign textAlign = TextAlign.start, bool autofocus = false, @@ -131,6 +132,7 @@ class TextFormField extends FormField { keyboardType: keyboardType, textInputAction: textInputAction, style: style, + strutStyle: strutStyle, textAlign: textAlign, textDirection: textDirection, textCapitalization: textCapitalization, diff --git a/packages/flutter/lib/src/painting/strut_style.dart b/packages/flutter/lib/src/painting/strut_style.dart index a6ff94482b..a1e1fbb3d5 100644 --- a/packages/flutter/lib/src/painting/strut_style.dart +++ b/packages/flutter/lib/src/painting/strut_style.dart @@ -5,6 +5,7 @@ import 'package:flutter/foundation.dart'; import 'basic_types.dart'; +import 'text_style.dart'; /// Defines the strut, which sets the minimum height a line can be /// relative to the baseline. Strut applies to all lines in the pararaph. @@ -12,7 +13,8 @@ import 'basic_types.dart'; /// Strut is a feature that allows minimum line heights to be set. The effect is as /// if a zero width space was included at the beginning of each line in the /// paragraph. This imaginary space is 'shaped' according the properties defined -/// in this class. +/// in this class. Flutter's strut is based on [typesetting strut](https://en.wikipedia.org/wiki/Strut_(typesetting)) +/// and CSS's [line-height](https://www.w3.org/TR/CSS2/visudet.html#line-height). /// /// No lines may be shorter than the strut. The ascent and descent of the strut /// are calculated, and any laid out text that has a shorter ascent or descent than @@ -263,6 +265,8 @@ class StrutStyle extends Diagnosticable { /// The `package` argument must be non-null if the font family is defined in a /// package. It is combined with the `fontFamily` argument to set the /// [fontFamily] property. + /// + /// If provided, fontSize must be positive and non-zero, leading must be zero or positive. const StrutStyle({ String fontFamily, List fontFamilyFallback, @@ -281,6 +285,56 @@ class StrutStyle extends Diagnosticable { assert(leading == null || leading >= 0), assert(package == null || (package != null && (fontFamily != null || fontFamilyFallback != null))); + /// Builds a StrutStyle that contains values of the equivalent properties in + /// the provided [textStyle]. + /// + /// The [textStyle] parameter must not be null. + /// + /// The named parameters override the [textStyle]'s argument's properties. + /// Since TextStyle does not contain [leading] or [forceStrutHeight], these values + /// will take on default values (null and false) unless otherwise specified. + /// + /// If provided, fontSize must be positive and non-zero, leading must be zero or positive. + /// + /// When [textStyle] has a package and a new [package] is also specified, the entire + /// font family fallback list should be redefined since the [textStyle]'s package data + /// is inherited by being prepended onto the font family names. If + /// [fontFamilyFallback] is meant to be empty, pass an empty list instead of null. + /// This prevents the previous package name from being prepended twice. + StrutStyle.fromTextStyle(TextStyle textStyle, { + String fontFamily, + List fontFamilyFallback, + double fontSize, + double height, + this.leading, // TextStyle does not have an equivalent (yet). + FontWeight fontWeight, + FontStyle fontStyle, + this.forceStrutHeight, + String debugLabel, + String package, + }) : assert(textStyle != null), + assert(fontSize == null || fontSize > 0), + assert(leading == null || leading >= 0), + assert(package == null || (package != null && (fontFamily != null || fontFamilyFallback != null))), + fontFamily = fontFamily != null ? (package == null ? fontFamily : 'packages/$package/$fontFamily') : textStyle.fontFamily, + _fontFamilyFallback = fontFamilyFallback ?? textStyle.fontFamilyFallback, + height = height ?? textStyle.height, + fontSize = fontSize ?? textStyle.fontSize, + fontWeight = fontWeight ?? textStyle.fontWeight, + fontStyle = fontStyle ?? textStyle.fontStyle, + debugLabel = debugLabel ?? textStyle.debugLabel, + _package = package; // the textStyle._package data is embedded in the fontFamily names, + // so we no longer need it. + + /// A [StrutStyle] that will have no impact on the text layout. + /// + /// Equivalent to having no strut at all. All lines will be laid out according to + /// the properties defined in [TextStyle]. + static const StrutStyle disabled = StrutStyle( + height: 0, + leading: 0, + ); + /// The name of the font to use when calcualting the strut (e.g., Roboto). If the /// font is defined in a package, this will be prefixed with /// 'packages/package_name/' (e.g. 'packages/cool_fonts/Roboto'). The @@ -413,6 +467,32 @@ class StrutStyle extends Diagnosticable { return RenderComparison.identical; } + /// Returns a new strut style that inherits its null values from corresponding + /// properties in the [other] [TextStyle]. + /// + /// The "missing" properties of the this strut style are _filled_ by the properties + /// of the provided [TextStyle]. This is possible because [StrutStyle] shares many of + /// the same basic properties as [TextStyle]. + /// + /// If the given text style is null, returns this strut style. + StrutStyle inheritFromTextStyle(TextStyle other) { + if (other == null) + return this; + + return StrutStyle( + fontFamily: fontFamily ?? other.fontFamily, + fontFamilyFallback: fontFamilyFallback ?? other.fontFamilyFallback, + fontSize: fontSize ?? other.fontSize, + height: height ?? other.height, + leading: leading, // No equivalent property in TextStyle yet. + fontWeight: fontWeight ?? other.fontWeight, + fontStyle: fontStyle ?? other.fontStyle, + forceStrutHeight: forceStrutHeight, // StrutStyle-unique property. + debugLabel: debugLabel ?? other.debugLabel, + // Package is embedded within the getters for fontFamilyFallback. + ); + } + @override bool operator ==(dynamic other) { if (identical(this, other)) @@ -442,6 +522,9 @@ class StrutStyle extends Diagnosticable { ); } + @override + String toStringShort() => '$runtimeType'; + /// Adds all properties prefixing property names with the optional `prefix`. @override void debugFillProperties(DiagnosticPropertiesBuilder properties, { String prefix = '' }) { @@ -454,7 +537,7 @@ class StrutStyle extends Diagnosticable { styles.add(DoubleProperty('${prefix}size', fontSize, defaultValue: null)); String weightDescription; if (fontWeight != null) { - weightDescription = '${fontWeight.index + 1}00'; + weightDescription = 'w${fontWeight.index + 1}00'; } // TODO(jacobr): switch this to use enumProperty which will either cause the // weight description to change to w600 from 600 or require existing diff --git a/packages/flutter/lib/src/rendering/editable.dart b/packages/flutter/lib/src/rendering/editable.dart index 3c7bc8ea8d..1b541e3ceb 100644 --- a/packages/flutter/lib/src/rendering/editable.dart +++ b/packages/flutter/lib/src/rendering/editable.dart @@ -140,6 +140,7 @@ class RenderEditable extends RenderBox { ValueNotifier showCursor, bool hasFocus, int maxLines = 1, + StrutStyle strutStyle, Color selectionColor, double textScaleFactor = 1.0, TextSelection selection, @@ -174,6 +175,7 @@ class RenderEditable extends RenderBox { textDirection: textDirection, textScaleFactor: textScaleFactor, locale: locale, + strutStyle: strutStyle, ), _cursorColor = cursorColor, _backgroundCursorColor = backgroundCursorColor, @@ -593,6 +595,16 @@ class RenderEditable extends RenderBox { markNeedsTextLayout(); } + /// The [StrutStyle] used by the renderer's internal [TextPainter] to + /// determine the strut to use. + StrutStyle get strutStyle => _textPainter.strutStyle; + set strutStyle(StrutStyle value) { + if (_textPainter.strutStyle == value) + return; + _textPainter.strutStyle = value; + markNeedsTextLayout(); + } + /// The color to use when painting the cursor. Color get cursorColor => _cursorColor; Color _cursorColor; diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 2e5aa10832..e5a792e563 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:ui' as ui; import 'package:flutter/foundation.dart'; +import 'package:flutter/painting.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/scheduler.dart'; import 'package:flutter/services.dart'; @@ -203,6 +204,7 @@ class EditableText extends StatefulWidget { this.obscureText = false, this.autocorrect = true, @required this.style, + StrutStyle strutStyle, @required this.cursorColor, @required this.backgroundCursorColor, this.textAlign = TextAlign.start, @@ -246,6 +248,7 @@ class EditableText extends StatefulWidget { assert(rendererIgnoresPointer != null), assert(scrollPadding != null), assert(dragStartBehavior != null), + _strutStyle = strutStyle, keyboardType = keyboardType ?? (maxLines == 1 ? TextInputType.text : TextInputType.multiline), inputFormatters = maxLines == 1 ? ( @@ -281,6 +284,38 @@ class EditableText extends StatefulWidget { /// The text style to use for the editable text. final TextStyle style; + /// {@template flutter.widgets.editableText.strutStyle} + /// The strut style used for the vertical layout. + /// + /// [StrutStyle] is used to establish a predictable vertical layout. + /// Since fonts may vary depending on user input and due to font + /// fallback, [StrutStyle.forceStrutHeight] is enabled by default + /// to lock all lines to the height of the base [TextStyle], provided by + /// [style]. This ensures the typed text fits within the alotted space. + /// + /// If null, the strut used will is inherit values from the [style] and will + /// have [StrutStyle.forceStrutHeight] set to true. When no [style] is + /// passed, the theme's [TextStyle] will be used to generate [strutStyle] + /// instead. + /// + /// To disable strut-based vertical alignment and allow dynamic vertical + /// layout based on the glyphs typed, use [StrutStyle.disabled]. + /// + /// Flutter's strut is based on [typesetting strut](https://en.wikipedia.org/wiki/Strut_(typesetting)) + /// and CSS's [line-height](https://www.w3.org/TR/CSS2/visudet.html#line-height). + /// {@endtemplate} + /// + /// Within editable text and textfields, [StrutStyle] will not use its standalone + /// default values, and will instead inherit omitted/null properties from the + /// [TextStyle] instead. See [StrutStyle.inheritFromTextStyle]. + StrutStyle get strutStyle { + if (_strutStyle == null) { + return style != null ? StrutStyle.fromTextStyle(style, forceStrutHeight: true) : StrutStyle.disabled; + } + return _strutStyle.inheritFromTextStyle(style); + } + final StrutStyle _strutStyle; + /// {@template flutter.widgets.editableText.textAlign} /// How the text should be aligned horizontally. /// @@ -1219,6 +1254,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien : _cursorVisibilityNotifier, hasFocus: _hasFocus, maxLines: widget.maxLines, + strutStyle: widget.strutStyle, selectionColor: widget.selectionColor, textScaleFactor: widget.textScaleFactor ?? MediaQuery.textScaleFactorOf(context), textAlign: widget.textAlign, @@ -1288,6 +1324,7 @@ class _Editable extends LeafRenderObjectWidget { this.showCursor, this.hasFocus, this.maxLines, + this.strutStyle, this.selectionColor, this.textScaleFactor, this.textAlign, @@ -1317,6 +1354,7 @@ class _Editable extends LeafRenderObjectWidget { final ValueNotifier showCursor; final bool hasFocus; final int maxLines; + final StrutStyle strutStyle; final Color selectionColor; final double textScaleFactor; final TextAlign textAlign; @@ -1345,6 +1383,7 @@ class _Editable extends LeafRenderObjectWidget { showCursor: showCursor, hasFocus: hasFocus, maxLines: maxLines, + strutStyle: strutStyle, selectionColor: selectionColor, textScaleFactor: textScaleFactor, textAlign: textAlign, @@ -1374,6 +1413,7 @@ class _Editable extends LeafRenderObjectWidget { ..showCursor = showCursor ..hasFocus = hasFocus ..maxLines = maxLines + ..strutStyle = strutStyle ..selectionColor = selectionColor ..textScaleFactor = textScaleFactor ..textAlign = textAlign diff --git a/packages/flutter/test/cupertino/text_field_test.dart b/packages/flutter/test/cupertino/text_field_test.dart index 1d66b13568..bd77931837 100644 --- a/packages/flutter/test/cupertino/text_field_test.dart +++ b/packages/flutter/test/cupertino/text_field_test.dart @@ -30,6 +30,27 @@ void main() { final MockClipboard mockClipboard = MockClipboard(); SystemChannels.platform.setMockMethodCallHandler(mockClipboard.handleMethodCall); + testWidgets( + 'takes available space horizontally and takes intrinsic space vertically no-strut', + (WidgetTester tester) async { + await tester.pumpWidget( + CupertinoApp( + home: Center( + child: ConstrainedBox( + constraints: BoxConstraints.loose(const Size(200, 200)), + child: const CupertinoTextField(strutStyle: StrutStyle.disabled), + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(CupertinoTextField)), + const Size(200, 29), // 29 is the height of the default font + padding etc. + ); + }, + ); + testWidgets( 'takes available space horizontally and takes intrinsic space vertically', (WidgetTester tester) async { @@ -46,7 +67,31 @@ void main() { expect( tester.getSize(find.byType(CupertinoTextField)), - const Size(200, 29), // 29 is the height of the default font + padding etc. + const Size(200, 29), // 29 is the height of the default font (17) + decoration (12). + ); + }, + ); + + testWidgets( + 'multi-lined text fields are intrinsically taller no-strut', + (WidgetTester tester) async { + await tester.pumpWidget( + CupertinoApp( + home: Center( + child: ConstrainedBox( + constraints: BoxConstraints.loose(const Size(200, 200)), + child: const CupertinoTextField( + maxLines: 3, + strutStyle: StrutStyle.disabled, + ), + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(CupertinoTextField)), + const Size(200, 63), // 63 is the height of the default font (17) * maxlines (3) + decoration height (12). ); }, ); @@ -72,6 +117,61 @@ void main() { }, ); + testWidgets( + 'strut height override', + (WidgetTester tester) async { + await tester.pumpWidget( + CupertinoApp( + home: Center( + child: ConstrainedBox( + constraints: BoxConstraints.loose(const Size(200, 200)), + child: const CupertinoTextField( + maxLines: 3, + strutStyle: StrutStyle( + fontSize: 8, + forceStrutHeight: true, + ), + ), + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(CupertinoTextField)), + const Size(200, 36), + ); + }, + ); + + testWidgets( + 'strut forces field taller', + (WidgetTester tester) async { + await tester.pumpWidget( + CupertinoApp( + home: Center( + child: ConstrainedBox( + constraints: BoxConstraints.loose(const Size(200, 200)), + child: const CupertinoTextField( + maxLines: 3, + style: TextStyle(fontSize: 10), + strutStyle: StrutStyle( + fontSize: 18, + forceStrutHeight: true, + ), + ), + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(CupertinoTextField)), + const Size(200, 66), + ); + }, + ); + testWidgets( 'default text field has a border', (WidgetTester tester) async { @@ -395,6 +495,64 @@ void main() { }, ); + testWidgets( + 'padding is in between prefix and suffix no-strut', + (WidgetTester tester) async { + await tester.pumpWidget( + const CupertinoApp( + home: Center( + child: CupertinoTextField( + padding: EdgeInsets.all(20.0), + prefix: SizedBox(height: 100.0, width: 100.0), + suffix: SizedBox(height: 50.0, width: 50.0), + strutStyle: StrutStyle.disabled, + ), + ), + ), + ); + + expect( + tester.getTopLeft(find.byType(EditableText)).dx, + // Size of prefix + padding. + 100.0 + 20.0, + ); + + expect(tester.getTopLeft(find.byType(EditableText)).dy, 291.5); + + expect( + tester.getTopRight(find.byType(EditableText)).dx, + 800.0 - 50.0 - 20.0, + ); + + await tester.pumpWidget( + const CupertinoApp( + home: Center( + child: CupertinoTextField( + padding: EdgeInsets.all(30.0), + prefix: SizedBox(height: 100.0, width: 100.0), + suffix: SizedBox(height: 50.0, width: 50.0), + strutStyle: StrutStyle.disabled, + ), + ), + ), + ); + + expect( + tester.getTopLeft(find.byType(EditableText)).dx, + 100.0 + 30.0, + ); + + // Since the highest component, the prefix box, is higher than + // the text + paddings, the text's vertical position isn't affected. + expect(tester.getTopLeft(find.byType(EditableText)).dy, 291.5); + + expect( + tester.getTopRight(find.byType(EditableText)).dx, + 800.0 - 50.0 - 30.0, + ); + }, + ); + testWidgets( 'padding is in between prefix and suffix', (WidgetTester tester) async { @@ -590,6 +748,45 @@ void main() { }, ); + testWidgets( + 'font style controls intrinsic height no-strut', + (WidgetTester tester) async { + await tester.pumpWidget( + const CupertinoApp( + home: Center( + child: CupertinoTextField( + strutStyle: StrutStyle.disabled, + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(CupertinoTextField)).height, + 29.0, + ); + + await tester.pumpWidget( + const CupertinoApp( + home: Center( + child: CupertinoTextField( + style: TextStyle( + // A larger font. + fontSize: 50.0, + ), + strutStyle: StrutStyle.disabled, + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(CupertinoTextField)).height, + 62.0, + ); + }, + ); + testWidgets( 'font style controls intrinsic height', (WidgetTester tester) async { @@ -656,6 +853,35 @@ void main() { }, ); + testWidgets( + 'text fields with no max lines can grow no-strut', + (WidgetTester tester) async { + await tester.pumpWidget( + const CupertinoApp( + home: Center( + child: CupertinoTextField( + maxLines: null, + strutStyle: StrutStyle.disabled, + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(CupertinoTextField)).height, + 29.0, // Initially one line high. + ); + + await tester.enterText(find.byType(CupertinoTextField), '\n'); + await tester.pump(); + + expect( + tester.getSize(find.byType(CupertinoTextField)).height, + 46.0, // Initially one line high. + ); + }, + ); + testWidgets( 'text fields with no max lines can grow', (WidgetTester tester) async { diff --git a/packages/flutter/test/material/input_decorator_test.dart b/packages/flutter/test/material/input_decorator_test.dart index 827e68cac3..b6671aee6d 100644 --- a/packages/flutter/test/material/input_decorator_test.dart +++ b/packages/flutter/test/material/input_decorator_test.dart @@ -268,6 +268,39 @@ void main() { expect(tester.getBottomLeft(find.text('label')).dy, tester.getBottomLeft(find.text('hint')).dy); }); + testWidgets('InputDecorator alignLabelWithHint for multiline TextField no-strut', (WidgetTester tester) async { + Widget buildFrame(bool alignLabelWithHint) { + return MaterialApp( + home: Material( + child: Directionality( + textDirection: TextDirection.ltr, + child: TextField( + maxLines: 8, + decoration: InputDecoration( + labelText: 'label', + alignLabelWithHint: alignLabelWithHint, + hintText: 'hint', + ), + strutStyle: StrutStyle.disabled, + ), + ), + ), + ); + } + + // alignLabelWithHint: false centers the label in the TextField + await tester.pumpWidget(buildFrame(false)); + await tester.pumpAndSettle(); + expect(tester.getTopLeft(find.text('label')).dy, 76.0); + expect(tester.getBottomLeft(find.text('label')).dy, 92.0); + + // alignLabelWithHint: true aligns the label with the hint. + await tester.pumpWidget(buildFrame(true)); + await tester.pumpAndSettle(); + expect(tester.getTopLeft(find.text('label')).dy, tester.getTopLeft(find.text('hint')).dy); + expect(tester.getBottomLeft(find.text('label')).dy, tester.getBottomLeft(find.text('hint')).dy); + }); + testWidgets('InputDecorator alignLabelWithHint for multiline TextField', (WidgetTester tester) async { Widget buildFrame(bool alignLabelWithHint) { return MaterialApp( diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index 9f49c7a2de..c9f436b7af 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -900,6 +900,7 @@ void main() { controller: controller, style: const TextStyle(color: Colors.black, fontSize: 34.0), maxLines: 3, + strutStyle: StrutStyle.disabled, ), ), ); @@ -1510,6 +1511,7 @@ void main() { decoration: InputDecoration.collapsed( hintText: 'hint', ), + strutStyle: StrutStyle.disabled, ), ), ); @@ -2132,6 +2134,7 @@ void main() { child: TextField( controller: controller, maxLines: 3, + strutStyle: StrutStyle.disabled, ), ) , ), @@ -2672,6 +2675,59 @@ void main() { expect(controller.selection.baseOffset, 0); }); + testWidgets('TextField baseline alignment no-strut', (WidgetTester tester) async { + final TextEditingController controllerA = TextEditingController(text: 'A'); + final TextEditingController controllerB = TextEditingController(text: 'B'); + final Key keyA = UniqueKey(); + final Key keyB = UniqueKey(); + + await tester.pumpWidget( + overlay( + child: Row( + crossAxisAlignment: CrossAxisAlignment.baseline, + textBaseline: TextBaseline.alphabetic, + children: [ + Expanded( + child: TextField( + key: keyA, + decoration: null, + controller: controllerA, + style: const TextStyle(fontSize: 10.0), + strutStyle: StrutStyle.disabled, + ) + ), + const Text( + 'abc', + style: TextStyle(fontSize: 20.0), + ), + Expanded( + child: TextField( + key: keyB, + decoration: null, + controller: controllerB, + style: const TextStyle(fontSize: 30.0), + strutStyle: StrutStyle.disabled, + ), + ), + ], + ), + ), + ); + + // The Ahem font extends 0.2 * fontSize below the baseline. + // So the three row elements line up like this: + // + // A abc B + // --------- baseline + // 2 4 6 space below the baseline = 0.2 * fontSize + // --------- rowBottomY + + final double rowBottomY = tester.getBottomLeft(find.byType(Row)).dy; + expect(tester.getBottomLeft(find.byKey(keyA)).dy, closeTo(rowBottomY - 4.0, 0.001)); + expect(tester.getBottomLeft(find.text('abc')).dy, closeTo(rowBottomY - 2.0, 0.001)); + expect(tester.getBottomLeft(find.byKey(keyB)).dy, rowBottomY); + }); + testWidgets('TextField baseline alignment', (WidgetTester tester) async { final TextEditingController controllerA = TextEditingController(text: 'A'); final TextEditingController controllerB = TextEditingController(text: 'B'); @@ -2718,6 +2774,7 @@ void main() { // --------- rowBottomY final double rowBottomY = tester.getBottomLeft(find.byType(Row)).dy; + // The values here should match the version with strut disabled ('TextField baseline alignment no-strut') expect(tester.getBottomLeft(find.byKey(keyA)).dy, closeTo(rowBottomY - 4.0, 0.001)); expect(tester.getBottomLeft(find.text('abc')).dy, closeTo(rowBottomY - 2.0, 0.001)); expect(tester.getBottomLeft(find.byKey(keyB)).dy, rowBottomY); @@ -4485,6 +4542,215 @@ void main() { ]); }); + testWidgets( + 'strut basic single line', + (WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + theme: ThemeData(platform: TargetPlatform.android), + home: const Material( + child: Center( + child: TextField(), + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(TextField)), + // This is the height of the decoration (24) plus the metrics from the default + // TextStyle of the theme (16). + const Size(800, 40), + ); + }, + ); + + testWidgets( + 'strut TextStyle increases height', + (WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + theme: ThemeData(platform: TargetPlatform.android), + home: const Material( + child: Center( + child: TextField( + style: TextStyle(fontSize: 20), + ), + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(TextField)), + // Strut should inherit the TextStyle.fontSize by default and produce the + // same height as if it were disabled. + const Size(800, 44), + ); + + await tester.pumpWidget( + MaterialApp( + theme: ThemeData(platform: TargetPlatform.android), + home: const Material( + child: Center( + child: TextField( + style: TextStyle(fontSize: 20), + strutStyle: StrutStyle.disabled, + ), + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(TextField)), + // The height here should match the previous version with strut enabled. + const Size(800, 44), + ); + }, + ); + + testWidgets( + 'strut basic multi line', + (WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + theme: ThemeData(platform: TargetPlatform.android), + home: const Material( + child: Center( + child: TextField( + maxLines: 6, + ), + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(TextField)), + // The height should be the input decoration (24) plus 6x the strut height (16). + const Size(800, 120), + ); + }, + ); + + testWidgets( + 'strut no force small strut', + (WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + theme: ThemeData(platform: TargetPlatform.android), + home: const Material( + child: Center( + child: TextField( + maxLines: 6, + strutStyle: StrutStyle( + // The small strut is overtaken by the larger + // TextStyle fontSize. + fontSize: 5, + ), + ), + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(TextField)), + // When the strut's height is smaller than TextStyle's and forceStrutHeight + // is disabled, then the TextStyle takes precedence. Should be the same height + // as 'strut basic multi line'. + const Size(800, 120), + ); + }, + ); + + testWidgets( + 'strut no force large strut', + (WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + theme: ThemeData(platform: TargetPlatform.android), + home: const Material( + child: Center( + child: TextField( + maxLines: 6, + strutStyle: StrutStyle( + fontSize: 25, + ), + ), + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(TextField)), + // When the strut's height is larger than TextStyle's and forceStrutHeight + // is disabled, then the StrutStyle takes precedence. + const Size(800, 174), + ); + }, + ); + + testWidgets( + 'strut height override', + (WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + theme: ThemeData(platform: TargetPlatform.android), + home: const Material( + child: Center( + child: TextField( + maxLines: 3, + strutStyle: StrutStyle( + fontSize: 8, + forceStrutHeight: true, + ), + ), + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(TextField)), + // The smaller font size of strut make the field shorter than normal. + const Size(800, 48), + ); + }, + ); + + testWidgets( + 'strut forces field taller', + (WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + theme: ThemeData(platform: TargetPlatform.android), + home: const Material( + child: Center( + child: TextField( + maxLines: 3, + style: TextStyle(fontSize: 10), + strutStyle: StrutStyle( + fontSize: 18, + forceStrutHeight: true, + ), + ), + ), + ), + ), + ); + + expect( + tester.getSize(find.byType(TextField)), + // When the strut fontSize is larger than a provided TextStyle, the + // the strut's height takes precedence. + const Size(800, 78), + ); + }, + ); + testWidgets('Caret center position', (WidgetTester tester) async { await tester.pumpWidget( overlay( diff --git a/packages/flutter/test/widgets/editable_text_cursor_test.dart b/packages/flutter/test/widgets/editable_text_cursor_test.dart index 946d6d2c38..357861900a 100644 --- a/packages/flutter/test/widgets/editable_text_cursor_test.dart +++ b/packages/flutter/test/widgets/editable_text_cursor_test.dart @@ -529,6 +529,7 @@ void main() { controller: controller, focusNode: focusNode, style: textStyle, + strutStyle: StrutStyle.disabled, ), ), ), diff --git a/packages/flutter/test/widgets/text_golden_test.dart b/packages/flutter/test/widgets/text_golden_test.dart index 4347cfa4cc..aee652de0b 100644 --- a/packages/flutter/test/widgets/text_golden_test.dart +++ b/packages/flutter/test/widgets/text_golden_test.dart @@ -270,7 +270,7 @@ void main() { ); await expectLater( find.byType(Container), - matchesGoldenFile('text_golden.Strut.1.png'), + matchesGoldenFile('text_golden.Strut.1.1.png'), ); }, skip: true); // Should only be on linux (skip: !Platform.isLinux). // Disabled for now until font inconsistency is resolved. @@ -299,7 +299,7 @@ void main() { ); await expectLater( find.byType(Container), - matchesGoldenFile('text_golden.Strut.2.png'), + matchesGoldenFile('text_golden.Strut.2.1.png'), ); }, skip: true); // Should only be on linux (skip: !Platform.isLinux). // Disabled for now until font inconsistency is resolved. @@ -351,7 +351,7 @@ void main() { ); await expectLater( find.byType(Container), - matchesGoldenFile('text_golden.Strut.3.png'), + matchesGoldenFile('text_golden.Strut.3.1.png'), ); }, skip: true); // Should only be on linux (skip: !Platform.isLinux). // Disabled for now until font inconsistency is resolved. @@ -387,7 +387,7 @@ void main() { ); await expectLater( find.byType(Container), - matchesGoldenFile('text_golden.Strut.4.png'), + matchesGoldenFile('text_golden.Strut.4.1.png'), ); }, skip: true); // Should only be on linux (skip: !Platform.isLinux). // Disabled for now until font inconsistency is resolved. @@ -439,7 +439,7 @@ void main() { ); await expectLater( find.byType(Container), - matchesGoldenFile('text_golden.StrutForce.1.png'), + matchesGoldenFile('text_golden.StrutForce.1.1.png'), ); }, skip: true); // Should only be on linux (skip: !Platform.isLinux). // Disabled for now until font inconsistency is resolved.