Additional improvements of the new Incognito NTP

Additional improvements of the new Incognito NTP

Changes in this CL:
1. Remove the lighter font from the title and subtitle (this is
   a leftover from the previous NTP).

2. Remove the trailing newline after the third bulletpoint. This made
   the bottom margin artificially larger.

3. Move the layout code from the onMeasure() to the onLayout() phase
   where it semantically belongs. Additionally, call
   setLayoutParams() after changing LayoutParams of a View (otherwise
   the change would not be registered).

4. If the screen is wide and the two sets of bulletpoints together
   are not too wide, arrange them next to each other by wrapping
   them in a linear layout.

5. If the bulletpoints are arranged next to each other, make the
   subtitle width equal to them. NOTE: The first layout pass calculates
   the width incorrectly by 10dp. I am still not sure why, so this is
   fixed in a hacky way for now.

6. Remove the ternary rule for padding-bottom (32/48/72 dp) in favor
   of a simpler binary one (32/72 dp) after feedback from the UX.

7. Resolve the TODO about the icon size (fortunately, it's a no-op).

BUG=693525
Review-Url: https://codereview.chromium.org/2963523002
Cr-Commit-Position: refs/heads/master@{#483694}
Committed: https://chromium.googlesource.com/chromium/src/+/c65e5b86d2530a035bf17612b2e12109c942d878

[msramek, 2017-06-27 13:11:19]

Description was changed from

==========
Additional improvements of the new Incognito NTP

Changes in this CL:
1. Remove the lighter font from the title and subtitle (this is
   a leftover from the previous NTP).

2. Remove the trailing newline after the third bulletpoint. This made
   the bottom margin artificially larger.

3. Move the layout code from the onMeasure() to the onLayout() phase
   where it semantically belongs.

4. If the screen is wide and the two sets of bulletpoints together
   are not too wide, arrange them next to each other by wrapping
   them in a linear layout.

5. If the bulletpoints are arranged next to each other, make the
   subtitle width equal to them. NOTE: The first layout pass calculates
   the width incorrectly by 10dp. I am still not sure why, so this is
   fixed in a hacky way for now.

6. Remove the ternary rule for padding-bottom (32/48/72 dp) in favor
   of a simpler binary one (32/72 dp) after feedback from the UX.

BUG=693525
==========

to

==========
Additional improvements of the new Incognito NTP

Changes in this CL:
1. Remove the lighter font from the title and subtitle (this is
   a leftover from the previous NTP).

2. Remove the trailing newline after the third bulletpoint. This made
   the bottom margin artificially larger.

3. Move the layout code from the onMeasure() to the onLayout() phase
   where it semantically belongs. Additionally, call
   setLayoutParams() after changing LayoutParams of a View (otherwise
   the change would not be registered).

4. If the screen is wide and the two sets of bulletpoints together
   are not too wide, arrange them next to each other by wrapping
   them in a linear layout.

5. If the bulletpoints are arranged next to each other, make the
   subtitle width equal to them. NOTE: The first layout pass calculates
   the width incorrectly by 10dp. I am still not sure why, so this is
   fixed in a hacky way for now.

6. Remove the ternary rule for padding-bottom (32/48/72 dp) in favor
   of a simpler binary one (32/72 dp) after feedback from the UX.

7. Resolve the TODO about the icon size (fortunately, it's a no-op).

BUG=693525
==========

[msramek, 2017-06-27 13:12:55]

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

[commit-bot: I haz the power, 2017-06-27 13:13:00]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msramek, 2017-06-27 13:19:56]

msramek@chromium.org changed reviewers:
+ mvanouwerkerk@chromium.org

[msramek, 2017-06-27 13:19:57]

Hi Michael,

Can you please have a look?

This is another iteration of the new Incognito NTP. Focusing primarily on the
wide screen layout, but there are also some other UI tweaks.

Thanks!
Martin

[commit-bot: I haz the power, 2017-06-27 14:03:15]

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

[commit-bot: I haz the power, 2017-06-27 14:03:15]

Dry run: This issue passed the CQ dry run.

[msramek, 2017-06-29 17:51:42]

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

[commit-bot: I haz the power, 2017-06-29 17:52:05]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msramek, 2017-06-29 18:11:21]

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

[commit-bot: I haz the power, 2017-06-29 18:11:38]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-29 19:02:33]

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

[commit-bot: I haz the power, 2017-06-29 19:02:33]

Dry run: This issue passed the CQ dry run.

[Michael van Ouwerkerk, 2017-06-30 10:30:39]

lgtm with nits

https://codereview.chromium.org/2963523002/diff/40001/chrome/android/java/res...
File chrome/android/java/res/layout/new_tab_page_incognito_md.xml (right):

