Commit 3ca3f90e authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

views: don't impose minimum sizes on comboboxes

It looks quite bad. This was an unintended regression from the fixed-width
look. Adding a test for this regression is unfortunately difficult - mocking
out the MenuController used by a single MenuItemView is complex to do.

Bug: 837158
Change-Id: Ie2e094a69c1d25bbcf8b53cfd66da4dec51e0f1b
Reviewed-on: https://chromium-review.googlesource.com/1030038
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554491}
parent 0fe9d9a3
......@@ -1097,6 +1097,10 @@ MenuItemView::MenuItemDimensions MenuItemView::CalculateDimensions() const {
}
void MenuItemView::ApplyMinimumDimensions(MenuItemDimensions* dims) const {
// Don't apply minimums to menus without controllers or to comboboxes.
if (!GetMenuController() || GetMenuController()->is_combobox())
return;
int used =
dims->standard_width + dims->children_width + dims->minor_text_width;
const MenuConfig& config = MenuConfig::instance();
......
......@@ -70,25 +70,16 @@ TEST(MenuItemViewUnitTest, TestMenuItemViewWithFlexibleWidthChild) {
ASSERT_EQ(flexible_view, submenu->GetMenuItemAt(1));
gfx::Size flexible_size = flexible_view->GetPreferredSize();
const MenuConfig& config = MenuConfig::instance();
if (config.minimum_menu_width)
EXPECT_EQ(flexible_size.width(), config.minimum_menu_width);
else
EXPECT_EQ(flexible_size.width(), 1);
if (!config.minimum_container_item_height) {
// ...but it should use whatever space is available to make a square.
int flex_height = flexible_view->GetHeightForWidth(label_size.width());
EXPECT_EQ(label_size.width(), flex_height);
// The submenu should be tall enough to allow for both menu items at the
// given width.
EXPECT_EQ(label_size.height() + flex_height,
submenu->GetPreferredSize().height());
} else {
EXPECT_EQ(flexible_size.height(), config.minimum_container_item_height);
}
EXPECT_EQ(1, flexible_size.width());
// ...but it should use whatever space is available to make a square.
int flex_height = flexible_view->GetHeightForWidth(label_size.width());
EXPECT_EQ(label_size.width(), flex_height);
// The submenu should be tall enough to allow for both menu items at the
// given width.
EXPECT_EQ(label_size.height() + flex_height,
submenu->GetPreferredSize().height());
}
// Tests that the top-level menu item with hidden children should contain the
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment