Create TextViewWithLeading.

Create TextViewWithLeading.

Create an extension of the TextView class that performs the calculations necessary to set the text leading in XML.

The calculation was taken from go/md-text-reco.

BUG=607803
Committed: https://crrev.com/916492e02a7ee04834f260c2e58d01c17f823b85
Cr-Commit-Position: refs/heads/master@{#392888}

[PEConn, 2016-05-06 09:36:22]

Description was changed from

==========
Create TextViewWithLeading.

Create an extension of the TextView class that performs the calculations
necessary to set the text leading in XML.

BUG=607803
==========

to

==========
Create TextViewWithLeading.

Create an extension of the TextView class that performs the calculations
necessary to set the text leading in XML.

The calculation was taken from go/md-text-reco.

BUG=607803
==========

[PEConn, 2016-05-06 09:42:46]

peconn@chromium.org changed reviewers:
+ bauerb@chromium.org, maybelle@chromium.org

[PEConn, 2016-05-06 09:42:49]

PTAL, and do we have any preferences for which owners to use for the other
files?

[May, 2016-05-06 11:18:18]

lgtm

Among the owners, maybe tedchoc@ would be the best to take a look at it?

[Bernhard Bauer, 2016-05-06 12:20:23]

On 2016/05/06 09:42:49, PEConn1 wrote:
> PTAL, and do we have any preferences for which owners to use for the other
> files?

Yeah, that's a bit tricky -- 3 out of 4 //ui/android/OWNERS are OOO right now,
and the 4th one is in a time zone ahead of us, so it's past working hours... I
would maybe try Miguel, as he is the first one to be back?

https://codereview.chromium.org/1955813002/diff/1/ui/android/java/src/org/chr...
File ui/android/java/src/org/chromium/ui/widget/TextViewWithLeading.java
(right):

https://codereview.chromium.org/1955813002/diff/1/ui/android/java/src/org/chr...
ui/android/java/src/org/chromium/ui/widget/TextViewWithLeading.java:18: * it to
be set in XML.
Can you add that setting leading will override both
android:lineSpacingMultiplier and android:lineSpacingMultiplier attributes?

https://codereview.chromium.org/1955813002/diff/1/ui/android/java/src/org/chr...
ui/android/java/src/org/chromium/ui/widget/TextViewWithLeading.java:28: float
oldLineSpacing = getPaint().getFontMetricsInt(null);
IIUC, the value we get here is the current value of what we call "leading"
above. So, I would call this |oldLeading| or something. It would make it a bit
clearer to me that we're setting the delta between current and desired value
below, and would only use the term "line spacing" in code that we don't control.

Also, if you want this as a float anyway, could you use getFontMetrics()?

[PEConn, 2016-05-06 15:11:40]

peconn@chromium.org changed reviewers:
+ tedchoc@chromium.org

[PEConn, 2016-05-06 15:11:40]

https://codereview.chromium.org/1955813002/diff/1/ui/android/java/src/org/chr...
File ui/android/java/src/org/chromium/ui/widget/TextViewWithLeading.java
(right):

https://codereview.chromium.org/1955813002/diff/1/ui/android/java/src/org/chr...
ui/android/java/src/org/chromium/ui/widget/TextViewWithLeading.java:18: * it to
be set in XML.
On 2016/05/06 12:20:23, Bernhard Bauer wrote:
> Can you add that setting leading will override both
> android:lineSpacingMultiplier and android:lineSpacingMultiplier attributes?

Done.

https://codereview.chromium.org/1955813002/diff/1/ui/android/java/src/org/chr...
ui/android/java/src/org/chromium/ui/widget/TextViewWithLeading.java:28: float
oldLineSpacing = getPaint().getFontMetricsInt(null);
On 2016/05/06 12:20:23, Bernhard Bauer wrote:
> IIUC, the value we get here is the current value of what we call "leading"
> above. So, I would call this |oldLeading| or something. It would make it a bit
> clearer to me that we're setting the delta between current and desired value
> below, and would only use the term "line spacing" in code that we don't
control.
> 
> Also, if you want this as a float anyway, could you use getFontMetrics()?

Done.

[Bernhard Bauer, 2016-05-06 15:16:16]

LGTM

[PEConn, 2016-05-06 15:23:13]

The CQ bit was checked by peconn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-06 15:23:23]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1955813002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1955813002/20001

[commit-bot: I haz the power, 2016-05-06 16:35:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-06 16:35:30]

Dry run: This issue passed the CQ dry run.

[PEConn, 2016-05-11 08:36:01]

peconn@chromium.org changed reviewers:
+ miguelg@chromium.org

[PEConn, 2016-05-11 08:36:02]

Miguel, could I please get an LGTM for the files in ui/ ?

[Miguel Garcia, 2016-05-11 08:37:58]

lgtm for ui/android

[PEConn, 2016-05-11 08:52:29]

The CQ bit was checked by peconn@chromium.org

[PEConn, 2016-05-11 08:52:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
maybelle@chromium.org
Link to the patchset: https://codereview.chromium.org/1955813002/#ps20001
(title: " ")

[commit-bot: I haz the power, 2016-05-11 08:52:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1955813002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1955813002/20001

[commit-bot: I haz the power, 2016-05-11 09:53:40]

Description was changed from

==========
Create TextViewWithLeading.

Create an extension of the TextView class that performs the calculations
necessary to set the text leading in XML.

The calculation was taken from go/md-text-reco.

BUG=607803
==========

to

==========
Create TextViewWithLeading.

Create an extension of the TextView class that performs the calculations
necessary to set the text leading in XML.

The calculation was taken from go/md-text-reco.

BUG=607803
==========

[commit-bot: I haz the power, 2016-05-11 09:53:40]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-11 09:55:37]

Description was changed from

==========
Create TextViewWithLeading.

Create an extension of the TextView class that performs the calculations
necessary to set the text leading in XML.

The calculation was taken from go/md-text-reco.

BUG=607803
==========

to

==========
Create TextViewWithLeading.

Create an extension of the TextView class that performs the calculations
necessary to set the text leading in XML.

The calculation was taken from go/md-text-reco.

BUG=607803

Committed: https://crrev.com/916492e02a7ee04834f260c2e58d01c17f823b85
Cr-Commit-Position: refs/heads/master@{#392888}
==========

[commit-bot: I haz the power, 2016-05-11 09:55:38]

Patchset 2 (id:??) landed as
https://crrev.com/916492e02a7ee04834f260c2e58d01c17f823b85
Cr-Commit-Position: refs/heads/master@{#392888}

[Ted C, 2016-05-11 17:25:53]

https://codereview.chromium.org/1955813002/diff/20001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/widget/TextViewWithLeading.java
(right):

https://codereview.chromium.org/1955813002/diff/20001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/widget/TextViewWithLeading.java:28: final
float oldLeading = getPaint().getFontMetrics(null);
out of curiosity, did you try this on pre-L?

Dan recently pointed out to me that lineSpacingExtra was applied for even single
lines pre L which rarely is what we wanted.

Maybe this accounts for it, but you should quickly test on JB/KK to see if this
looks bad there.