Fix spacing on welcome screen for small screen (iPhone 4)

Fix spacing on welcome screen for small screen (iPhone 4)

Shift the welcome screen contents up to allow for correct spacing when
the full screen size is too small to align the contents in the center
and the bottom button with the button on the next screen.

BUG=721916
Review-Url: https://codereview.chromium.org/2914013002
Cr-Commit-Position: refs/heads/master@{#476326}
Committed: https://chromium.googlesource.com/chromium/src/+/8a05953afa5a5f242d4f00fc994a3f3acab185e3

[lody, 2017-05-31 16:24:05]

lod@chromium.org changed reviewers:
+ kkhorimoto@chromium.org

[lody, 2017-05-31 16:24:06]

Please review this change. See the linked bug for more info/ a screenshot of the
problem. Thanks!

[kkhorimoto, 2017-05-31 20:44:40]

https://codereview.chromium.org/2914013002/diff/20001/ios/chrome/browser/ui/f...
File ios/chrome/browser/ui/first_run/welcome_to_chrome_view.mm (right):

https://codereview.chromium.org/2914013002/diff/20001/ios/chrome/browser/ui/f...
ios/chrome/browser/ui/first_run/welcome_to_chrome_view.mm:454: // for the button
+ kOptInLabelPadding.
Optional: I feel like the wording here is a little clunky.  How about:

"...centered horizontally and vertically.  If necessary, it's shifted up to
allow |kOptInLabelPadding| between |optInLabel| and |OKButton|."

https://codereview.chromium.org/2914013002/diff/20001/ios/chrome/browser/ui/f...
ios/chrome/browser/ui/first_run/welcome_to_chrome_view.mm:458: CGFloat yAlign =
I'm not a fan of |yAlign| as a variable name here.  How about |originY|?

https://codereview.chromium.org/2914013002/diff/20001/ios/chrome/browser/ui/f...
ios/chrome/browser/ui/first_run/welcome_to_chrome_view.mm:465: }
This whole block of logic can be simplified a bit:

CGFloat padding = kOptInLabelPadding[self.cr_heightSizeClass];
CGFloat originY = std::min(
    (CGRectGetHeight(self.bounds) - containerViewSize.height) / 2.0,
    CGRectGetMinY(self.OKButton.frame) - padding- containerViewSize.height);

[lody, 2017-06-01 08:41:19]

https://codereview.chromium.org/2914013002/diff/20001/ios/chrome/browser/ui/f...
File ios/chrome/browser/ui/first_run/welcome_to_chrome_view.mm (right):

https://codereview.chromium.org/2914013002/diff/20001/ios/chrome/browser/ui/f...
ios/chrome/browser/ui/first_run/welcome_to_chrome_view.mm:454: // for the button
+ kOptInLabelPadding.
On 2017/05/31 20:44:40, kkhorimoto_ wrote:
> Optional: I feel like the wording here is a little clunky.  How about:
> 
> "...centered horizontally and vertically.  If necessary, it's shifted up to
> allow |kOptInLabelPadding| between |optInLabel| and |OKButton|."

Done.

https://codereview.chromium.org/2914013002/diff/20001/ios/chrome/browser/ui/f...
ios/chrome/browser/ui/first_run/welcome_to_chrome_view.mm:458: CGFloat yAlign =
On 2017/05/31 20:44:40, kkhorimoto_ wrote:
> I'm not a fan of |yAlign| as a variable name here.  How about |originY|?

Done.

https://codereview.chromium.org/2914013002/diff/20001/ios/chrome/browser/ui/f...
ios/chrome/browser/ui/first_run/welcome_to_chrome_view.mm:465: }
On 2017/05/31 20:44:39, kkhorimoto_ wrote:
> This whole block of logic can be simplified a bit:
> 
> CGFloat padding = kOptInLabelPadding[self.cr_heightSizeClass];
> CGFloat originY = std::min(
>     (CGRectGetHeight(self.bounds) - containerViewSize.height) / 2.0,
>     CGRectGetMinY(self.OKButton.frame) - padding- containerViewSize.height);

Good suggestion, that's way more readable!

[kkhorimoto, 2017-06-01 16:41:50]

lgtm, but can you add a test for this?  Just size the view to the iPhone 4
screen size and verify that the container view is shifted up properly.  Thanks!

[lody, 2017-06-01 16:45:27]

The CQ bit was checked by lod@chromium.org

[commit-bot: I haz the power, 2017-06-01 16:45:48]

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-01 16:56:31]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496335527374110,
"parent_rev": "5ad11b2bab309cdc5d11cc8825d957213b16094e", "commit_rev":
"8a05953afa5a5f242d4f00fc994a3f3acab185e3"}

[commit-bot: I haz the power, 2017-06-01 16:56:41]

Description was changed from

==========
Fix spacing on welcome screen for small screen (iPhone 4)

Shift the welcome screen contents up to allow for correct spacing when
the full screen size is too small to align the contents in the center
and the bottom button with the button on the next screen.

BUG=721916
==========

to

==========
Fix spacing on welcome screen for small screen (iPhone 4)

Shift the welcome screen contents up to allow for correct spacing when
the full screen size is too small to align the contents in the center
and the bottom button with the button on the next screen.

BUG=721916

Review-Url: https://codereview.chromium.org/2914013002
Cr-Commit-Position: refs/heads/master@{#476326}
Committed:
https://chromium.googlesource.com/chromium/src/+/8a05953afa5a5f242d4f00fc994a...
==========

[commit-bot: I haz the power, 2017-06-01 16:56:43]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/8a05953afa5a5f242d4f00fc994a...