From 58b421aaf4cd9dbb43c8da27aac01f6586fc759a Mon Sep 17 00:00:00 2001
From: Jason Simmons <jsimmons@chromium.org>
Date: Fri, 18 Sep 2015 13:41:33 -0700
Subject: [PATCH] Ensure that EditableText always shows a cursor

Previously EditableText would render a text widget with no cursor if the text
value was empty.

Also adjust the height of the cursor widget to reflect the style's line
height, and update the cursor painting to match.
---
 .../lib/src/widgets/editable_text.dart        | 35 +++++++++------
 packages/unit/test/widget/input_test.dart     | 44 +++++++++++++++++--
 2 files changed, 63 insertions(+), 16 deletions(-)

diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart
index 3280da4c235..8b3ff14e86e 100644
--- a/packages/flutter/lib/src/widgets/editable_text.dart
+++ b/packages/flutter/lib/src/widgets/editable_text.dart
@@ -156,6 +156,13 @@ class EditableText extends StatefulComponent {
   Timer _cursorTimer;
   bool _showCursor = false;
 
+  /// Whether the blinking cursor is visible (exposed for testing).
+  bool get test_showCursor => _showCursor;
+
+  /// The cursor blink interval (exposed for testing).
+  Duration get test_cursorBlinkPeriod =>
+      new Duration(milliseconds: _kCursorBlinkPeriod);
+
   void _cursorTick(Timer timer) {
     setState(() {
       _showCursor = !_showCursor;
@@ -184,11 +191,12 @@ class EditableText extends StatefulComponent {
     if (!_showCursor)
       return;
 
+    double cursorHeight = style.fontSize + 2.0 * _kCursorHeightOffset;
     Rect cursorRect =  new Rect.fromLTWH(
       _kCursorGap,
-      -_kCursorHeightOffset,
+      (size.height - cursorHeight) / 2.0,
       _kCursorWidth,
-      style.fontSize + 2 * _kCursorHeightOffset
+      cursorHeight
     );
     canvas.drawRect(cursorRect, new Paint()..color = cursorColor);
   }
@@ -203,21 +211,22 @@ class EditableText extends StatefulComponent {
     else if (!focused && _cursorTimer != null)
       _stopCursorTimer();
 
-    if (!value.composing.isValid) {
+    Widget text;
+    if (value.composing.isValid) {
+      TextStyle composingStyle = style.merge(const TextStyle(decoration: underline));
+      text = new StyledText(elements: [
+        style,
+        value.textBefore(value.composing),
+        [composingStyle, value.textInside(value.composing)],
+        value.textAfter(value.composing)
+      ]);
+    } else {
       // TODO(eseidel): This is the wrong height if empty!
-      return new Row([new Text(value.text, style: style)]);
+      text = new Text(value.text, style: style);
     }
 
-    TextStyle composingStyle = style.merge(const TextStyle(decoration: underline));
-    StyledText text = new StyledText(elements: [
-      style,
-      value.textBefore(value.composing),
-      [composingStyle, value.textInside(value.composing)],
-      value.textAfter(value.composing)
-    ]);
-
     Widget cursor = new Container(
-      height: style.fontSize,
+      height: style.fontSize * style.height,
       width: _kCursorGap + _kCursorWidth,
       child: new CustomPaint(callback: _paintCursor, token: _showCursor)
     );
diff --git a/packages/unit/test/widget/input_test.dart b/packages/unit/test/widget/input_test.dart
index b7b4f3762ae..e0379a916bb 100644
--- a/packages/unit/test/widget/input_test.dart
+++ b/packages/unit/test/widget/input_test.dart
@@ -1,4 +1,5 @@
 import 'package:mojo_services/keyboard/keyboard.mojom.dart';
+import 'package:quiver/testing/async.dart';
 import 'package:sky/rendering.dart';
 import 'package:sky/services.dart';
 import 'package:sky/widgets.dart';
@@ -20,12 +21,12 @@ class MockKeyboard implements KeyboardService {
 }
 
 void main() {
+  MockKeyboard mockKeyboard = new MockKeyboard();
+  serviceMocker.registerMockService(KeyboardServiceName, mockKeyboard);
+
   test('Editable text has consistent width', () {
     WidgetTester tester = new WidgetTester();
 
-    MockKeyboard mockKeyboard = new MockKeyboard();
-    serviceMocker.registerMockService(KeyboardServiceName, mockKeyboard);
-
     GlobalKey inputKey = new GlobalKey();
     String inputValue;
 
@@ -57,4 +58,41 @@ void main() {
     // Check that the Input with text has the same size as the empty Input.
     expect((input.renderObject as RenderBox).size, equals(emptyInputSize));
   });
+
+  test('Cursor blinks', () {
+    WidgetTester tester = new WidgetTester();
+
+    GlobalKey inputKey = new GlobalKey();
+
+    Widget builder() {
+      return new Center(
+        child: new Input(
+          key: inputKey,
+          placeholder: 'Placeholder'
+        )
+      );
+    }
+
+    new FakeAsync().run((async) {
+      tester.pumpFrame(builder);
+
+      EditableText editableText = tester.findWidget(
+          (Widget widget) => widget is EditableText);
+
+      // Check that the cursor visibility toggles after each blink interval.
+      void checkCursorToggle() {
+        bool initialShowCursor = editableText.test_showCursor;
+        async.elapse(editableText.test_cursorBlinkPeriod);
+        expect(editableText.test_showCursor, equals(!initialShowCursor));
+        async.elapse(editableText.test_cursorBlinkPeriod);
+        expect(editableText.test_showCursor, equals(initialShowCursor));
+      }
+
+      checkCursorToggle();
+
+      // Try the test again with a nonempty EditableText.
+      mockKeyboard.client.setComposingText('X', 1);
+      checkCursorToggle();
+    });
+  });
 }