Add TextStyle fontFamily:, extend support for fontWeight:

Add TextStyle fontFamily:, extend support for fontWeight:

Defined constants for all 9 CSS font-weight values
with conventional names from the "Common weight
name mapping" section of
https://developer.mozilla.org/en-US/docs/Web/CSS/font-weight.

The FontWeight enum now just enumerates the actual
CSS weight values.

I've moved the TextStyle class into its own file.

R=ianh@google.com

Committed: https://chromium.googlesource.com/external/mojo/+/c687468cb79e9c0349a927ea3969b1d2f69f6a55

[hansmuller, 2015-06-17 21:16:56]

hansmuller@chromium.org changed reviewers:
+ ianh@google.com

[hansmuller, 2015-06-17 21:27:14]

Patchset #1 (id:1) has been deleted

[hansmuller, 2015-06-17 21:27:27]

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

[hansmuller, 2015-06-17 21:27:28]

PTAL

[Hixie, 2015-06-17 21:34:05]

lgtm

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/rendering/t...
File sky/sdk/lib/rendering/text_style.dart (right):

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/rendering/t...
sky/sdk/lib/rendering/text_style.dart:1: // Copyright 2015 The Chromium Authors.
All rights reserved.
This should be in ../painting/, since it doesn't depend on
../rendering/object.dart

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/rendering/t...
sky/sdk/lib/rendering/text_style.dart:19: const thin = FontWeight.w100;
Do we need constants for all of them? I find it confusing. Is extraLight bolder
than thin? Which is bolder, normal or medium? Is black more or less than
extraBold?

I would just have medium and bold.

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/theme2/typo...
File sky/sdk/lib/theme2/typography.dart (right):

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/theme2/typo...
sky/sdk/lib/theme2/typography.dart:14: _TextTheme(Color color54, Color color87)
I'd use the wXXX enum values here, it would make it clearer what the relative
values were.

[Hixie, 2015-06-17 21:35:49]

I didn't notice the font-family stuff at first. In general it's better to
separate the CL that moves a file from the CLs that update that same content.

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/rendering/t...
File sky/sdk/lib/rendering/text_style.dart (right):

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/rendering/t...
sky/sdk/lib/rendering/text_style.dart:122: result.add('${prefix}fontFamily:
"${fontFamily}"');
This won't work. What if someone specifies the font 'hello";}' ?

[hansmuller, 2015-06-17 22:45:07]

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/rendering/t...
File sky/sdk/lib/rendering/text_style.dart (right):

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/rendering/t...
sky/sdk/lib/rendering/text_style.dart:1: // Copyright 2015 The Chromium Authors.
All rights reserved.
On 2015/06/17 21:34:04, Hixie wrote:
> This should be in ../painting/, since it doesn't depend on
> ../rendering/object.dart

Done.

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/rendering/t...
sky/sdk/lib/rendering/text_style.dart:19: const thin = FontWeight.w100;
On 2015/06/17 21:34:04, Hixie wrote:
> Do we need constants for all of them? I find it confusing. Is extraLight
bolder
> than thin? Which is bolder, normal or medium? Is black more or less than
> extraBold?
> 
> I would just have medium and bold.

I did not make up these names :-) They're from
https://developer.mozilla.org/en-US/docs/Web/CSS/font-family. I assume they mean
something to the font-obsessed.

It seems like "normal" and "bold" are the minimal set although typography.dart
also uses "light". If they prove to be rarely-used or confusing I promise to
take them out.

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/rendering/t...
sky/sdk/lib/rendering/text_style.dart:122: result.add('${prefix}fontFamily:
"${fontFamily}"');
On 2015/06/17 21:35:49, Hixie wrote:
> This won't work. What if someone specifies the font 'hello";}' ?

You're right. We're going to need to escape fontFamily values here and before
passing them to CSS. Or I suppose (for now) we could just disallow special
characters in font-family names. I will add a TODO.

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/theme2/typo...
File sky/sdk/lib/theme2/typography.dart (right):

https://codereview.chromium.org/1173323004/diff/20001/sky/sdk/lib/theme2/typo...
sky/sdk/lib/theme2/typography.dart:14: _TextTheme(Color color54, Color color87)
On 2015/06/17 21:34:04, Hixie wrote:
> I'd use the wXXX enum values here, it would make it clearer what the relative
> values were.

Done.

[hansmuller1, 2015-06-17 22:48:33]

Committed patchset #2 (id:40001) manually as
c687468cb79e9c0349a927ea3969b1d2f69f6a55 (presubmit successful).

[Hixie, 2015-06-17 22:49:42]

I'm not saying you made up the names. :-)

The Web has all kinds of crazy. We should be very careful about what we import
of that crazy. In this particular case, having labels for 9 values just doesn't
work. It's unintuitive. We should just use the numbers, except maybe for
"normal" and "bold", IMHO.

[hansmuller, 2015-06-17 22:58:29]

On 2015/06/17 22:49:42, Hixie wrote:
> I'm not saying you made up the names. :-)
> 
> The Web has all kinds of crazy. We should be very careful about what we import
> of that crazy. In this particular case, having labels for 9 values just
doesn't
> work. It's unintuitive. We should just use the numbers, except maybe for
> "normal" and "bold", IMHO.

OK. https://codereview.chromium.org/1189483007/