EditableText underlines the word being edited

sky/sdk/lib/editing/input.dart

Index: sky/sdk/lib/editing/input.dart
diff --git a/sky/sdk/lib/editing/input.dart b/sky/sdk/lib/editing/input.dart
index 12985f81f954c73dfd328976e7038fdf8d1b763f..d6b6de94750900f83a900c2d414d3ac484a1a79b 100644
--- a/sky/sdk/lib/editing/input.dart
+++ b/sky/sdk/lib/editing/input.dart
@@ -2,6 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import '../painting/text_style.dart';
+import '../theme/colors.dart';
+import '../theme/typography.dart' as typography;
 import '../widgets/basic.dart';
 import 'editable_string.dart';
 import 'editable_text.dart';
@@ -22,27 +25,7 @@ class Input extends Component {
     );
   }
 
-  // static final Style _style = new Style('''
-  //   transform: translateX(0);
-  //   margin: 8px;
-  //   padding: 8px;
-  //   border-bottom: 1px solid ${Grey[200]};
-  //   align-self: center;
-  //   height: 1.2em;
-  //   white-space: pre;

[Hixie, 2015/06/19 22:39:44]

did white-space:pre get carried over?

[hansmuller, 2015/06/19 23:32:12]

Good point. The height and white-space haven't been carried forward.
I'll add a comment.

-  //   overflow: hidden;'''
-  // );
-
-  // static final Style _placeholderStyle = new Style('''
-  //   top: 8px;
-  //   left: 8px;
-  //   position: absolute;
-  //   ${typography.black.caption};'''
-  // );
-
-  // static final String _focusedInlineStyle = '''
-  //   padding: 7px;
-  //   border-bottom: 2px solid ${Blue[500]};''';
+  static final TextStyle _placeholderStyle = typography.black.caption;
 
   String placeholder;
   ValueChanged onChanged;
@@ -73,21 +56,35 @@ class Input extends Component {
       _isAttachedToKeyboard = true;
     }
 
-    List<Widget> children = [];
-
+    List<Widget> textChildren = [];

[Hixie, 2015/06/19 22:39:44]

<Widget>[].

[hansmuller, 2015/06/19 23:32:12]

Done.

     if (placeholder != null && _value.isEmpty) {
-      children.add(new Container(
-          // style: _placeholderStyle,
-          child: new Text(placeholder)
+      textChildren.add(new Container(

[Hixie, 2015/06/19 22:39:44]

What is the Container() for?

[hansmuller, 2015/06/19 23:32:12]

Oops, thanks for pointing that out.

+        child: new Text(placeholder, style: _placeholderStyle)
       ));
     }
+    textChildren.add(
+      new EditableText(value: _editableValue, focused: focused));

[Hixie, 2015/06/19 22:39:44]

If you put a line break after a (, but a line break before its corresponding ).

[hansmuller, 2015/06/19 23:32:12]

Done.

+
+    Container text = new Container(
+      padding: const EdgeDims.only(left: 8.0, right: 8.0, top: 2.0),
+      child: new Stack(textChildren),
+      constraints: new BoxConstraints(minWidth: double.INFINITY)

[abarth-chromium, 2015/06/19 22:41:38]

You might as well just write:

width: double.INFINITY

But rather than do this, see below.

[hansmuller, 2015/06/19 23:32:12]

Acknowledged.

+    );
 
-    children.add(new EditableText(value: _editableValue, focused: focused));
+    Color focusColor = focused ? Blue[400] : Grey[200];
+    Container focusHighlight = new Container(
+      margin: const EdgeDims.only(top: 8.0),
+      constraints: new BoxConstraints.tightFor(height: focused ? 2.0 : 1.0),
+      decoration: new BoxDecoration(backgroundColor: focusColor)

[abarth-chromium, 2015/06/19 22:41:38]

Why not use a bottom border to draw this line?  That's much cheaper than
creating an entire entry in the render tree.

[hansmuller, 2015/06/19 23:32:12]

Done.

+    );
+
+    Flex input = new Flex([text, focusHighlight],
+      direction: FlexDirection.vertical, 
+      justifyContent: FlexJustifyContent.center
+    );
 
     return new Listener(
-      // style: _style,
-      // inlineStyle: focused ? _focusedInlineStyle : null,
-      child: new Stack(children),
+      child: input,
       onPointerDown: (_) => keyboard.showByRequest()
     );
   }
@@ -97,5 +94,4 @@ class Input extends Component {
       keyboard.hide();
     super.didUnmount();
   }
-
 }