Improve the factoring between Input and RawEditableLine

RawEditableLine is now responsible for the scrolling behavior, which
removes the need for callbacks between RawEditableLine and Input. It
also fixes a bug whereby the whole Input widget (including its icon)
would scroll when the text got long.

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

@@ -15,7 +15,7 @@ export 'package:flutter/rendering.dart' show ValueChanged;
 export 'package:flutter/services.dart' show KeyboardType;
 
 /// A material design text input field.
-class Input extends Scrollable {
+class Input extends StatefulComponent {
   Input({
     GlobalKey key,
     this.initialValue: '',
@@ -30,11 +30,7 @@ class Input extends Scrollable {
     this.autofocus: false,
     this.onChanged,
     this.onSubmitted
-  }) : super(
-    key: key,
-    initialScrollOffset: 0.0,
-    scrollDirection: Axis.horizontal
-  ) {
+  }) : super(key: key) {
     assert(key != null);
   }
 
@@ -74,17 +70,15 @@ class Input extends Scrollable {
   /// Called when the user indicates that they are done editing the text in the field.
   final ValueChanged<String> onSubmitted;
 
-  InputState createState() => new InputState();
+  _InputState createState() => new _InputState();
 }
 
-class InputState extends ScrollableState<Input> {
+class _InputState extends State<Input> {
   String _value;
   EditableString _editableString;
   KeyboardHandle _keyboardHandle = KeyboardHandle.unattached;
 
-  double _contentWidth = 0.0;
-  double _containerWidth = 0.0;
-
+  // Used by tests.
   EditableString get editableValue => _editableString;
 
   void initState() {
@@ -97,6 +91,23 @@ class InputState extends ScrollableState<Input> {
     );
   }
 
+  void dispose() {
+    if (_keyboardHandle.attached)
+      _keyboardHandle.release();
+    super.dispose();
+  }
+
+  void _attachOrDetachKeyboard(bool focused) {
+    if (focused && !_keyboardHandle.attached) {
+      _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();
+    }
+  }
+
   void _handleTextUpdated() {
     if (_value != _editableString.text) {
       setState(() {
@@ -113,13 +124,40 @@ class InputState extends ScrollableState<Input> {
       config.onSubmitted(_value);
   }
 
-  Widget _buildEditableField({
-    ThemeData themeData,
-    bool focused,
-    Color focusHighlightColor,
-    TextStyle textStyle,
-    double topPadding
-  }) {
+  Widget build(BuildContext context) {
+    assert(debugCheckHasMaterial(context));
+    ThemeData themeData = Theme.of(context);
+    bool focused = Focus.at(context, autofocus: config.autofocus);
+
+    _attachOrDetachKeyboard(focused);
+
+    TextStyle textStyle = config.style ?? themeData.text.subhead;
+    Color focusHighlightColor = themeData.accentColor;
+    if (themeData.primarySwatch != null)
+      focusHighlightColor = focused ? themeData.primarySwatch[400] : themeData.hintColor;
+    double topPadding = config.isDense ? 12.0 : 16.0;
+
+    List<Widget> stackChildren = <Widget>[];
+
+    if (config.labelText != null) {
+      TextStyle labelStyle = themeData.text.caption.copyWith(color: focused ? focusHighlightColor : themeData.hintColor);
+      stackChildren.add(new Positioned(
+        left: 0.0,
+        top: topPadding,
+        child: new Text(config.labelText, style: labelStyle)
+      ));
+      topPadding += labelStyle.fontSize + (config.isDense ? 4.0 : 8.0);
+    }
+
+    if (config.hintText != null && _value.isEmpty) {
+      TextStyle hintStyle = textStyle.copyWith(color: themeData.hintColor);
+      stackChildren.add(new Positioned(
+        left: 0.0,
+        top: topPadding,
+        child: new Text(config.hintText, style: hintStyle)
+      ));
+    }
+
     Color cursorColor = themeData.primarySwatch == null ?
       themeData.accentColor :
       themeData.primarySwatch[200];
@@ -138,7 +176,7 @@ class InputState extends ScrollableState<Input> {
       }
     }
 
-    return new Container(
+    stackChildren.add(new Container(
       margin: margin,
       padding: padding,
       decoration: new BoxDecoration(
@@ -149,68 +187,13 @@ class InputState extends ScrollableState<Input> {
           )
         )
       ),
-      child: new SizeObserver(
-        onSizeChanged: _handleContainerSizeChanged,
       child: new RawEditableLine(
         value: _editableString,
         focused: focused,
         style: textStyle,
         hideText: config.hideText,
-          cursorColor: cursorColor,
-          onContentSizeChanged: _handleContentSizeChanged,
-          scrollOffset: scrollOffsetVector
-        )
+        cursorColor: cursorColor
       )
-    );
-  }
-
-  Widget buildContent(BuildContext context) {
-    assert(debugCheckHasMaterial(context));
-    ThemeData themeData = Theme.of(context);
-    bool focused = Focus.at(context, autofocus: config.autofocus);
-
-    if (focused && !_keyboardHandle.attached) {
-      _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();
-    }
-
-    TextStyle textStyle = config.style ?? themeData.text.subhead;
-    Color focusHighlightColor = themeData.accentColor;
-    if (themeData.primarySwatch != null)
-      focusHighlightColor = focused ? themeData.primarySwatch[400] : themeData.hintColor;
-    double topPadding = config.isDense ? 12.0 : 16.0;
-
-    List<Widget> stackChildren = <Widget>[];
-
-    if (config.labelText != null) {
-      TextStyle labelStyle = themeData.text.caption.copyWith(color: focused ? focusHighlightColor : themeData.hintColor);
-      stackChildren.add(new Positioned(
-        left: 0.0,
-        top: topPadding,
-        child: new Text(config.labelText, style: labelStyle)
-      ));
-      topPadding += labelStyle.fontSize + (config.isDense ? 4.0 : 8.0);
-    }
-
-    if (config.hintText != null && _value.isEmpty) {
-      TextStyle hintStyle = textStyle.copyWith(color: themeData.hintColor);
-      stackChildren.add(new Positioned(
-        left: 0.0,
-        top: topPadding,
-        child: new Text(config.hintText, style: hintStyle)
-      ));
-    }
-
-    stackChildren.add(_buildEditableField(
-      themeData: themeData,
-      focused: focused,
-      focusHighlightColor: focusHighlightColor,
-      textStyle: textStyle,
-      topPadding: topPadding
     ));
 
     if (config.errorText != null && !config.isDense) {
@@ -261,33 +244,4 @@ class InputState extends ScrollableState<Input> {
       )
     );
   }
-
-  void dispose() {
-    if (_keyboardHandle.attached)
-      _keyboardHandle.release();
-    super.dispose();
-  }
-
-  ScrollBehavior createScrollBehavior() => new BoundedBehavior();
-  BoundedBehavior get scrollBehavior => super.scrollBehavior;
-
-  void _handleContainerSizeChanged(Size newSize) {
-    _containerWidth = newSize.width;
-    _updateScrollBehavior();
-  }
-
-  void _handleContentSizeChanged(Size newSize) {
-    _contentWidth = newSize.width;
-    _updateScrollBehavior();
-  }
-
-  void _updateScrollBehavior() {
-    // Set the scroll offset to match the content width so that the cursor
-    // (which is always at the end of the text) will be visible.
-    scrollTo(scrollBehavior.updateExtents(
-      contentExtent: _contentWidth,
-      containerExtent: _containerWidth,
-      scrollOffset: _contentWidth
-    ));
-  }
 }

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

@@ -22,13 +22,14 @@ class RenderEditableLine extends RenderBox {
   RenderEditableLine({
     StyledTextSpan text,
     Color cursorColor,
-    bool showCursor,
-    this.onContentSizeChanged,
-    Offset scrollOffset
+    bool showCursor: false,
+    Offset paintOffset: Offset.zero,
+    this.onContentSizeChanged
   }) : _textPainter = new TextPainter(text),
        _cursorColor = cursorColor,
        _showCursor = showCursor,
-       _scrollOffset = scrollOffset {
+       _paintOffset = paintOffset {
+  assert(!showCursor || cursorColor != null);
   // TODO(abarth): These min/max values should be the default for TextPainter.
   _textPainter
     ..minWidth = 0.0
@@ -71,12 +72,12 @@ class RenderEditableLine extends RenderBox {
     markNeedsPaint();
   }
 
-  Offset get scrollOffset => _scrollOffset;
-  Offset _scrollOffset;
-  void set scrollOffset(Offset value) {
-    if (_scrollOffset == value)
+  Offset get paintOffset => _paintOffset;
+  Offset _paintOffset;
+  void set paintOffset(Offset value) {
+    if (_paintOffset == value)
       return;
-    _scrollOffset = value;
+    _paintOffset = value;
     markNeedsPaint();
   }
 
@@ -155,12 +156,12 @@ class RenderEditableLine extends RenderBox {
   }
 
   void _paintContents(PaintingContext context, Offset offset) {
-    _textPainter.paint(context.canvas, offset - _scrollOffset);
+    _textPainter.paint(context.canvas, offset + _paintOffset);
 
     if (_showCursor) {
       Rect cursorRect =  new Rect.fromLTWH(
-        offset.dx + _contentSize.width - _kCursorWidth - _scrollOffset.dx,
-        offset.dy + _kCursorHeightOffset - _scrollOffset.dy,
+        offset.dx + _paintOffset.dx + _contentSize.width - _kCursorWidth,
+        offset.dy + _paintOffset.dy + _kCursorHeightOffset,
         _kCursorWidth,
         size.height - 2.0 * _kCursorHeightOffset
       );

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

@@ -11,26 +11,39 @@ import 'package:flutter/rendering.dart';
 
 import 'basic.dart';
 import 'framework.dart';
+import 'scrollable.dart';
+import 'scroll_behavior.dart';
 
 const Duration _kCursorBlinkHalfPeriod = const Duration(milliseconds: 500);
 
 /// A range of characters in a string of tet.
 class TextRange {
   const TextRange({ this.start, this.end });
+
+  /// A text range that starts and ends at position.
   const TextRange.collapsed(int position)
     : start = position,
       end = position;
+
+  /// A text range that contains nothing and is not in the text.
   const TextRange.empty()
     : start = -1,
       end = -1;
 
+  /// The index of the first character in the range.
   final int start;
+
+  /// The next index after the characters in this range.
   final int end;
 
+  /// Whether this range represents a valid position in the text.
   bool get isValid => start >= 0 && end >= 0;
+
+  /// Whether this range is empty (but still potentially placed inside the text).
   bool get isCollapsed => start == end;
 }
 
+/// A string that can be manipulated by a keyboard.
 class EditableString implements KeyboardClient {
   EditableString({this.text: '', this.onUpdated, this.onSubmitted}) {
     assert(onUpdated != null);
@@ -39,23 +52,35 @@ class EditableString implements KeyboardClient {
     selection = new TextRange(start: text.length, end: text.length);
   }
 
+  /// The current text being edited.
   String text;
+
+  // The range of text that is still being composed.
   TextRange composing = const TextRange.empty();
+
+  /// The range of text that is currently selected.
   TextRange selection;
 
+  /// Called whenever the text changes.
   final VoidCallback onUpdated;
+
+  /// Called whenever the user indicates they are done editing the string.
   final VoidCallback onSubmitted;
 
+  /// A keyboard client stub that can be attached to a keyboard service.
   KeyboardClientStub stub;
 
+  /// The text before the given range.
   String textBefore(TextRange range) {
     return text.substring(0, range.start);
   }
 
+  /// The text after the given range.
   String textAfter(TextRange range) {
     return text.substring(range.end);
   }
 
+  /// The text inside the given range.
   String textInside(TextRange range) {
     return text.substring(range.start, range.end);
   }
@@ -141,35 +166,72 @@ class EditableString implements KeyboardClient {
   }
 }
 
-class RawEditableLine extends StatefulComponent {
+/// A basic single-line input control.
+///
+/// This control is not intended to be used directly. Instead, consider using
+/// [Input], which provides focus management and material design.
+class RawEditableLine extends Scrollable {
   RawEditableLine({
     Key key,
     this.value,
     this.focused: false,
     this.hideText: false,
     this.style,
-    this.cursorColor,
-    this.onContentSizeChanged,
-    this.scrollOffset
-  }) : super(key: key);
+    this.cursorColor
+  }) : super(
+    key: key,
+    initialScrollOffset: 0.0,
+    scrollDirection: Axis.horizontal
+  );
 
+  /// The editable string being displayed in this widget.
   final EditableString value;
+
+  /// Whether this widget is focused.
   final bool focused;
+
+  /// Whether to hide the text being edited (e.g., for passwords).
   final bool hideText;
+
+  /// The text style to use for the editable text.
   final TextStyle style;
+
+  /// The color to use when painting the cursor.
   final Color cursorColor;
-  final SizeChangedCallback onContentSizeChanged;
-  final Offset scrollOffset;
 
   RawEditableTextState createState() => new RawEditableTextState();
 }
 
-class RawEditableTextState extends State<RawEditableLine> {
-  // TODO(abarth): Move the cursor timer into RenderEditableLine so we can
-  // remove this extra widget.
+class RawEditableTextState extends ScrollableState<RawEditableLine> {
   Timer _cursorTimer;
   bool _showCursor = false;
 
+  double _contentWidth = 0.0;
+  double _containerWidth = 0.0;
+
+  ScrollBehavior createScrollBehavior() => new BoundedBehavior();
+  BoundedBehavior get scrollBehavior => super.scrollBehavior;
+
+  void _handleContainerSizeChanged(Size newSize) {
+    _containerWidth = newSize.width;
+    _updateScrollBehavior();
+  }
+
+  void _handleContentSizeChanged(Size newSize) {
+    _contentWidth = newSize.width;
+    _updateScrollBehavior();
+  }
+
+  void _updateScrollBehavior() {
+    // Set the scroll offset to match the content width so that the cursor
+    // (which is always at the end of the text) will be visible.
+    scrollTo(scrollBehavior.updateExtents(
+      contentExtent: _contentWidth,
+      containerExtent: _containerWidth,
+      scrollOffset: _contentWidth
+    ));
+  }
+
   /// Whether the blinking cursor is actually visible at this precise moment
   /// (it's hidden half the time, since it blinks).
   bool get cursorCurrentlyVisible => _showCursor;
@@ -202,7 +264,7 @@ class RawEditableTextState extends State<RawEditableLine> {
     _showCursor = false;
   }
 
-  Widget build(BuildContext context) {
+  Widget buildContent(BuildContext context) {
     assert(config.style != null);
     assert(config.focused != null);
     assert(config.cursorColor != null);
@@ -212,14 +274,17 @@ class RawEditableTextState extends State<RawEditableLine> {
     else if (!config.focused && _cursorTimer != null)
       _stopCursorTimer();
 
-    return new _EditableLineWidget(
+    return new SizeObserver(
+      onSizeChanged: _handleContainerSizeChanged,
+      child: new _EditableLineWidget(
         value: config.value,
         style: config.style,
         cursorColor: config.cursorColor,
         showCursor: _showCursor,
         hideText: config.hideText,
-      onContentSizeChanged: config.onContentSizeChanged,
-      scrollOffset: config.scrollOffset
+        onContentSizeChanged: _handleContentSizeChanged,
+        paintOffset: new Offset(-scrollOffset, 0.0)
+      )
     );
   }
 }
@@ -233,7 +298,7 @@ class _EditableLineWidget extends LeafRenderObjectWidget {
     this.showCursor,
     this.hideText,
     this.onContentSizeChanged,
-    this.scrollOffset
+    this.paintOffset
   }) : super(key: key);
 
   final EditableString value;
@@ -242,7 +307,7 @@ class _EditableLineWidget extends LeafRenderObjectWidget {
   final bool showCursor;
   final bool hideText;
   final SizeChangedCallback onContentSizeChanged;
-  final Offset scrollOffset;
+  final Offset paintOffset;
 
   RenderEditableLine createRenderObject() {
     return new RenderEditableLine(
@@ -250,7 +315,7 @@ class _EditableLineWidget extends LeafRenderObjectWidget {
       cursorColor: cursorColor,
       showCursor: showCursor,
       onContentSizeChanged: onContentSizeChanged,
-      scrollOffset: scrollOffset
+      paintOffset: paintOffset
     );
   }
 
@@ -260,7 +325,7 @@ class _EditableLineWidget extends LeafRenderObjectWidget {
     renderObject.cursorColor = cursorColor;
     renderObject.showCursor = showCursor;
     renderObject.onContentSizeChanged = onContentSizeChanged;
-    renderObject.scrollOffset = scrollOffset;
+    renderObject.paintOffset = paintOffset;
   }
 
   StyledTextSpan get _styledTextSpan {

packages/flutter/test/widget/input_test.dart

@@ -133,7 +133,7 @@ void main() {
 
       const String testValue = 'ABC';
       mockKeyboard.client.commitText(testValue, testValue.length);
-      InputState input = tester.findStateOfType(InputState);
+      dynamic input = inputKey.currentState;
 
       // Delete characters and verify that the selection follows the length
       // of the text.