New issue
Advanced search Search tips

Issue 893529 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

Dynamic Type: Popup Menu's icons should be aligned with the center of the first line

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

Issue description

When opening the tools menu, the icon associated with an action should be aligned on the y-axis with the center of the first line.
 
Simulator Screen Shot - iPhone X - 2018-10-09 at 13.22.17.png
330 KB View Download
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 10

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

commit b181c1c7dbae73e90ea8677cbb5ac95bdac8f7a2
Author: Gauthier Ambard <gambard@chromium.org>
Date: Wed Oct 10 13:30:30 2018

[iOS] Adjust icon y-position in the popup menus

This CL adjust the position of the icons and the badges in the popup
menus so they are aligned with the first line of the action item when
the font size increases.

Bug:  893529 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: Ief6b938fb1a27e3b71471484b632d83e870a70c7
Reviewed-on: https://chromium-review.googlesource.com/c/1270999
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598304}
[modify] https://crrev.com/b181c1c7dbae73e90ea8677cbb5ac95bdac8f7a2/ios/chrome/browser/ui/popup_menu/cells/popup_menu_tools_item.mm

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 10

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

commit d2b8fbfcaa72c5b95dc9c9a5aec247b9a40c9333
Author: Gauthier Ambard <gambard@chromium.org>
Date: Wed Oct 10 13:39:48 2018

Revert "[iOS] Adjust icon y-position in the popup menus"

This reverts commit b181c1c7dbae73e90ea8677cbb5ac95bdac8f7a2.

Reason for revert: Breaks the compilation. Not sure about how it got through the CQ.

Original change's description:
> [iOS] Adjust icon y-position in the popup menus
> 
> This CL adjust the position of the icons and the badges in the popup
> menus so they are aligned with the first line of the action item when
> the font size increases.
> 
> Bug:  893529 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
> Change-Id: Ief6b938fb1a27e3b71471484b632d83e870a70c7
> Reviewed-on: https://chromium-review.googlesource.com/c/1270999
> Commit-Queue: Gauthier Ambard <gambard@chromium.org>
> Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598304}

TBR=olivierrobin@chromium.org,gambard@chromium.org

Change-Id: I8e5697b83f45f9a25b50bdd87dfaf49620214c2e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  893529 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/c/1273519
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598305}
[modify] https://crrev.com/d2b8fbfcaa72c5b95dc9c9a5aec247b9a40c9333/ios/chrome/browser/ui/popup_menu/cells/popup_menu_tools_item.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 10

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

commit dd63f41b2606031e35bb6fb239731f662c562171
Author: Gauthier Ambard <gambard@chromium.org>
Date: Wed Oct 10 15:57:36 2018

Reland "[iOS] Adjust icon y-position in the popup menus"

This is a reland of b181c1c7dbae73e90ea8677cbb5ac95bdac8f7a2
which got reverted because of a wrong rebase.
crrev.com/c/1270956 has now landed, so it is safe to reland this.

Original change's description:
> [iOS] Adjust icon y-position in the popup menus
>
> This CL adjust the position of the icons and the badges in the popup
> menus so they are aligned with the first line of the action item when
> the font size increases.
>
> Bug:  893529 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
> Change-Id: Ief6b938fb1a27e3b71471484b632d83e870a70c7
> Reviewed-on: https://chromium-review.googlesource.com/c/1270999
> Commit-Queue: Gauthier Ambard <gambard@chromium.org>
> Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598304}

Bug:  893529 
Change-Id: Iaa7a96ea7f678be790adc46fb2382fb9be86f97f
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/c/1273149
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598345}
[modify] https://crrev.com/dd63f41b2606031e35bb6fb239731f662c562171/ios/chrome/browser/ui/popup_menu/cells/popup_menu_tools_item.mm

Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
Verified on 72.0.3582.0 Canary, iPhone X  iOS 11.4.1
Looks good.

https://drive.google.com/file/d/19N6esQc9-67GlT-K5gQg_-mYiSUk47nP/view
Verified on 71.0.3578.9 Beta in iPhone X(iOS 11.4.1), iPhone 8plus(iOS 12.1 beta) and iPad Pro(iOS 12.0.1)

The position of the icons in the popup menus are aligned with the first line of the item when the font size increases, looks good.


Sign in to add a comment