New issue
Advanced search Search tips

Issue 893540 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

Dynamic Type: Improve TabGrid support for biggest content size

Project Member Reported by gambard@chromium.org, Oct 9

Issue description

When the user is choosing an "accessibility" preferred content size, the TabGrid isn't supporting it very well.

On the attached screenshot, you can see that the Title of the opened tab is unreadable.
Discussing it with martijn@, he suggested that:
The number of tabs we are displaying on one line should be reduced (i.e. 1 per line on phone), increasing the width and height of the tab screenshot.
The tab's title should be displayed on two lines (so we would need to increase the height of the toolbar displaying the tab's title).
The favicon should be removed.

All this only apply for the "accessibility" categories. For the other categories, the current behavior is working well.

How feasible this is in you opinion, given the current implementation?
 
Simulator Screen Shot - iPhone SE - 2018-10-09 at 13.36.42.png
78.9 KB View Download
I like this solution. This is straight-forward but time consuming.
Yeah, I think it is the most straightforward solution. I am doing it for the NTP, it is usually faster and easier than what I anticipated.

Changing the number of line for the title and removing the favicon is easy.
I am more concerned about increasing the size/height and changing the number of tabs per line. Do you have any idea how hard it is?
It is modifying the collection view layout, not difficult.
Does the in/out animations will work nicely if the size of the header is changing?
Mark, any thoughts on this? I can imagine two possible implementations when the header is higher:

- The higher header start to overlap the thumbnail snapshot, cropping the top part of the snap. Not ideal I would say, but perhaps significantly easier to implement. 

- A higher header that pushes the thumbnail snapshot down. Cropping the bottom part of the snap. I imagine this might effect the matching of the thumbnail snap with the tab, since the snap would have to cover more vertical space.  
Owner: mrsuyi@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db29d9f8a658ec762c375a3dec48e6126b59acbd

commit db29d9f8a658ec762c375a3dec48e6126b59acbd
Author: Yi Su <mrsuyi@chromium.org>
Date: Thu Jan 10 11:24:53 2019

Hide favicon in top bar of TabGrid thumbnail under accessibility font
size.

This CL hides the favicon in the top bar of TabGrid thumbnail under
accessibility font size.

Bug: 893540
Change-Id: I0fa632e73537e78598ecc22432a1b25e8135b5d8
Reviewed-on: https://chromium-review.googlesource.com/c/1403415
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Reviewed-by: edchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621543}
[modify] https://crrev.com/db29d9f8a658ec762c375a3dec48e6126b59acbd/ios/chrome/browser/ui/tab_grid/grid/grid_cell.mm

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 16 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/845594fab3d16176e416a25a65b9b9d5490effef

commit 845594fab3d16176e416a25a65b9b9d5490effef
Author: Yi Su <mrsuyi@chromium.org>
Date: Wed Jan 16 09:27:38 2019

Use bigger tab view under accessibility font size.

This CL adjusts the size of tab views and height of their titles in tab
grid under accessibility font size. Title will be displayed in 2 lines
when accessibility font size is chosen.

Screen shots:
iPhoneXR:   https://drive.google.com/open?id=1anVBsA3cOBumUqt38OpM6FtprLBFfup5

iPad landscape:
50%:  https://drive.google.com/open?id=1E9m9Viqhval_l9Est8U1-sQktI7c_38l
66%:  https://drive.google.com/open?id=1GuXD-miJgBPL3FnhlSRLoDrKMFa9xv0w
100%: https://drive.google.com/open?id=1jiAutUdf-wcACcckxtFbJFmS_S-2f7t2

iPad Portrait:
40%:  https://drive.google.com/open?id=1A9WsE-_vT0kdzuAhEDNQH-UQtVqLGF53
60%:  https://drive.google.com/open?id=1CR-holR-9JcuvMBjadrzWjcoDFuaJU4i
100%: https://drive.google.com/open?id=1k8BCFtqKavozdpoihA7tYg3PT7ho7Eo7

Bug: 893540
Change-Id: I314d081439909cf6f3711d06df579c73a9685e99
Reviewed-on: https://chromium-review.googlesource.com/c/1409546
Reviewed-by: edchin <edchin@chromium.org>
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623172}
[modify] https://crrev.com/845594fab3d16176e416a25a65b9b9d5490effef/ios/chrome/browser/ui/tab_grid/grid/grid_cell.mm
[modify] https://crrev.com/845594fab3d16176e416a25a65b9b9d5490effef/ios/chrome/browser/ui/tab_grid/grid/grid_constants.h
[modify] https://crrev.com/845594fab3d16176e416a25a65b9b9d5490effef/ios/chrome/browser/ui/tab_grid/grid/grid_constants.mm
[modify] https://crrev.com/845594fab3d16176e416a25a65b9b9d5490effef/ios/chrome/browser/ui/tab_grid/grid/grid_layout.mm
[modify] https://crrev.com/845594fab3d16176e416a25a65b9b9d5490effef/ios/chrome/browser/ui/tab_grid/grid/grid_view_controller.mm

