[tab_grid] New collection view layout.

[tab_grid] New collection view layout.

WIP collection view layout.

Screenshots for 5s, 6, 7, and ipad pro are available
in this folder:
https://drive.google.com/open?id=0BwS6YyQeisH5TFRYVFVZdW4tSms

BUG=686770
.

Review-Url: https://codereview.chromium.org/2702113002
Cr-Commit-Position: refs/heads/master@{#451943}
Committed: https://chromium.googlesource.com/chromium/src/+/13668983ecf62c2b171aedb5e64deeb5dd09c494

[edchin, 2017-02-20 00:28:19]

Description was changed from

==========
[tab_grid] New collection view layout.

BUG=
==========

to

==========
[tab_grid] New collection view layout.

WIP collection view layout.

Screenshots:
5s: https://screenshot.googleplex.com/MYnfzcGt50r.png
5s landscape: https://screenshot.googleplex.com/iGwHnm9marE.png
6: https://screenshot.googleplex.com/rrYhhSZjgH8.png
6 landscape: https://screenshot.googleplex.com/SgMqXzTiSmH.png
7+: https://screenshot.googleplex.com/J6z8eKOUh3c.png
7+ landscape: https://screenshot.googleplex.com/9GDXYOOAbmi.png
ipad pro: https://screenshot.googleplex.com/p9OpVHzVRC1.png
ipad pro landscape: https://screenshot.googleplex.com/CFUh67UZiJv.png

BUG=686770.
==========

[edchin, 2017-02-20 00:43:05]

Patchset #1 (id:1) has been deleted

[edchin, 2017-02-20 00:44:17]

edchin@chromium.org changed reviewers:
+ jif@chromium.org, lpromero@chromium.org, marq@chromium.org, sczs@chromium.org

[marq (ping after 24h), 2017-02-20 10:04:02]

On 2017/02/20 00:44:17, edchin wrote:

If you link to images/screenshots, please make sure they are visible to Chromium
accounts.

[marq (ping after 24h), 2017-02-20 10:09:47]

lgtm

LGTM % nits.

https://codereview.chromium.org/2702113002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tab_grid/tab_grid_collection_view_layout.h
(right):

https://codereview.chromium.org/2702113002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_grid/tab_grid_collection_view_layout.h:12: //
Generally speaking, compact size classes have 2 columns, and regular size
English style nit: Prefer to spell out single-digit numbers in most cases ("..
have two columns ... more than two columns ")

https://codereview.chromium.org/2702113002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tab_grid/tab_grid_collection_view_layout.mm
(right):

https://codereview.chromium.org/2702113002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_grid/tab_grid_collection_view_layout.mm:14:
const UIEdgeInsets kEdgeInsets = {20.0f, 20.0f, 20.0f, 20.0f};
kSectionInsets, clarifying role rather than type.

https://codereview.chromium.org/2702113002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_grid/tab_grid_collection_view_layout.mm:31: int
numColumns = static_cast<int>(floor(boundsSize.width - kInterTabSpacing) /
'columns' is a perfectly good name for this variable, and it might make your
line wrapping a bit easier.

https://codereview.chromium.org/2702113002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_grid/tab_grid_collection_view_layout.mm:35: if
(numColumns < 2 || tabWidth > 250.0f) {
Extract 250 into a constant and define its use.

[jif, 2017-02-20 10:53:21]

lgtm

[edchin, 2017-02-21 21:32:44]

Description was changed from

==========
[tab_grid] New collection view layout.

WIP collection view layout.

Screenshots:
5s: https://screenshot.googleplex.com/MYnfzcGt50r.png
5s landscape: https://screenshot.googleplex.com/iGwHnm9marE.png
6: https://screenshot.googleplex.com/rrYhhSZjgH8.png
6 landscape: https://screenshot.googleplex.com/SgMqXzTiSmH.png
7+: https://screenshot.googleplex.com/J6z8eKOUh3c.png
7+ landscape: https://screenshot.googleplex.com/9GDXYOOAbmi.png
ipad pro: https://screenshot.googleplex.com/p9OpVHzVRC1.png
ipad pro landscape: https://screenshot.googleplex.com/CFUh67UZiJv.png

BUG=686770.
==========

to

==========
[tab_grid] New collection view layout.

WIP collection view layout.

Screenshots for 5s, 6, 7, and ipad pro are available
in this folder:
https://drive.google.com/open?id=0BwS6YyQeisH5TFRYVFVZdW4tSms

BUG=686770.
==========

[edchin, 2017-02-21 22:29:07]

Patchset #2 (id:40001) has been deleted

[edchin, 2017-02-21 22:30:12]

https://codereview.chromium.org/2702113002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tab_grid/tab_grid_collection_view_layout.mm
(right):

https://codereview.chromium.org/2702113002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_grid/tab_grid_collection_view_layout.mm:14:
const UIEdgeInsets kEdgeInsets = {20.0f, 20.0f, 20.0f, 20.0f};
On 2017/02/20 10:09:47, marq wrote:
> kSectionInsets, clarifying role rather than type.

Done.

https://codereview.chromium.org/2702113002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_grid/tab_grid_collection_view_layout.mm:31: int
numColumns = static_cast<int>(floor(boundsSize.width - kInterTabSpacing) /
On 2017/02/20 10:09:47, marq wrote:
> 'columns' is a perfectly good name for this variable, and it might make your
> line wrapping a bit easier.

Done.

https://codereview.chromium.org/2702113002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_grid/tab_grid_collection_view_layout.mm:35: if
(numColumns < 2 || tabWidth > 250.0f) {
On 2017/02/20 10:09:47, marq wrote:
> Extract 250 into a constant and define its use.

Done.

[edchin, 2017-02-21 22:41:33]

The CQ bit was checked by edchin@chromium.org

[edchin, 2017-02-21 22:41:34]

The patchset sent to the CQ was uploaded after l-g-t-m from jif@chromium.org,
marq@chromium.org
Link to the patchset: https://codereview.chromium.org/2702113002/#ps80001
(title: "Rebase.")

[commit-bot: I haz the power, 2017-02-21 22:43:11]

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-02-22 00:47:24]

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

[commit-bot: I haz the power, 2017-02-22 00:47:25]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[edchin, 2017-02-22 07:23:36]

The CQ bit was checked by edchin@chromium.org

[commit-bot: I haz the power, 2017-02-22 07:25:08]

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-02-22 09:05:24]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487748216337710,
"parent_rev": "5ae55b8c8ed39e3a1019afba988b5df208f53420", "commit_rev":
"13668983ecf62c2b171aedb5e64deeb5dd09c494"}

[commit-bot: I haz the power, 2017-02-22 09:06:02]

Description was changed from

==========
[tab_grid] New collection view layout.

WIP collection view layout.

Screenshots for 5s, 6, 7, and ipad pro are available
in this folder:
https://drive.google.com/open?id=0BwS6YyQeisH5TFRYVFVZdW4tSms

BUG=686770.
==========

to

==========
[tab_grid] New collection view layout.

WIP collection view layout.

Screenshots for 5s, 6, 7, and ipad pro are available
in this folder:
https://drive.google.com/open?id=0BwS6YyQeisH5TFRYVFVZdW4tSms

BUG=686770.

Review-Url: https://codereview.chromium.org/2702113002
Cr-Commit-Position: refs/heads/master@{#451943}
Committed:
https://chromium.googlesource.com/chromium/src/+/13668983ecf62c2b171aedb5e64d...
==========

[commit-bot: I haz the power, 2017-02-22 09:06:03]

Committed patchset #3 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/13668983ecf62c2b171aedb5e64d...