Theme Input, style EditableText, restore text input cursor

Theme Input, style EditableText, restore text input cursor

The Input component conforms a little more closely
to the Material spec:
http://www.google.com/design/spec/components/text-fields.html#text-fields-single-line-text-field

Added a TextStyle attribute to EditableText and "themed" the text style for Input.

Restored the blinking EditableText input cursor.

R=abarth@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/43c3cc3d7a89ed896f33b72e23ce6895e1338fcd

[hansmuller, 2015-06-29 23:20:16]

hansmuller@chromium.org changed reviewers:
+ abarth@chromium.org

[hansmuller, 2015-06-29 23:24:20]

Patchset #1 (id:1) has been deleted

[hansmuller, 2015-06-29 23:27:02]

Patchset #1 (id:20001) has been deleted

[hansmuller, 2015-06-29 23:27:40]

hansmuller@chromium.org changed reviewers:
+ hansmuller@chromium.org

[hansmuller, 2015-06-29 23:27:41]

PTAL

[abarth-chromium, 2015-06-29 23:33:08]

LGTM w/ comments below

https://codereview.chromium.org/1216133002/diff/40001/sky/sdk/lib/editing/edi...
File sky/sdk/lib/editing/editable_text.dart (right):

https://codereview.chromium.org/1216133002/diff/40001/sky/sdk/lib/editing/edi...
sky/sdk/lib/editing/editable_text.dart:73: canvas.drawRect(cursorRect, new
Paint()..color = cursorColor);
Interesting that you're drawing outside the bounds here.

https://codereview.chromium.org/1216133002/diff/40001/sky/sdk/lib/editing/edi...
sky/sdk/lib/editing/editable_text.dart:87: if (focused == true && _cursorTimer
== null)
Why compare against true?  That shouldn't be needed.

https://codereview.chromium.org/1216133002/diff/40001/sky/sdk/lib/editing/edi...
sky/sdk/lib/editing/editable_text.dart:89: else if (focused != true &&
_cursorTimer != null)
ditto

https://codereview.chromium.org/1216133002/diff/40001/sky/sdk/lib/editing/edi...
sky/sdk/lib/editing/editable_text.dart:104: return _showCursor ? new Flex([text,
_buildCursor()]) : text;
It would be more efficient to always build the render tree structure for the
cursor by skip painting it when _showCursor is false.  Otherwise we're going to
cause layouts when the cursor appears and disappears.

[hansmuller, 2015-06-29 23:56:34]

https://codereview.chromium.org/1216133002/diff/40001/sky/sdk/lib/editing/edi...
File sky/sdk/lib/editing/editable_text.dart (right):

https://codereview.chromium.org/1216133002/diff/40001/sky/sdk/lib/editing/edi...
sky/sdk/lib/editing/editable_text.dart:73: canvas.drawRect(cursorRect, new
Paint()..color = cursorColor);
On 2015/06/29 23:33:08, abarth wrote:
> Interesting that you're drawing outside the bounds here.

The cursor extends _kCursorHeightOffset above and below the line.

The Material spec doesn't define this value (as far as I can tell) but that's
what the screenshots looked like to me.

https://codereview.chromium.org/1216133002/diff/40001/sky/sdk/lib/editing/edi...
sky/sdk/lib/editing/editable_text.dart:87: if (focused == true && _cursorTimer
== null)
On 2015/06/29 23:33:08, abarth wrote:
> Why compare against true?  That shouldn't be needed.

I've asserted that the focused: must be set and removed this test.

https://codereview.chromium.org/1216133002/diff/40001/sky/sdk/lib/editing/edi...
sky/sdk/lib/editing/editable_text.dart:89: else if (focused != true &&
_cursorTimer != null)
On 2015/06/29 23:33:08, abarth wrote:
> ditto

Done.

https://codereview.chromium.org/1216133002/diff/40001/sky/sdk/lib/editing/edi...
sky/sdk/lib/editing/editable_text.dart:104: return _showCursor ? new Flex([text,
_buildCursor()]) : text;
On 2015/06/29 23:33:08, abarth wrote:
> It would be more efficient to always build the render tree structure for the
> cursor by skip painting it when _showCursor is false.  Otherwise we're going
to
> cause layouts when the cursor appears and disappears.

Done.

[hansmuller1, 2015-06-30 00:00:42]

Committed patchset #2 (id:60001) manually as
43c3cc3d7a89ed896f33b72e23ce6895e1338fcd (presubmit successful).

[abarth-chromium, 2015-06-30 02:01:06]

On 2015/06/29 at 23:56:34, hansmuller wrote:
>
https://codereview.chromium.org/1216133002/diff/40001/sky/sdk/lib/editing/edi...
> File sky/sdk/lib/editing/editable_text.dart (right):
> 
>
https://codereview.chromium.org/1216133002/diff/40001/sky/sdk/lib/editing/edi...
> sky/sdk/lib/editing/editable_text.dart:73: canvas.drawRect(cursorRect, new
Paint()..color = cursorColor);
> On 2015/06/29 23:33:08, abarth wrote:
> > Interesting that you're drawing outside the bounds here.
> 
> The cursor extends _kCursorHeightOffset above and below the line.
> 
> The Material spec doesn't define this value (as far as I can tell) but that's
what the screenshots looked like to me.

Yeah, that make sense.  We've just been on the lookout for use cases for drawing
outside your bounds.  Shadows is another one.