New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 651641 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: 2017-09-29
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug
Launch-M-Target: 64-Beta

Blocking:
issue 630357
issue 177940


Participants' hotlists:
Harmony-Ready-For-Review


Sign in to add a comment

Harmony - update Zoom dialog [needs UX approval]

Project Member Reported by shrike@chromium.org, Sep 30 2016

Issue description

Mock attached.
 
P - Zoom 20170816.png
1002 KB View Download
S - Zoom 20170816.png
12.7 KB View Download

Comment 1 by shrike@chromium.org, Sep 30 2016

Labels: M-56
Status: Assigned (was: Untriaged)

Comment 2 by shrike@chromium.org, Sep 30 2016

Owner: shrike@chromium.org

Comment 4 by shrike@chromium.org, Sep 30 2016

Labels: Proj-HarmonyDialogs
Labels: -OS-Mac

Comment 6 by shrike@chromium.org, Oct 11 2016

Owner: pkasting@chromium.org
Owner: ----
Status: Available (was: Assigned)
Unassigning my Harmony bugs pending re-triage of who should own what.

Comment 8 by tapted@chromium.org, Apr 21 2017

Blocking: 177940

Comment 9 by varkha@chromium.org, Apr 26 2017

Cc: tapted@chromium.org ellyjo...@chromium.org pkasting@chromium.org
Owner: varkha@chromium.org
Status: Started (was: Available)
Draft at https://codereview.chromium.org/2845593002
Cc: tdander...@chromium.org bettes@chromium.org girard@chromium.org hwi@chromium.org
Labels: OS-Mac
Current mock this is based on is at:
https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Spec#%2FSPEC-secondary-UI-07a-layout.png

There was a discussion in the CL about what happens at 100% where the existing code would have hidden the zoom icon decorator in the location bar. I think it would make sense to keep the icon at 100% and change the iconography slightly to either always have +/- (otherwise it looks too much like a search) or alternatively, have +/- as accented badges on a magnifying glass.
This would benefit touchscreen users by making the zoom directly accessible even from a 100% state.

Attaching current looks from the CL and
+girard@, +tdanderson, +bettes@, +hwi@ for discussion.
zoom.png
6.9 KB View Download
The current visuals look good to me. 

Note that the current design expands the functionality of the bubble - it used to only let you reset to 100%, but now it's much more capable. With the expanded functionality, it doesn't make sense for the decorator to disappear at 100%. 

We've discussed the visual appearance of the decorator, because I agree that the "100%" mode might look like a "find in page" control. I had suggested that we change it to a "+/-" visual, possibly highlighting whichever was currently in effect.... but I suspect this wouldn't work given the sizing of the decorator. We might consider using a dot or star for "100%", or even sticking with the "+" since I suspect most users would go to this control when they want to zoom in. bettes@, wdyt?
My suggestion is to use '+' in this scenario (rather than nothing or a special icon), and consider just using '+' all the time rather than using a '-' in the case where the zoom level is below 100%.

Since the indicator doesn't show the actual zoom level, and users have to open the bubble to see that anyway, I don't think it's of much benefit to distinguish "zoomed out" versus "zoomed in" in the indicator -- and it might mislead some people into thinking that clicking a '-' indicator will cause it to zoom out more, which might not be what they want.  Just using a static indicator seems better IMO.
Project Member

Comment 13 by bugdroid1@chromium.org, May 18 2017

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

commit c4d839e6e1f0963b2760620cb0ed072b802bf225
Author: varkha <varkha@chromium.org>
Date: Thu May 18 00:22:04 2017

Updates Zoom bubble layout and adds +/- buttons

Adds + and - buttons to the Zoom bubble.
Updates layout to only have one row.

