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

Issue 653292 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Feature

Blocking:
issue 660928



Sign in to add a comment

Update the padding between icons in the Ash material design system tray

Project Member Reported by yiyix@chromium.org, Oct 5 2016

Issue description

The items in the tray area of the shelf is separated by a fixed padding which does not take the internal icon padding into account. As a result, some tray items seem to be closer and some seem to be further away (Please refer to the picture for details, please ignore the clock digits are shown in black.)




 
Screenshot from 2016-10-05 18:33:19.png
6.0 KB View Download

Comment 1 by yiyix@chromium.org, Oct 5 2016

Description: Show this description
Cc: sgabr...@chromium.org
Labels: -Pri-3 M-56 Pri-2
Yi, did you get a chance to look at how the internal icon padding could be taken into account when calculating spacing? If not, no worries.

+Sebastien as FYI. Please take a look at the updated icon spacing in an upcoming canary and let us know your thoughts.
Owner: yiyix@chromium.org
Status: Assigned (was: Untriaged)
The implementation is close to perfect. 
One thing to fix, the first icon from the right (accessibility in the preview you sent me on Friday) is too much to the right by 2dp, shifting the entire row.

See image attached. top 2 status tray are my spec, bottom is your screenshot.
comparison.png
51.5 KB View Download
Owner: tdander...@chromium.org
When looking more into this, there seems to be something wrong with how we build the padding around the icons. As you can see in the example attached (spec top, implementation bottom) based on today's build, IME is too far (while accessibility was too close in #3), battery and Wifi are too far apart. It seems like not all icon are based on a 16*16 square, creating various spacing.


spacing.png
337 KB View Download
Labels: -Pri-2 Pri-1
Owner: est...@chromium.org
Summary: Update the padding between icons in the Ash material design system tray (was: Update the tray padding beside each items by using the internal icon padding.)
Evan, mind taking a look into these spacing irregularities?
Another one, this time around the screen sharing icon. Top is implementation, bottom is what's specced.


spacing.png
51.6 KB View Download
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 28 2016

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

commit 2449770edb6a482bf442bcdb669cd15d5a2f2632
Author: estade <estade@chromium.org>
Date: Fri Oct 28 19:10:00 2016

Simplify layout of tray items and fix spacing for MD.

BUG= 653292 

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

[modify] https://crrev.com/2449770edb6a482bf442bcdb669cd15d5a2f2632/ash/common/system/chromeos/network/tray_network.cc
[modify] https://crrev.com/2449770edb6a482bf442bcdb669cd15d5a2f2632/ash/common/system/chromeos/power/tray_power.cc
[modify] https://crrev.com/2449770edb6a482bf442bcdb669cd15d5a2f2632/ash/common/system/date/date_view.cc
[modify] https://crrev.com/2449770edb6a482bf442bcdb669cd15d5a2f2632/ash/common/system/date/tray_date.h
[modify] https://crrev.com/2449770edb6a482bf442bcdb669cd15d5a2f2632/ash/common/system/tray/fixed_sized_image_view.cc
[modify] https://crrev.com/2449770edb6a482bf442bcdb669cd15d5a2f2632/ash/common/system/tray/system_tray.cc
[modify] https://crrev.com/2449770edb6a482bf442bcdb669cd15d5a2f2632/ash/common/system/tray/tray_image_item.cc
[modify] https://crrev.com/2449770edb6a482bf442bcdb669cd15d5a2f2632/ash/common/system/tray/tray_image_item.h
[modify] https://crrev.com/2449770edb6a482bf442bcdb669cd15d5a2f2632/ash/common/system/tray/tray_item_view.cc
[modify] https://crrev.com/2449770edb6a482bf442bcdb669cd15d5a2f2632/ash/common/system/tray/tray_item_view.h
[modify] https://crrev.com/2449770edb6a482bf442bcdb669cd15d5a2f2632/ash/common/system/tray/tray_utils.cc

Comment 9 by est...@chromium.org, Oct 28 2016

Status: Fixed (was: Assigned)
I believe this is now to spec. Sebastien can confirm or deny when it hits canary.
Status: Assigned (was: Fixed)
On ToT I see extra padding to the left of the time and to the right of the profile picture (attached), which I think is from your above change. Can you please take a look?
system-tray.png
4.4 KB View Download
I'm not seeing the extra padding to the left of the time. I do see the extra space right of the user icon. Do you have any other changes on your tot?
Sorry, I think I must have had some local changes in that screenshot. Just tried again on ToT and am seeing the following (@1x and @2x attached). Seems like it's just the extra padding to the right.
oct28-1x.png
28.0 KB View Download
oct28-2x.png
45.5 KB View Download
Blocking: 660928
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 2 2016

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

commit 4b338bc886f384d287b2e11fc913016301ed6761
Author: estade <estade@chromium.org>
Date: Wed Nov 02 00:36:49 2016

Some more fixes to cros system tray icons (esp. screen tray items)

More things should use TrayItemView (and not override its layout manager)

BUG= 653292 

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

[modify] https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761/ash/common/system/cast/tray_cast.cc
[modify] https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761/ash/common/system/cast/tray_cast.h
[modify] https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761/ash/common/system/chromeos/media_security/multi_profile_media_tray_item.cc
[modify] https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761/ash/common/system/chromeos/screen_security/screen_capture_tray_item.cc
[modify] https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761/ash/common/system/chromeos/screen_security/screen_capture_tray_item.h
[modify] https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761/ash/common/system/chromeos/screen_security/screen_share_tray_item.cc
[modify] https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761/ash/common/system/chromeos/screen_security/screen_share_tray_item.h
[modify] https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761/ash/common/system/chromeos/screen_security/screen_tray_item.cc
[modify] https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761/ash/common/system/chromeos/screen_security/screen_tray_item.h
[modify] https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761/ash/common/system/tray/tray_item_view.cc
[modify] https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761/ash/common/system/tray/tray_item_view.h
[modify] https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761/ash/common/system/user/tray_user_separator.h

Status: Fixed (was: Assigned)
I believe this to be fixed.
Project Member

Comment 18 by bugdroid1@chromium.org, Nov 16 2016

Status: Verified (was: Fixed)

Sign in to add a comment