New issue
Advanced search Search tips

Issue 850172 link

Starred by 1 user
Status: Fixed
Owner:
ftirelo@chromium.org
Closed: Jun 2018
Cc:
msw@chromium.org
ellyjo...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Consolidate horizontal margins for menus/autofill dropdown

Project Member Reported by ftirelo@chromium.org, Jun 6 2018 
As observed by msw@ in crrev.com/c/1088755, we have several constants in menu_config.cc that seem to make sense to be consolidated into a single horizontal_padding and touchable_horizontal_padding. Or maybe use LayoutProvider instead, which defines DistanceMetric::DISTANCE_RELATED_CONTROL_HORIZONTAL as 8.

We need to decide though what to do with constants that are platform dependent, such as label_to_arrow_padding that is set to 0 on Mac.

Opening this to discuss and track.
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 7 2018 

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

commit 4ea283bffe496b27dd41051c0348ff2600e0bab0
Author: Fabio Tirelo <ftirelo@chromium.org>
Date: Thu Jun 07 20:06:55 2018

Consolidate menu item horizontal paddings

Merge the following into item_horizontal_padding:
 - icon_to_label_padding
 - item_left_margin
 - label_to_minor_text_padding

Merge the following into touchable_item_horizontal_padding:
 - touchable_icon_to_label_padding
 - touchable_item_left_margin

Bug:  850172 
Change-Id: I3ae0207b1a1d332ac636b70a2f3c48817068ca20
Reviewed-on: https://chromium-review.googlesource.com/1089431
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565385}
[modify] https://crrev.com/4ea283bffe496b27dd41051c0348ff2600e0bab0/chrome/browser/ui/app_list/app_context_menu_unittest.cc
[modify] https://crrev.com/4ea283bffe496b27dd41051c0348ff2600e0bab0/chrome/browser/ui/views/autofill/autofill_popup_view_native_views.cc
[modify] https://crrev.com/4ea283bffe496b27dd41051c0348ff2600e0bab0/chrome/browser/ui/views/profiles/dice_accounts_menu.cc
[modify] https://crrev.com/4ea283bffe496b27dd41051c0348ff2600e0bab0/ui/views/controls/menu/menu_config.cc
[modify] https://crrev.com/4ea283bffe496b27dd41051c0348ff2600e0bab0/ui/views/controls/menu/menu_config.h
[modify] https://crrev.com/4ea283bffe496b27dd41051c0348ff2600e0bab0/ui/views/controls/menu/menu_item_view.cc
[modify] https://crrev.com/4ea283bffe496b27dd41051c0348ff2600e0bab0/ui/views/controls/menu/menu_item_view_unittest.cc
[modify] https://crrev.com/4ea283bffe496b27dd41051c0348ff2600e0bab0/ui/views/controls/menu/submenu_view.cc
[modify] https://crrev.com/4ea283bffe496b27dd41051c0348ff2600e0bab0/ui/views/layout/layout_provider.cc
[modify] https://crrev.com/4ea283bffe496b27dd41051c0348ff2600e0bab0/ui/views/layout/layout_provider.h

Comment 2 by ftirelo@chromium.org, Jun 11 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment