Commit 2b9a46a9 authored by spqchan's avatar spqchan Committed by Commit Bot

[MacViews] Fix for incorrect extension padding

Move the extensions 2pt to the right on Mac to match the native Mac
placements.

Bug: 792593
Change-Id: I579520a37043667dfa289b110287c034447c3812
Reviewed-on: https://chromium-review.googlesource.com/982353Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546536}
parent f2512cc3
...@@ -91,6 +91,12 @@ int GetLayoutConstant(LayoutConstant constant) { ...@@ -91,6 +91,12 @@ int GetLayoutConstant(LayoutConstant constant) {
return touch_optimized_material ? 68 : 120; return touch_optimized_material ? 68 : 120;
case TAB_STANDARD_WIDTH: case TAB_STANDARD_WIDTH:
return touch_optimized_material ? 245 : 193; return touch_optimized_material ? 245 : 193;
case TOOLBAR_ACTION_LEFT_PADDING:
#if defined(OS_MACOSX)
return 2;
#else
return 0;
#endif
case TOOLBAR_ELEMENT_PADDING: case TOOLBAR_ELEMENT_PADDING:
return hybrid ? 8 : 0; return hybrid ? 8 : 0;
case TOOLBAR_STANDARD_SPACING: { case TOOLBAR_STANDARD_SPACING: {
......
...@@ -98,6 +98,10 @@ enum LayoutConstant { ...@@ -98,6 +98,10 @@ enum LayoutConstant {
// one side) // one side)
TAB_STANDARD_WIDTH, TAB_STANDARD_WIDTH,
// Additional left horizontal padding between the browser actions and the
// omnibox.
TOOLBAR_ACTION_LEFT_PADDING,
// Additional horizontal padding between the elements in the toolbar. // Additional horizontal padding between the elements in the toolbar.
TOOLBAR_ELEMENT_PADDING, TOOLBAR_ELEMENT_PADDING,
......
...@@ -88,6 +88,7 @@ bool ToolbarActionsBar::disable_animations_for_testing_ = false; ...@@ -88,6 +88,7 @@ bool ToolbarActionsBar::disable_animations_for_testing_ = false;
ToolbarActionsBar::PlatformSettings::PlatformSettings() ToolbarActionsBar::PlatformSettings::PlatformSettings()
: item_spacing(GetLayoutConstant(TOOLBAR_STANDARD_SPACING)), : item_spacing(GetLayoutConstant(TOOLBAR_STANDARD_SPACING)),
left_padding(GetLayoutConstant(TOOLBAR_ACTION_LEFT_PADDING)),
icons_per_overflow_menu_row(1) {} icons_per_overflow_menu_row(1) {}
ToolbarActionsBar::ToolbarActionsBar(ToolbarActionsBarDelegate* delegate, ToolbarActionsBar::ToolbarActionsBar(ToolbarActionsBarDelegate* delegate,
...@@ -167,7 +168,9 @@ gfx::Size ToolbarActionsBar::GetFullSize() const { ...@@ -167,7 +168,9 @@ gfx::Size ToolbarActionsBar::GetFullSize() const {
num_rows += (std::max(0, icon_count - 1) / num_icons); num_rows += (std::max(0, icon_count - 1) / num_icons);
} }
return gfx::ScaleToFlooredSize(GetViewSize(), num_icons, num_rows); gfx::Size size = gfx::ScaleToFlooredSize(GetViewSize(), num_icons, num_rows);
size.Enlarge(platform_settings_.left_padding, 0);
return size;
} }
int ToolbarActionsBar::GetMinimumWidth() const { int ToolbarActionsBar::GetMinimumWidth() const {
...@@ -175,7 +178,8 @@ int ToolbarActionsBar::GetMinimumWidth() const { ...@@ -175,7 +178,8 @@ int ToolbarActionsBar::GetMinimumWidth() const {
} }
int ToolbarActionsBar::GetMaximumWidth() const { int ToolbarActionsBar::GetMaximumWidth() const {
return IconCountToWidth(toolbar_actions_.size()); return IconCountToWidth(toolbar_actions_.size()) +
platform_settings_.left_padding;
} }
int ToolbarActionsBar::IconCountToWidth(size_t icons) const { int ToolbarActionsBar::IconCountToWidth(size_t icons) const {
...@@ -183,7 +187,8 @@ int ToolbarActionsBar::IconCountToWidth(size_t icons) const { ...@@ -183,7 +187,8 @@ int ToolbarActionsBar::IconCountToWidth(size_t icons) const {
} }
size_t ToolbarActionsBar::WidthToIconCount(int pixels) const { size_t ToolbarActionsBar::WidthToIconCount(int pixels) const {
return base::ClampToRange(pixels / GetViewSize().width(), 0, int available_width = pixels - platform_settings_.left_padding;
return base::ClampToRange(available_width / GetViewSize().width(), 0,
static_cast<int>(toolbar_actions_.size())); static_cast<int>(toolbar_actions_.size()));
} }
...@@ -275,7 +280,9 @@ gfx::Rect ToolbarActionsBar::GetFrameForIndex( ...@@ -275,7 +280,9 @@ gfx::Rect ToolbarActionsBar::GetFrameForIndex(
const auto size = GetViewSize(); const auto size = GetViewSize();
return gfx::Rect( return gfx::Rect(
gfx::Point(index_in_row * size.width(), row_index * size.height()), size); gfx::Point(index_in_row * size.width() + platform_settings_.left_padding,
row_index * size.height()),
size);
} }
std::vector<ToolbarActionViewController*> std::vector<ToolbarActionViewController*>
......
...@@ -58,6 +58,10 @@ class ToolbarActionsBar : public ToolbarActionsModel::Observer, ...@@ -58,6 +58,10 @@ class ToolbarActionsBar : public ToolbarActionsModel::Observer,
// container and the first item, and between the last item and end of // container and the first item, and between the last item and end of
// the container. // the container.
int item_spacing; int item_spacing;
// The additional spacing between the container and omnibox.
int left_padding;
// The number of icons per row in the overflow menu. // The number of icons per row in the overflow menu.
int icons_per_overflow_menu_row; int icons_per_overflow_menu_row;
}; };
......
...@@ -254,8 +254,9 @@ TEST_P(ToolbarActionsBarUnitTest, BasicToolbarActionsBarTest) { ...@@ -254,8 +254,9 @@ TEST_P(ToolbarActionsBarUnitTest, BasicToolbarActionsBarTest) {
// By default, all three actions should be visible. // By default, all three actions should be visible.
EXPECT_EQ(3u, toolbar_actions_bar()->GetIconCount()); EXPECT_EQ(3u, toolbar_actions_bar()->GetIconCount());
const gfx::Size view_size = ToolbarActionsBar::GetViewSize(); const gfx::Size view_size = ToolbarActionsBar::GetViewSize();
// Check the widths. // Check the widths.
int expected_width = 3 * view_size.width(); int expected_width = 3 * view_size.width() + platform_settings.left_padding;
EXPECT_EQ(expected_width, toolbar_actions_bar()->GetFullSize().width()); EXPECT_EQ(expected_width, toolbar_actions_bar()->GetFullSize().width());
// Since all icons are showing, the current width should be the max width. // Since all icons are showing, the current width should be the max width.
int maximum_width = expected_width; int maximum_width = expected_width;
...@@ -271,7 +272,7 @@ TEST_P(ToolbarActionsBarUnitTest, BasicToolbarActionsBarTest) { ...@@ -271,7 +272,7 @@ TEST_P(ToolbarActionsBarUnitTest, BasicToolbarActionsBarTest) {
EXPECT_EQ(2u, toolbar_actions_bar()->GetIconCount()); EXPECT_EQ(2u, toolbar_actions_bar()->GetIconCount());
// The current width should now be enough for two icons. // The current width should now be enough for two icons.
expected_width = 2 * view_size.width(); expected_width = 2 * view_size.width() + platform_settings.left_padding;
EXPECT_EQ(expected_width, toolbar_actions_bar()->GetFullSize().width()); EXPECT_EQ(expected_width, toolbar_actions_bar()->GetFullSize().width());
// The maximum and minimum widths should have remained constant (since we have // The maximum and minimum widths should have remained constant (since we have
// the same number of actions). // the same number of actions).
...@@ -443,7 +444,11 @@ TEST_P(ToolbarActionsBarUnitTest, TestHighlightMode) { ...@@ -443,7 +444,11 @@ TEST_P(ToolbarActionsBarUnitTest, TestHighlightMode) {
TEST_P(ToolbarActionsBarUnitTest, TestActionFrameBounds) { TEST_P(ToolbarActionsBarUnitTest, TestActionFrameBounds) {
const auto icon_rect = [](int x, int y) { const auto icon_rect = [](int x, int y) {
const auto size = ToolbarActionsBar::GetViewSize(); const auto size = ToolbarActionsBar::GetViewSize();
return gfx::Rect(gfx::Point(x * size.width(), y * size.height()), size); return gfx::Rect(
gfx::Point(
x * size.width() + GetLayoutConstant(TOOLBAR_ACTION_LEFT_PADDING),
y * size.height()),
size);
}; };
constexpr int kIconsPerOverflowRow = 3; constexpr int kIconsPerOverflowRow = 3;
......
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