[MacViews-Browser] Inconsistent spacing in the new Settings Menu |
|||||||||||||||||
Issue descriptionChrome Version: Canary 67.0.3393.0 OS: macOS 10.12.6 What steps will reproduce the problem? (1) Click on the Settings Button on the Toolbar (2) Hover over the items (3) Take a look at the top/buttom spacings between dividers and hover What is the expected result? To see consistent spacing. What happens instead? Different spacing. Please use labels and text to provide additional information. Example screenshots are attached. Thanks Mehmet
,
Apr 16 2018
,
Apr 18 2018
I did a bunch of fiddling around here and figured out what's up. Attached screenshots: 8-top and 8-bottom are the current look, which has this vertical imbalance because of a bug in my original implementation. 8-new-top and 8-new-bottom are how it will look when I fix that bug I was also intrigued and experimented with 2pt everywhere - see 2-top and 2-bottom. It looks kind of nice I think.
,
Apr 18 2018
https://chromium-review.googlesource.com/c/chromium/src/+/1017063 goes from 8-{top,bottom} to 8-new-{top,bottom}, which looks way better. I don't think there's any need to look super hard at 2-{top,bottom} right now.
,
Apr 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d2cf70461093c265efbd23d7ff7a7b4431c66bd3 commit d2cf70461093c265efbd23d7ff7a7b4431c66bd3 Author: Elly Fong-Jones <ellyjones@chromium.org> Date: Wed Apr 18 21:18:31 2018 macviews: fix separator heights When I adjusted the separator height and thickness for material menus, I forgot to also adjust these constants. Set them to 9 - 1pt of line and 8pt of padding, since the full separators have 8pt on either side. The "spacing" separator is also 9 despite having no line, for visual consistency with the ones that do have lines. Bug: 831201 Change-Id: I14ed22823d7af9f6194281ef914ca64395feca31 Reviewed-on: https://chromium-review.googlesource.com/1017063 Reviewed-by: Leonard Grey <lgrey@chromium.org> Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#551825} [modify] https://crrev.com/d2cf70461093c265efbd23d7ff7a7b4431c66bd3/ui/views/controls/menu/menu_config_mac.mm
,
Apr 19 2018
Able to reproduce the issue on chrome reported version 67.0.3393.0 Verified the fix on Mac 10.12.6 on Chrome version #68.0.3400.0 as per the comment#0 Attaching screenshot for reference. Observed "Able to see consistent spacing" Hence, the fix is working as expected. Adding the verified label. Thanks!
,
Apr 19 2018
Thank you, spacing looks great now :-)
,
Apr 20 2018
8-new-top and 8-new-bottom LGTM! Woohoo! I appreciate the 2-top and 2-bottom. Because this spacing has ramifications across various UI surfaces, I'd like to hold off on any density refinements till we assess everything systematically.
,
Apr 23 2018
#8: Alright, cool. In that case this bug is Fixed :) |
|||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||
Comment 1 by ellyjo...@chromium.org
, Apr 11 2018Labels: M-68 MacViews-Browser Target-68
Owner: ellyjo...@chromium.org
Status: Assigned (was: Untriaged)