Comment 9 by mrsuyi@chromium.org, Jan 16 (6 days ago)

Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 18 (5 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7eaa39c9fd92d0130726f09ef1c116c8bf713500

commit 7eaa39c9fd92d0130726f09ef1c116c8bf713500
Author: Justin Cohen <justincohen@chromium.org>
Date: Fri Jan 18 00:09:06 2019

Revert "Use bigger tab view under accessibility font size."

This reverts commit 845594fab3d16176e416a25a65b9b9d5490effef.

Reason for revert: Breaks NTP animation

Bug:  923109 , 893540

Original change's description:
> Use bigger tab view under accessibility font size.
> 
> This CL adjusts the size of tab views and height of their titles in tab
> grid under accessibility font size. Title will be displayed in 2 lines
> when accessibility font size is chosen.
> 
> Screen shots:
> iPhoneXR:   https://drive.google.com/open?id=1anVBsA3cOBumUqt38OpM6FtprLBFfup5
> 
> iPad landscape:
> 50%:  https://drive.google.com/open?id=1E9m9Viqhval_l9Est8U1-sQktI7c_38l
> 66%:  https://drive.google.com/open?id=1GuXD-miJgBPL3FnhlSRLoDrKMFa9xv0w
> 100%: https://drive.google.com/open?id=1jiAutUdf-wcACcckxtFbJFmS_S-2f7t2
> 
> iPad Portrait:
> 40%:  https://drive.google.com/open?id=1A9WsE-_vT0kdzuAhEDNQH-UQtVqLGF53
> 60%:  https://drive.google.com/open?id=1CR-holR-9JcuvMBjadrzWjcoDFuaJU4i
> 100%: https://drive.google.com/open?id=1k8BCFtqKavozdpoihA7tYg3PT7ho7Eo7
> 
> Bug: 893540
> Change-Id: I314d081439909cf6f3711d06df579c73a9685e99
> Reviewed-on: https://chromium-review.googlesource.com/c/1409546
> Reviewed-by: edchin <edchin@chromium.org>
> Commit-Queue: Yi Su <mrsuyi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#623172}

TBR=edchin@chromium.org,mrsuyi@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 893540
Change-Id: I9255619f7a49adb4fc7b2326a5cb288d3be26725
Reviewed-on: https://chromium-review.googlesource.com/c/1419258
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: edchin <edchin@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623923}
[modify] https://crrev.com/7eaa39c9fd92d0130726f09ef1c116c8bf713500/ios/chrome/browser/ui/tab_grid/grid/grid_cell.mm
[modify] https://crrev.com/7eaa39c9fd92d0130726f09ef1c116c8bf713500/ios/chrome/browser/ui/tab_grid/grid/grid_constants.h
[modify] https://crrev.com/7eaa39c9fd92d0130726f09ef1c116c8bf713500/ios/chrome/browser/ui/tab_grid/grid/grid_constants.mm
[modify] https://crrev.com/7eaa39c9fd92d0130726f09ef1c116c8bf713500/ios/chrome/browser/ui/tab_grid/grid/grid_layout.mm
[modify] https://crrev.com/7eaa39c9fd92d0130726f09ef1c116c8bf713500/ios/chrome/browser/ui/tab_grid/grid/grid_view_controller.mm

Comment 11 by srikanthg@chromium.org, Jan 18 (5 days ago)

Status: Assigned (was: Fixed)
Reopening as the fix from comment#8 is reverted in comment#10.
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 18 (4 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/19dec234c20e988961baa980f1ffacf05fe436ed

commit 19dec234c20e988961baa980f1ffacf05fe436ed
Author: Yi Su <mrsuyi@chromium.org>
Date: Fri Jan 18 10:25:40 2019

Reland "Use bigger tab view under accessibility font size."

This is a reland of 845594fab3d16176e416a25a65b9b9d5490effef

Original change's description:
> Use bigger tab view under accessibility font size.
> 
> This CL adjusts the size of tab views and height of their titles in tab
> grid under accessibility font size. Title will be displayed in 2 lines
> when accessibility font size is chosen.
> 
> Screen shots:
> iPhoneXR:   https://drive.google.com/open?id=1anVBsA3cOBumUqt38OpM6FtprLBFfup5
> 
> iPad landscape:
> 50%:  https://drive.google.com/open?id=1E9m9Viqhval_l9Est8U1-sQktI7c_38l
> 66%:  https://drive.google.com/open?id=1GuXD-miJgBPL3FnhlSRLoDrKMFa9xv0w
> 100%: https://drive.google.com/open?id=1jiAutUdf-wcACcckxtFbJFmS_S-2f7t2
> 
> iPad Portrait:
> 40%:  https://drive.google.com/open?id=1A9WsE-_vT0kdzuAhEDNQH-UQtVqLGF53
> 60%:  https://drive.google.com/open?id=1CR-holR-9JcuvMBjadrzWjcoDFuaJU4i
> 100%: https://drive.google.com/open?id=1k8BCFtqKavozdpoihA7tYg3PT7ho7Eo7
> 
> Bug: 893540
> Change-Id: I314d081439909cf6f3711d06df579c73a9685e99
> Reviewed-on: https://chromium-review.googlesource.com/c/1409546
> Reviewed-by: edchin <edchin@chromium.org>
> Commit-Queue: Yi Su <mrsuyi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#623172}

Bug: 893540
Change-Id: Idfdd3a9cee1fce3a7dc092ba967f9be3859edee5
Reviewed-on: https://chromium-review.googlesource.com/c/1419842
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624058}
[modify] https://crrev.com/19dec234c20e988961baa980f1ffacf05fe436ed/ios/chrome/browser/ui/tab_grid/grid/grid_cell.mm
[modify] https://crrev.com/19dec234c20e988961baa980f1ffacf05fe436ed/ios/chrome/browser/ui/tab_grid/grid/grid_constants.h
[modify] https://crrev.com/19dec234c20e988961baa980f1ffacf05fe436ed/ios/chrome/browser/ui/tab_grid/grid/grid_constants.mm
[modify] https://crrev.com/19dec234c20e988961baa980f1ffacf05fe436ed/ios/chrome/browser/ui/tab_grid/grid/grid_layout.mm
[modify] https://crrev.com/19dec234c20e988961baa980f1ffacf05fe436ed/ios/chrome/browser/ui/tab_grid/grid/grid_view_controller.mm

Sign in to add a comment