Add better handling for very, very small promo dialogs.

Add better handling for very, very small promo dialogs.

Previously when there was no illustration, the header was
unconditionally moved outside of the scrollable content.  This
makes for a better UI overall, but on small screens it can make
the content impossible to interact with.

Now, we move the header in an out of the scrollable region based
on the height of the scroll container.  If the height is too
small, then the header is pushed into the scrollable container.

BUG=716808
Review-Url: https://codereview.chromium.org/2902793002
Cr-Commit-Position: refs/heads/master@{#474062}
Committed: https://chromium.googlesource.com/chromium/src/+/7b270ad84689e4a41259edbecdebcce2ecb8772b

[Ted C, 2017-05-23 18:28:53]

tedchoc@chromium.org changed reviewers:
+ twellington@chromium.org, yusufo@chromium.org

[Ted C, 2017-05-23 18:28:54]

PTAL

[Yusuf, 2017-05-23 18:41:28]

lgtm

https://codereview.chromium.org/2902793002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/PromoDialogLayout.java
(right):

https://codereview.chromium.org/2902793002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/widget/PromoDialogLayout.java:157:
int padding =
getResources().getDimensionPixelSize(R.dimen.promo_dialog_padding);
How about:

int padding = applyHeaderPadding ?
 getResources().getDimensionPixelSize(R.dimen.promo_dialog_padding) : 0;
ApiCompatibilityUtils.setPaddingRelative(mHeaderView, padding, 0, padding, 0);

https://codereview.chromium.org/2902793002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/widget/PromoDialogTest.java
(right):

https://codereview.chromium.org/2902793002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/widget/PromoDialogTest.java:235:
DialogParams dialogParams = new DialogParams();
would a helper function for these make it less readable?

[Ted C, 2017-05-23 19:54:35]

https://codereview.chromium.org/2902793002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/PromoDialogLayout.java
(right):

https://codereview.chromium.org/2902793002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/widget/PromoDialogLayout.java:157:
int padding =
getResources().getDimensionPixelSize(R.dimen.promo_dialog_padding);
On 2017/05/23 18:41:28, Yusuf wrote:
> How about:
> 
> int padding = applyHeaderPadding ?
>  getResources().getDimensionPixelSize(R.dimen.promo_dialog_padding) : 0;
> ApiCompatibilityUtils.setPaddingRelative(mHeaderView, padding, 0, padding, 0);

Done.

https://codereview.chromium.org/2902793002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/widget/PromoDialogTest.java
(right):

https://codereview.chromium.org/2902793002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/widget/PromoDialogTest.java:235:
DialogParams dialogParams = new DialogParams();
On 2017/05/23 18:41:28, Yusuf wrote:
> would a helper function for these make it less readable?

They have subtle existing differences that I think make a helper function more
difficult.  I found a bunch of redundant code that I could pull out to smaller
helpers that I think makes this more readable and less verbose (LMK).

https://codereview.chromium.org/2902793002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/widget/PromoDialogTest.java:321:
// is valid.
turns out the Wrapper at the top triggers a measure so this didn't do what I
wanted, so I removed it.

[Theresa, 2017-05-23 19:57:53]

lgtm

[Ted C, 2017-05-23 20:08:05]

The CQ bit was checked by tedchoc@chromium.org

[Ted C, 2017-05-23 20:08:06]

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

[commit-bot: I haz the power, 2017-05-23 20:09:07]

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-05-23 21:29:31]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495570085772600,
"parent_rev": "dbe9253353485376a03c2c35bdd462922ecb80af", "commit_rev":
"b43c7c0b0d8cdc5255cfc10f51f1fe3de296c19a"}

[commit-bot: I haz the power, 2017-05-23 21:29:54]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495570085772600,
"parent_rev": "47e012dc18b9182214af6f93f05518e635dd4674", "commit_rev":
"7b270ad84689e4a41259edbecdebcce2ecb8772b"}

[commit-bot: I haz the power, 2017-05-23 21:30:05]

Description was changed from

==========
Add better handling for very, very small promo dialogs.

Previously when there was no illustration, the header was
unconditionally moved outside of the scrollable content.  This
makes for a better UI overall, but on small screens it can make
the content impossible to interact with.

Now, we move the header in an out of the scrollable region based
on the height of the scroll container.  If the height is too
small, then the header is pushed into the scrollable container.

BUG=716808
==========

to

==========
Add better handling for very, very small promo dialogs.

Previously when there was no illustration, the header was
unconditionally moved outside of the scrollable content.  This
makes for a better UI overall, but on small screens it can make
the content impossible to interact with.

Now, we move the header in an out of the scrollable region based
on the height of the scroll container.  If the height is too
small, then the header is pushed into the scrollable container.

BUG=716808

Review-Url: https://codereview.chromium.org/2902793002
Cr-Commit-Position: refs/heads/master@{#474062}
Committed:
https://chromium.googlesource.com/chromium/src/+/7b270ad84689e4a41259edbecdeb...
==========

[commit-bot: I haz the power, 2017-05-23 21:30:10]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/7b270ad84689e4a41259edbecdeb...