Merge pull request #1347 from abarth/editable_line

Input widget shrinks when typing a space

packages/flutter/lib/rendering.dart

@@ -12,7 +12,7 @@ export 'src/rendering/block.dart';
 export 'src/rendering/box.dart';
 export 'src/rendering/custom_layout.dart';
 export 'src/rendering/debug.dart';
-export 'src/rendering/editable_paragraph.dart';
+export 'src/rendering/editable_line.dart';
 export 'src/rendering/error.dart';
 export 'src/rendering/flex.dart';
 export 'src/rendering/grid.dart';

packages/flutter/lib/src/material/input.dart

@@ -61,18 +61,18 @@ class Input extends Scrollable {
 
 class InputState extends ScrollableState<Input> {
   String _value;
-  EditableString _editableValue;
+  EditableString _editableString;
   KeyboardHandle _keyboardHandle = KeyboardHandle.unattached;
 
   double _contentWidth = 0.0;
   double _containerWidth = 0.0;
 
-  EditableString get editableValue => _editableValue;
+  EditableString get editableValue => _editableString;
 
   void initState() {
     super.initState();
     _value = config.initialValue;
-    _editableValue = new EditableString(
+    _editableString = new EditableString(
       text: _value,
       onUpdated: _handleTextUpdated,
       onSubmitted: _handleTextSubmitted
@@ -80,9 +80,9 @@ class InputState extends ScrollableState<Input> {
   }
 
   void _handleTextUpdated() {
-    if (_value != _editableValue.text) {
+    if (_value != _editableString.text) {
       setState(() {
-        _value = _editableValue.text;
+        _value = _editableString.text;
       });
       if (config.onChanged != null)
         config.onChanged(_value);
@@ -100,10 +100,10 @@ class InputState extends ScrollableState<Input> {
     bool focused = Focus.at(context, autofocus: config.autofocus);
 
     if (focused && !_keyboardHandle.attached) {
-      _keyboardHandle = keyboard.show(_editableValue.stub, config.keyboardType);
-      _keyboardHandle.setText(_editableValue.text);
-      _keyboardHandle.setSelection(_editableValue.selection.start,
-                                   _editableValue.selection.end);
+      _keyboardHandle = keyboard.show(_editableString.stub, config.keyboardType);
+      _keyboardHandle.setText(_editableString.text);
+      _keyboardHandle.setSelection(_editableString.selection.start,
+                                   _editableString.selection.end);
     } else if (!focused && _keyboardHandle.attached) {
       _keyboardHandle.release();
     }
@@ -127,8 +127,8 @@ class InputState extends ScrollableState<Input> {
       focusHighlightColor = focused ? themeData.primarySwatch[400] : themeData.hintColor;
     }
 
-    textChildren.add(new EditableText(
-      value: _editableValue,
+    textChildren.add(new RawEditableLine(
+      value: _editableString,
       focused: focused,
       style: textStyle,
       hideText: config.hideText,

packages/flutter/lib/src/rendering/editable_paragraph.dart → packages/flutter/lib/src/rendering/editable_line.dart

@@ -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:ui' as ui;
+
 import 'package:flutter/painting.dart';
 
 import 'box.dart';
@@ -13,24 +15,43 @@ const _kCursorGap = 1.0; // pixels
 const _kCursorHeightOffset = 2.0; // pixels
 const _kCursorWidth = 1.0; // pixels
 
-/// A render object used by EditableText widgets.  This is similar to
-/// RenderParagraph but also renders a cursor and provides support for
-/// scrolling.
-class RenderEditableParagraph extends RenderParagraph {
+final String _kZeroWidthSpace = new String.fromCharCode(0x200B);
 
-  RenderEditableParagraph({
-    TextSpan text,
+/// A single line of editable text.
+class RenderEditableLine extends RenderBox {
+  RenderEditableLine({
+    StyledTextSpan text,
     Color cursorColor,
     bool showCursor,
     this.onContentSizeChanged,
     Offset scrollOffset
-  }) : _cursorColor = cursorColor,
+  }) : _textPainter = new TextPainter(text),
+       _cursorColor = cursorColor,
        _showCursor = showCursor,
-       _scrollOffset = scrollOffset,
-       super(text);
+       _scrollOffset = scrollOffset {
+  // TODO(abarth): These min/max values should be the default for TextPainter.
+  _textPainter
+    ..minWidth = 0.0
+    ..maxWidth = double.INFINITY
+    ..minHeight = 0.0
+    ..maxHeight = double.INFINITY;
+  }
 
   SizeChangedCallback onContentSizeChanged;
-  Size _contentSize;
+
+  /// The text to display
+  StyledTextSpan get text => _textPainter.text;
+  final TextPainter _textPainter;
+  void set text(StyledTextSpan value) {
+    if (_textPainter.text == value)
+      return;
+    StyledTextSpan oldStyledText = _textPainter.text;
+    if (oldStyledText.style != value.style)
+      _layoutTemplate = null;
+    _textPainter.text = value;
+    _constraintsForCurrentLayout = null;
+    markNeedsLayout();
+  }
 
   Color get cursorColor => _cursorColor;
   Color _cursorColor;
@@ -59,39 +80,74 @@ class RenderEditableParagraph extends RenderParagraph {
     markNeedsPaint();
   }
 
-  BoxConstraints _getTextContraints(BoxConstraints constraints) {
-    assert(constraints.isNormalized);
-    return new BoxConstraints(
-      minWidth: 0.0,
-      maxWidth: double.INFINITY,
-      minHeight: constraints.minHeight,
-      maxHeight: constraints.maxHeight
-    );
-  }
+  Size _contentSize;
 
-  double _getIntrinsicWidth(BoxConstraints constraints) {
-    // There should be no difference between the minimum and maximum width
-    // because we only support single-line text.
-    layoutText(_getTextContraints(constraints));
-    return constraints.constrainWidth(
-      textPainter.width + _kCursorGap + _kCursorWidth
-    );
+  ui.Paragraph _layoutTemplate;
+  double get _preferredHeight {
+    if (_layoutTemplate == null) {
+      ui.ParagraphBuilder builder = new ui.ParagraphBuilder()
+        ..pushStyle(text.style.textStyle)
+        ..addText(_kZeroWidthSpace);
+      // TODO(abarth): ParagraphBuilder#build's argument should be optional.
+      // TODO(abarth): These min/max values should be the default for ui.Paragraph.
+      _layoutTemplate = builder.build(new ui.ParagraphStyle())
+        ..minWidth = 0.0
+        ..maxWidth = double.INFINITY
+        ..minHeight = 0.0
+        ..maxHeight = double.INFINITY
+        ..layout();
+    }
+    return _layoutTemplate.height;
   }
 
   double getMinIntrinsicWidth(BoxConstraints constraints) {
-    return _getIntrinsicWidth(constraints);
+    assert(constraints.isNormalized);
+    return constraints.constrainWidth(0.0);
   }
 
   double getMaxIntrinsicWidth(BoxConstraints constraints) {
-    return _getIntrinsicWidth(constraints);
+    assert(constraints.isNormalized);
+    return constraints.constrainWidth(0.0);
   }
 
-  void performLayout() {
-    layoutText(_getTextContraints(constraints));
-    Size contentSize = new Size(textPainter.width + _kCursorGap + _kCursorWidth, textPainter.height);
-    size = constraints.constrain(contentSize);
+  double getMinIntrinsicHeight(BoxConstraints constraints) {
+    assert(constraints.isNormalized);
+    return constraints.constrainHeight(_preferredHeight);
+  }
+
+  double getMaxIntrinsicHeight(BoxConstraints constraints) {
+    assert(constraints.isNormalized);
+    return constraints.constrainHeight(_preferredHeight);
+  }
+
+  bool hitTestSelf(Point position) => true;
 
-    if (_contentSize == null || _contentSize != contentSize) {
+  BoxConstraints _constraintsForCurrentLayout; // when null, we don't have a current layout
+
+  // TODO(abarth): This logic should live in TextPainter and be shared with RenderParagraph.
+  void _layoutText(BoxConstraints constraints) {
+    assert(constraints != null);
+    assert(constraints.isNormalized);
+    if (_constraintsForCurrentLayout == constraints)
+      return; // already cached this layout
+    _textPainter.maxWidth = constraints.maxWidth;
+    _textPainter.minWidth = constraints.minWidth;
+    _textPainter.minHeight = constraints.minHeight;
+    _textPainter.maxHeight = constraints.maxHeight;
+    _textPainter.layout();
+    // By default, we shrinkwrap to the intrinsic width.
+    double width = constraints.constrainWidth(_textPainter.maxIntrinsicWidth);
+    _textPainter.minWidth = width;
+    _textPainter.maxWidth = width;
+    _textPainter.layout();
+    _constraintsForCurrentLayout = constraints;
+  }
+
+  void performLayout() {
+    size = new Size(constraints.maxWidth, constraints.constrainHeight(_preferredHeight));
+    _layoutText(new BoxConstraints(minHeight: constraints.minHeight, maxHeight: constraints.maxHeight));
+    Size contentSize = new Size(_textPainter.width + _kCursorGap + _kCursorWidth, _textPainter.height);
+    if (_contentSize != contentSize) {
       _contentSize = contentSize;
       if (onContentSizeChanged != null)
         onContentSizeChanged(_contentSize);
@@ -99,7 +155,7 @@ class RenderEditableParagraph extends RenderParagraph {
   }
 
   void _paintContents(PaintingContext context, Offset offset) {
-    textPainter.paint(context.canvas, offset - _scrollOffset);
+    _textPainter.paint(context.canvas, offset - _scrollOffset);
 
     if (_showCursor) {
       Rect cursorRect =  new Rect.fromLTWH(
@@ -113,12 +169,10 @@ class RenderEditableParagraph extends RenderParagraph {
   }
 
   void paint(PaintingContext context, Offset offset) {
-    layoutText(_getTextContraints(constraints));
     final bool hasVisualOverflow = (_contentSize.width > size.width);
     if (hasVisualOverflow)
       context.pushClipRect(needsCompositing, offset, Point.origin & size, _paintContents);
     else
       _paintContents(context, offset);
   }
-
 }

packages/flutter/lib/src/rendering/paragraph.dart

@@ -24,55 +24,56 @@ class RenderParagraph extends RenderBox {
 
   RenderParagraph(
     TextSpan text
-  ) : textPainter = new TextPainter(text) {
+  ) : _textPainter = new TextPainter(text) {
     assert(text != null);
   }
 
-  final TextPainter textPainter;
+  final TextPainter _textPainter;
 
   BoxConstraints _constraintsForCurrentLayout; // when null, we don't have a current layout
 
   /// The text to display
-  TextSpan get text => textPainter.text;
+  TextSpan get text => _textPainter.text;
   void set text(TextSpan value) {
-    if (textPainter.text == value)
+    if (_textPainter.text == value)
       return;
-    textPainter.text = value;
+    _textPainter.text = value;
     _constraintsForCurrentLayout = null;
     markNeedsLayout();
   }
 
-  void layoutText(BoxConstraints constraints) {
+  // TODO(abarth): This logic should live in TextPainter and be shared with RenderEditableLine.
+  void _layoutText(BoxConstraints constraints) {
     assert(constraints != null);
     assert(constraints.isNormalized);
     if (_constraintsForCurrentLayout == constraints)
       return; // already cached this layout
-    textPainter.maxWidth = constraints.maxWidth;
-    textPainter.minWidth = constraints.minWidth;
-    textPainter.minHeight = constraints.minHeight;
-    textPainter.maxHeight = constraints.maxHeight;
-    textPainter.layout();
+    _textPainter.maxWidth = constraints.maxWidth;
+    _textPainter.minWidth = constraints.minWidth;
+    _textPainter.minHeight = constraints.minHeight;
+    _textPainter.maxHeight = constraints.maxHeight;
+    _textPainter.layout();
     // By default, we shrinkwrap to the intrinsic width.
-    double width = constraints.constrainWidth(textPainter.maxIntrinsicWidth);
-    textPainter.minWidth = width;
-    textPainter.maxWidth = width;
-    textPainter.layout();
+    double width = constraints.constrainWidth(_textPainter.maxIntrinsicWidth);
+    _textPainter.minWidth = width;
+    _textPainter.maxWidth = width;
+    _textPainter.layout();
     _constraintsForCurrentLayout = constraints;
   }
 
   double getMinIntrinsicWidth(BoxConstraints constraints) {
-    layoutText(constraints);
-    return constraints.constrainWidth(textPainter.minIntrinsicWidth);
+    _layoutText(constraints);
+    return constraints.constrainWidth(_textPainter.minIntrinsicWidth);
   }
 
   double getMaxIntrinsicWidth(BoxConstraints constraints) {
-    layoutText(constraints);
-    return constraints.constrainWidth(textPainter.maxIntrinsicWidth);
+    _layoutText(constraints);
+    return constraints.constrainWidth(_textPainter.maxIntrinsicWidth);
   }
 
   double _getIntrinsicHeight(BoxConstraints constraints) {
-    layoutText(constraints);
-    return constraints.constrainHeight(textPainter.size.height);
+    _layoutText(constraints);
+    return constraints.constrainHeight(_textPainter.size.height);
   }
 
   double getMinIntrinsicHeight(BoxConstraints constraints) {
@@ -87,15 +88,15 @@ class RenderParagraph extends RenderBox {
 
   double computeDistanceToActualBaseline(TextBaseline baseline) {
     assert(!needsLayout);
-    layoutText(constraints);
-    return textPainter.computeDistanceToActualBaseline(baseline);
+    _layoutText(constraints);
+    return _textPainter.computeDistanceToActualBaseline(baseline);
   }
 
   bool hitTestSelf(Point position) => true;
 
   void performLayout() {
-    layoutText(constraints);
-    size = constraints.constrain(textPainter.size);
+    _layoutText(constraints);
+    size = constraints.constrain(_textPainter.size);
   }
 
   void paint(PaintingContext context, Offset offset) {
@@ -106,8 +107,8 @@ class RenderParagraph extends RenderBox {
     //
     // TODO(abarth): Make computing the min/max intrinsic width/height
     // a non-destructive operation.
-    layoutText(constraints);
-    textPainter.paint(context.canvas, offset);
+    _layoutText(constraints);
+    _textPainter.paint(context.canvas, offset);
   }
 
   // we should probably expose a way to do precise (inter-glpyh) hit testing

packages/flutter/lib/src/widgets/editable_text.dart → packages/flutter/lib/src/widgets/editable.dart

@@ -140,8 +140,8 @@ class EditableString implements KeyboardClient {
   }
 }
 
-class EditableText extends StatefulComponent {
-  EditableText({
+class RawEditableLine extends StatefulComponent {
+  RawEditableLine({
     Key key,
     this.value,
     this.focused: false,
@@ -160,10 +160,12 @@ class EditableText extends StatefulComponent {
   final SizeChangedCallback onContentSizeChanged;
   final Offset scrollOffset;
 
-  EditableTextState createState() => new EditableTextState();
+  RawEditableTextState createState() => new RawEditableTextState();
 }
 
-class EditableTextState extends State<EditableText> {
+class RawEditableTextState extends State<RawEditableLine> {
+  // TODO(abarth): Move the cursor timer into RenderEditableLine so we can
+  // remove this extra widget.
   Timer _cursorTimer;
   bool _showCursor = false;
 
@@ -209,25 +211,20 @@ class EditableTextState extends State<EditableText> {
     else if (!config.focused && _cursorTimer != null)
       _stopCursorTimer();
 
-    return new SizedBox(
-      width: double.INFINITY,
-      child: new _EditableTextWidget(
-        value: config.value,
-        style: config.style,
-        cursorColor: config.cursorColor,
-        showCursor: _showCursor,
-        hideText: config.hideText,
-        onContentSizeChanged: config.onContentSizeChanged,
-        scrollOffset: config.scrollOffset
-      )
+    return new _EditableLineWidget(
+      value: config.value,
+      style: config.style,
+      cursorColor: config.cursorColor,
+      showCursor: _showCursor,
+      hideText: config.hideText,
+      onContentSizeChanged: config.onContentSizeChanged,
+      scrollOffset: config.scrollOffset
     );
   }
 }
 
-final String _kZeroWidthSpace = new String.fromCharCode(0x200B);
-
-class _EditableTextWidget extends LeafRenderObjectWidget {
-  _EditableTextWidget({
+class _EditableLineWidget extends LeafRenderObjectWidget {
+  _EditableLineWidget({
     Key key,
     this.value,
     this.style,
@@ -246,9 +243,9 @@ class _EditableTextWidget extends LeafRenderObjectWidget {
   final SizeChangedCallback onContentSizeChanged;
   final Offset scrollOffset;
 
-  RenderEditableParagraph createRenderObject() {
-    return new RenderEditableParagraph(
-      text: _buildTextSpan(),
+  RenderEditableLine createRenderObject() {
+    return new RenderEditableLine(
+      text: _styledTextSpan,
       cursorColor: cursorColor,
       showCursor: showCursor,
       onContentSizeChanged: onContentSizeChanged,
@@ -256,17 +253,16 @@ class _EditableTextWidget extends LeafRenderObjectWidget {
     );
   }
 
-  void updateRenderObject(RenderEditableParagraph renderObject,
-                          _EditableTextWidget oldWidget) {
-    renderObject.text = _buildTextSpan();
+  void updateRenderObject(RenderEditableLine renderObject,
+                          _EditableLineWidget oldWidget) {
+    renderObject.text = _styledTextSpan;
     renderObject.cursorColor = cursorColor;
     renderObject.showCursor = showCursor;
     renderObject.onContentSizeChanged = onContentSizeChanged;
     renderObject.scrollOffset = scrollOffset;
   }
 
-  // Construct a TextSpan that renders the EditableString using the chosen style.
-  TextSpan _buildTextSpan() {
+  StyledTextSpan get _styledTextSpan {
     if (!hideText && value.composing.isValid) {
       TextStyle composingStyle = style.merge(
         const TextStyle(decoration: TextDecoration.underline)
@@ -284,8 +280,6 @@ class _EditableTextWidget extends LeafRenderObjectWidget {
     String text = value.text;
     if (hideText)
       text = new String.fromCharCodes(new List<int>.filled(text.length, 0x2022));
-    return new StyledTextSpan(style, <TextSpan>[
-      new PlainTextSpan(text.isEmpty ? _kZeroWidthSpace : text)
-    ]);
+    return new StyledTextSpan(style, <TextSpan>[ new PlainTextSpan(text) ]);
   }
 }

packages/flutter/lib/widgets.dart

@@ -9,7 +9,7 @@ export 'src/widgets/basic.dart';
 export 'src/widgets/binding.dart';
 export 'src/widgets/dismissable.dart';
 export 'src/widgets/drag_target.dart';
-export 'src/widgets/editable_text.dart';
+export 'src/widgets/editable.dart';
 export 'src/widgets/enter_exit_transition.dart';
 export 'src/widgets/focus.dart';
 export 'src/widgets/framework.dart';

packages/flutter/test/widget/input_test.dart

@@ -31,7 +31,7 @@ void main() {
   MockKeyboard mockKeyboard = new MockKeyboard();
   serviceMocker.registerMockService(KeyboardService.serviceName, mockKeyboard);
 
-  test('Editable text has consistent width', () {
+  test('Editable text has consistent size', () {
     testWidgets((WidgetTester tester) {
       GlobalKey inputKey = new GlobalKey();
       String inputValue;
@@ -53,17 +53,21 @@ void main() {
       Element input = tester.findElementByKey(inputKey);
       Size emptyInputSize = (input.renderObject as RenderBox).size;
 
-      // Simulate entry of text through the keyboard.
-      expect(mockKeyboard.client, isNotNull);
-      const String testValue = 'Test';
-      mockKeyboard.client.setComposingText(testValue, testValue.length);
+      void enterText(String testValue) {
+        // Simulate entry of text through the keyboard.
+        expect(mockKeyboard.client, isNotNull);
+        mockKeyboard.client.setComposingText(testValue, testValue.length);
 
-      // Check that the onChanged event handler fired.
-      expect(inputValue, equals(testValue));
+        // Check that the onChanged event handler fired.
+        expect(inputValue, equals(testValue));
 
-      tester.pumpWidget(builder());
+        tester.pumpWidget(builder());
+      }
+
+      enterText(' ');
+      expect((input.renderObject as RenderBox).size, equals(emptyInputSize));
 
-      // Check that the Input with text has the same size as the empty Input.
+      enterText('Test');
       expect((input.renderObject as RenderBox).size, equals(emptyInputSize));
     });
   });
@@ -85,7 +89,7 @@ void main() {
 
       tester.pumpWidget(builder());
 
-      EditableTextState editableText = tester.findStateOfType(EditableTextState);
+      RawEditableTextState editableText = tester.findStateOfType(RawEditableTextState);
 
       // Check that the cursor visibility toggles after each blink interval.
       void checkCursorToggle() {