https://codereview.chromium.org/2963523002/diff/40001/chrome/android/java/res...
chrome/android/java/res/layout/new_tab_page_incognito_md.xml:54:
android:singleLine="false"
Here and below: the default value is false, so this line can be deleted.

https://developer.android.com/reference/android/widget/TextView.html#attr_and...

https://codereview.chromium.org/2963523002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPageViewMD.java
(right):

https://codereview.chromium.org/2963523002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPageViewMD.java:66:
mFirstLayout = true;
You could assign true on line 43 and save a line.

[msramek, 2017-06-30 13:27:23]

Thank you!

https://codereview.chromium.org/2963523002/diff/40001/chrome/android/java/res...
File chrome/android/java/res/layout/new_tab_page_incognito_md.xml (right):

https://codereview.chromium.org/2963523002/diff/40001/chrome/android/java/res...
chrome/android/java/res/layout/new_tab_page_incognito_md.xml:54:
android:singleLine="false"
On 2017/06/30 10:30:39, Michael van Ouwerkerk wrote:
> Here and below: the default value is false, so this line can be deleted.
> 
>
https://developer.android.com/reference/android/widget/TextView.html#attr_and...

Done.

https://codereview.chromium.org/2963523002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPageViewMD.java
(right):

https://codereview.chromium.org/2963523002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPageViewMD.java:66:
mFirstLayout = true;
On 2017/06/30 10:30:39, Michael van Ouwerkerk wrote:
> You could assign true on line 43 and save a line.

Done.

[msramek, 2017-06-30 13:27:31]

The CQ bit was checked by msramek@chromium.org

[msramek, 2017-06-30 13:27:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
mvanouwerkerk@chromium.org
Link to the patchset: https://codereview.chromium.org/2963523002/#ps60001
(title: "Addressed comments.")

[commit-bot: I haz the power, 2017-06-30 13:27:42]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-30 14:20:11]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1498829251533950,
"parent_rev": "2bfa479b45745b4c1ef2fbdbfedfc8e457f204c4", "commit_rev":
"c65e5b86d2530a035bf17612b2e12109c942d878"}

[commit-bot: I haz the power, 2017-06-30 14:20:30]

Description was changed from

==========
Additional improvements of the new Incognito NTP

Changes in this CL:
1. Remove the lighter font from the title and subtitle (this is
   a leftover from the previous NTP).

2. Remove the trailing newline after the third bulletpoint. This made
   the bottom margin artificially larger.

3. Move the layout code from the onMeasure() to the onLayout() phase
   where it semantically belongs. Additionally, call
   setLayoutParams() after changing LayoutParams of a View (otherwise
   the change would not be registered).

4. If the screen is wide and the two sets of bulletpoints together
   are not too wide, arrange them next to each other by wrapping
   them in a linear layout.

5. If the bulletpoints are arranged next to each other, make the
   subtitle width equal to them. NOTE: The first layout pass calculates
   the width incorrectly by 10dp. I am still not sure why, so this is
   fixed in a hacky way for now.

6. Remove the ternary rule for padding-bottom (32/48/72 dp) in favor
   of a simpler binary one (32/72 dp) after feedback from the UX.

7. Resolve the TODO about the icon size (fortunately, it's a no-op).

BUG=693525
==========

to

==========
Additional improvements of the new Incognito NTP

Changes in this CL:
1. Remove the lighter font from the title and subtitle (this is
   a leftover from the previous NTP).

2. Remove the trailing newline after the third bulletpoint. This made
   the bottom margin artificially larger.

3. Move the layout code from the onMeasure() to the onLayout() phase
   where it semantically belongs. Additionally, call
   setLayoutParams() after changing LayoutParams of a View (otherwise
   the change would not be registered).

4. If the screen is wide and the two sets of bulletpoints together
   are not too wide, arrange them next to each other by wrapping
   them in a linear layout.

5. If the bulletpoints are arranged next to each other, make the
   subtitle width equal to them. NOTE: The first layout pass calculates
   the width incorrectly by 10dp. I am still not sure why, so this is
   fixed in a hacky way for now.

6. Remove the ternary rule for padding-bottom (32/48/72 dp) in favor
   of a simpler binary one (32/72 dp) after feedback from the UX.

7. Resolve the TODO about the icon size (fortunately, it's a no-op).

BUG=693525

Review-Url: https://codereview.chromium.org/2963523002
Cr-Commit-Position: refs/heads/master@{#483694}
Committed:
https://chromium.googlesource.com/chromium/src/+/c65e5b86d2530a035bf17612b2e1...
==========

[commit-bot: I haz the power, 2017-06-30 14:20:31]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/c65e5b86d2530a035bf17612b2e1...