With this change the zoom icon (ZoomView) in location bar
can be shown even at 100% zoom as long as the zoom bubble
is also shown (otherwise the bubble loses its anchor when
using + / - buttons to go over 100% zoom without closing
the bubble.
In this scenario a kZoomPlusIcon is used (rather than none).
Whenever the zoom bubble gets closed the location bar gets
updated to update layout and visibility of the zoom icon.

BUG= 651641 
TEST=no new tests but a few existing tests updated or checked
    ZoomControllerTest
    ZoomDecorationTest
    ZoomControllerBrowserTest
    PDFExtensionTest.PdfZoomWithoutBubble
TBR=phajdan.jr@chromium.org (for a trivial change in chrome/test/base/test_browser_window.h)

Review-Url: https://codereview.chromium.org/2845593002
Cr-Commit-Position: refs/heads/master@{#472611}

[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/app/generated_resources.grd
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/app/vector_icons/BUILD.gn
[add] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/app/vector_icons/add.1x.icon
[add] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/app/vector_icons/add.icon
[add] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/app/vector_icons/remove.1x.icon
[add] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/app/vector_icons/remove.icon
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/browser/ui/cocoa/browser/zoom_bubble_controller.mm
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/browser/ui/cocoa/location_bar/zoom_decoration.mm
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/browser/ui/cocoa/location_bar/zoom_decoration_browsertest.mm
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/browser/ui/location_bar/location_bar.h
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/browser/ui/views/location_bar/zoom_bubble_view.h
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/browser/ui/views/location_bar/zoom_view.cc
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/browser/ui/zoom/zoom_controller_unittest.cc
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/chrome/test/base/test_browser_window.h
[modify] https://crrev.com/c4d839e6e1f0963b2760620cb0ed072b802bf225/components/zoom/zoom_controller.cc

Project Member

Comment 14 by bugdroid1@chromium.org, May 18 2017

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

commit ee42f00f402c8d2aff960d60079e77a94fc5aed5
Author: thestig <thestig@chromium.org>
Date: Thu May 18 02:06:34 2017

Revert of Updates Zoom bubble layout and adds +/- buttons (patchset #17 id:560001 of https://codereview.chromium.org/2845593002/ )

Reason for revert:
Broke "ChromiumOS x86-generic Compile" bot.

The bot uses gcc instead of clang, and needs more std::move()s.

Original issue's description:
> Updates Zoom bubble layout and adds +/- buttons
>
> Adds + and - buttons to the Zoom bubble.
> Updates layout to only have one row.
>
> With this change the zoom icon (ZoomView) in location bar
> can be shown even at 100% zoom as long as the zoom bubble
> is also shown (otherwise the bubble loses its anchor when
> using + / - buttons to go over 100% zoom without closing
> the bubble.
> In this scenario a kZoomPlusIcon is used (rather than none).
> Whenever the zoom bubble gets closed the location bar gets
> updated to update layout and visibility of the zoom icon.
>
> BUG= 651641 
> TEST=no new tests but a few existing tests updated or checked
>     ZoomControllerTest
>     ZoomDecorationTest
>     ZoomControllerBrowserTest
>     PDFExtensionTest.PdfZoomWithoutBubble
> TBR=phajdan.jr@chromium.org (for a trivial change in chrome/test/base/test_browser_window.h)
>
> Review-Url: https://codereview.chromium.org/2845593002
> Cr-Commit-Position: refs/heads/master@{#472611}
> Committed: https://chromium.googlesource.com/chromium/src/+/c4d839e6e1f0963b2760620cb0ed072b802bf225

TBR=estade@chromium.org,pkasting@chromium.org,tapted@chromium.org,wjmaclean@chromium.org,varkha@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 651641 

Review-Url: https://codereview.chromium.org/2891883002
Cr-Commit-Position: refs/heads/master@{#472625}

[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/app/generated_resources.grd
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/app/vector_icons/BUILD.gn
[delete] https://crrev.com/c295631f2dda9cc64b1b9becd12527c12efe8802/chrome/app/vector_icons/add.1x.icon
[delete] https://crrev.com/c295631f2dda9cc64b1b9becd12527c12efe8802/chrome/app/vector_icons/add.icon
[delete] https://crrev.com/c295631f2dda9cc64b1b9becd12527c12efe8802/chrome/app/vector_icons/remove.1x.icon
[delete] https://crrev.com/c295631f2dda9cc64b1b9becd12527c12efe8802/chrome/app/vector_icons/remove.icon
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/browser/ui/cocoa/browser/zoom_bubble_controller.mm
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/browser/ui/cocoa/location_bar/zoom_decoration.mm
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/browser/ui/cocoa/location_bar/zoom_decoration_browsertest.mm
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/browser/ui/location_bar/location_bar.h
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/browser/ui/views/location_bar/zoom_bubble_view.h
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/browser/ui/views/location_bar/zoom_view.cc
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/browser/ui/zoom/zoom_controller_unittest.cc
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/chrome/test/base/test_browser_window.h
[modify] https://crrev.com/ee42f00f402c8d2aff960d60079e77a94fc5aed5/components/zoom/zoom_controller.cc

Project Member

Comment 15 by bugdroid1@chromium.org, May 18 2017

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

commit f6f47b5a77854b023c3ccd6c57d1584051f2280a
Author: varkha <varkha@chromium.org>
Date: Thu May 18 18:55:10 2017

Updates Zoom bubble layout and adds +/- buttons

Adds + and - buttons to the Zoom bubble.
Updates layout to only have one row.

With this change the zoom icon (ZoomView) in location bar
can be shown even at 100% zoom as long as the zoom bubble
is also shown (otherwise the bubble loses its anchor when
using + / - buttons to go over 100% zoom without closing
the bubble.
In this scenario a kZoomPlusIcon is used (rather than none).
Whenever the zoom bubble gets closed the location bar gets
updated to update layout and visibility of the zoom icon.

BUG= 651641 
TEST=no new tests but a few existing tests updated or checked
    ZoomControllerTest
    ZoomDecorationTest
    ZoomControllerBrowserTest
    PDFExtensionTest.PdfZoomWithoutBubble
TBR=phajdan.jr@chromium.org (for a trivial change in chrome/test/base/test_browser_window.h)

Review-Url: https://codereview.chromium.org/2845593002
Cr-Original-Commit-Position: refs/heads/master@{#472611}
Committed: https://chromium.googlesource.com/chromium/src/+/c4d839e6e1f0963b2760620cb0ed072b802bf225
Review-Url: https://codereview.chromium.org/2845593002
Cr-Commit-Position: refs/heads/master@{#472891}

[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/app/generated_resources.grd
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/app/vector_icons/BUILD.gn
[add] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/app/vector_icons/add.1x.icon
[add] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/app/vector_icons/add.icon
[add] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/app/vector_icons/remove.1x.icon
[add] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/app/vector_icons/remove.icon
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/browser/ui/cocoa/browser/zoom_bubble_controller.mm
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/browser/ui/cocoa/location_bar/zoom_decoration.mm
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/browser/ui/cocoa/location_bar/zoom_decoration_browsertest.mm
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/browser/ui/location_bar/location_bar.h
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/browser/ui/views/location_bar/zoom_bubble_view.h
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/browser/ui/views/location_bar/zoom_view.cc
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/browser/ui/zoom/zoom_controller_unittest.cc
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/chrome/test/base/test_browser_window.h
[modify] https://crrev.com/f6f47b5a77854b023c3ccd6c57d1584051f2280a/components/zoom/zoom_controller.cc

See also  bug 725566 .
Status: Fixed (was: Started)
Labels: -M-56 M-60
Status: Assigned (was: Fixed)
Harmony dialog update bugs shouldn't be closed until we have formal UX signoff the that final implementation matches the latest mocks.

Once you've convinced yourself you're ready, make sure your dialog has a BrowserTest hook to show it from the command line, then send the relevant command line to bettes@ and hwi@ so they can test.
Status: Started (was: Assigned)
Thanks for the pointers. https://bugs.chromium.org/p/chromium/issues/detail?id=635173#c15 had enough info to see how it's done. I have a draft CL to enable the test - https://codereview.chromium.org/2963893002/. Will send it to designers once it lands.
Status: Assigned (was: Started)
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a857186d00beb74ffb1ebb6870d2ebdd351882e

commit 8a857186d00beb74ffb1ebb6870d2ebdd351882e
Author: varkha <varkha@chromium.org>
Date: Thu Jun 29 02:57:05 2017

[ui] Adds a browser_tests hook to show ZoomBubbleView

To show the dialog use:
browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive
 --dialog=ZoomBubbleDialogTest.InvokeDialog_default

BUG=  635173  

Review-Url: https://codereview.chromium.org/2963893002
Cr-Commit-Position: refs/heads/master@{#483270}

[modify] https://crrev.com/8a857186d00beb74ffb1ebb6870d2ebdd351882e/chrome/browser/ui/views/location_bar/zoom_bubble_view_browsertest.cc

The bubble can be shown with this command:

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive --dialog=ZoomBubbleDialogTest.InvokeDialog_default

bettes@, hwi@, could you please test this?
Owner: bettes@chromium.org
Labels: -M-60
Labels: -Pri-2 Pri-1
Summary: Harmony - update Zoom dialog [needs UX approval] (was: Harmony - update Zoom dialog)

Comment 27 Deleted

Feedback for Zoom

_All

- Corner radii looks to be 4dp, not 2
- We need to replace icon assets. Where are these coming from?
- New layout, specs and previews below

https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview/03-Zoom#%3Fz=width

_Win

- X-axis alignment is off by 1px
- Y-axis alignment is off by 1px

_Mac

- X-axis alignment is off by 2px
- Y-axis alignment is off by 2px

------------------------

All notes can be tracked in the implementation review deck: 
https://docs.google.com/presentation/d/1efIBdWozcOXv1hEaIxdcdf4A1-8P8HQu7bRpEokLisI/edit?userstoinvite=crivero@google.com&ts=5991f0cf#slide=id.g24441bd74f_3_0
Screen Shot 2017-08-15 at 12.59.05 PM.png
222 KB View Download
Description: Show this description
Description: Show this description
Owner: varkha@chromium.org
Status: Assigned (was: Available)
See comment 28 for specific review notes
Labels: Launch-M-Target-64-Beta
NextAction: 2017-09-29
Status: Started (was: Assigned)
I have a CL that will adjust the layout and update the icons. The corner radius seems right to me on Linux / Chrome OS builds as well as the alignment so this must be coming from elsewhere or have already been fixed separately.
zoom.png
6.0 KB View Download
Attaching the old 1x look and the new (square cap) looks for 1x, 1.25x, 1.33x and 2x.
mag-old-1.0.png
3.0 KB View Download
mag-new-1.0.png
3.1 KB View Download
mag-new-1.25.png
3.3 KB View Download
mag-new-1.33.png
3.5 KB View Download
mag-new-2.0.png
3.3 KB View Download
bettes@, please take a look at slight modification based on the comments that pkasting@ had on code review. He thought (and I agree) that it made sense to use the same logic as in FindBarView (Find in Page) such that the 16dp gap between the controls is counted between the apparent icons. So the spacing of ^ / v in Find toast should be same as - / + in the zoom toast. I find it visually looking better and requiring less mouse travel.
zoom-new.png
6.3 KB View Download
Cc: -tdander...@chromium.org bsep@chromium.org
+bsep@ for visibility in control spacing gaps in #37.
37 LGTM. 

12x2 bar for - 
12x2 + 2x12 cross for +
with sharp edges
Project Member

Comment 40 by bugdroid1@chromium.org, Sep 21 2017

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

commit 98a273dbd384790979baed706eb8907435654314
Author: Valery Arkhangorodsky <varkha@chromium.org>
Date: Thu Sep 21 22:05:07 2017

Modifies layout of the zoom bubble

Makes ZoomBubbleView follow the layout from the mock in the bug.
Fixes the vector icons (+ / -) to have sharp edges rather than using a
rounded stroke.

Bug:  651641 
Test: Visual (compare to mock)
Change-Id: I23bd9633037cf529e69f5b72ae9810ca5124da96
Reviewed-on: https://chromium-review.googlesource.com/673486
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Valery Arkhangorodsky <varkha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503578}
[modify] https://crrev.com/98a273dbd384790979baed706eb8907435654314/chrome/app/vector_icons/BUILD.gn
[delete] https://crrev.com/ba35577855190826b59b6795963d32cc4fdb86aa/chrome/app/vector_icons/add.1x.icon
[modify] https://crrev.com/98a273dbd384790979baed706eb8907435654314/chrome/app/vector_icons/add.icon
[delete] https://crrev.com/ba35577855190826b59b6795963d32cc4fdb86aa/chrome/app/vector_icons/remove.1x.icon
[modify] https://crrev.com/98a273dbd384790979baed706eb8907435654314/chrome/app/vector_icons/remove.icon
[modify] https://crrev.com/98a273dbd384790979baed706eb8907435654314/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc

Cc: ranjitkan@chromium.org rbasuvula@chromium.org nyerramilli@chromium.org msrchandra@chromium.org varkha@chromium.org
 Issue 767761  has been merged into this issue.
Owner: bettes@chromium.org
For reference this is how this dialog would look (Chrome OS build 1x) with 10 dp strokes on the +/- icons (instead of the current 12. I think it blends better with the find-in-page bubble and the windows controls.
zoom-10dp-icons.png
9.1 KB View Download
Sorry, these measure to 12 still. Am i missing something? 
Screen Shot 2017-09-22 at 11.23.01 AM.png
82.4 KB View Download
What about the one in #37?
Owner: varkha@chromium.org
#43, you are right. Missed the change in the last increment on that CL which changed the icon from 12x12 to 14x14. Should be easy to fix.
Project Member

Comment 46 by bugdroid1@chromium.org, Sep 22 2017

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

commit 046a0850924e5387fa7956116e27e037ab69a603
Author: Valery Arkhangorodsky <varkha@chromium.org>
Date: Fri Sep 22 20:20:17 2017

Corrects glyph size in + and - icons

Using CAP_SQUARE stroke in the icons implies (stroke_thickness/2)
endcaps. This CL corrects the icons to account for this end cap
thickness.

TBR=estade@chromium.org

Bug:  651641 
Test: Visual
Change-Id: Ib3759cb23218b9117fab981f82dfa77ec585c838
Reviewed-on: https://chromium-review.googlesource.com/679057
Reviewed-by: Valery Arkhangorodsky <varkha@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Valery Arkhangorodsky <varkha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503836}
[modify] https://crrev.com/046a0850924e5387fa7956116e27e037ab69a603/chrome/app/vector_icons/add.icon
[modify] https://crrev.com/046a0850924e5387fa7956116e27e037ab69a603/chrome/app/vector_icons/remove.icon

Adding it to Harmony-Ready-To-Review hotlist. Seems like the fix in #46 should reflect in tomorrow's canary. 
Owner: bettes@chromium.org
Status: Assigned (was: Started)
To bettes@ to confirm the looks in canary.
The NextAction date has arrived: 2017-09-29
Owner: varkha@chromium.org
Final edit here: 
All icons (+, -, and x) need to be converted from #5a5a5a to #757575 please.

Hopefully, the close-x icon being used here is used globally throughout Harmony 
(kClose16Icon)? The goal is to globally convert the close-x to #757575.

https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Spec#%2FSPEC-secondary-UI-00-stickersheet.png%3Ff=hidden

Screen Shot 2017-10-11 at 6.36.28 PM.png
206 KB View Download
Owner: bettes@chromium.org
#50, I've opened  bug 774563  to track the colors since I think it has much wider scope than the zoom dialog layout, please comment there. I can post an initial CL for this but I don't plan to actively work on it. I suggest we close this bug and track the remaining work in  bug 774563 .
Status: Fixed (was: Assigned)
Thanks! This bug LGTM. Marking as fixed.  

Sign in to add a comment