Index: chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc |
diff --git a/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc b/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc |
index 88acf5493217afa34a172b46759624ae99a4ccaf..c82088a7411e116105f257c92a14f5ce2c8d4b9c 100644 |
--- a/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc |
+++ b/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc |
@@ -38,8 +38,6 @@ IconLabelBubbleView::IconLabelBubbleView(int contained_image, |
: background_painter_(nullptr), |
image_(new views::ImageView()), |
label_(new views::Label(base::string16(), font_list)), |
- builtin_leading_padding_(0), |
- builtin_trailing_padding_(0), |
is_extension_icon_(false), |
parent_background_color_(parent_background_color) { |
if (contained_image) { |
@@ -63,9 +61,8 @@ IconLabelBubbleView::IconLabelBubbleView(int contained_image, |
// child views in the location bar have the same height. The visible height of |
// the bubble should be smaller, so use an empty border to shrink down the |
// content bounds so the background gets painted correctly. |
- const int padding = GetLayoutConstant(LOCATION_BAR_BUBBLE_VERTICAL_PADDING); |
- SetBorder( |
- views::Border::CreateEmptyBorder(gfx::Insets(padding, 0, padding, 0))); |
+ SetBorder(views::Border::CreateEmptyBorder( |
+ gfx::Insets(GetLayoutConstant(LOCATION_BAR_BUBBLE_VERTICAL_PADDING), 0))); |
} |
IconLabelBubbleView::~IconLabelBubbleView() { |
@@ -98,13 +95,6 @@ void IconLabelBubbleView::SetLabel(const base::string16& label) { |
void IconLabelBubbleView::SetImage(const gfx::ImageSkia& image_skia) { |
image_->SetImage(image_skia); |
- |
- if (ui::MaterialDesignController::IsModeMaterial()) { |
- gfx::GetVisibleMargins(image_skia, &builtin_leading_padding_, |
- &builtin_trailing_padding_); |
Peter Kasting
2016/05/20 20:54:11
If this call is removed, GetVisibleMargins() can b
Evan Stade
2016/05/20 22:33:53
yes, as noted above. The only reason I haven't don
|
- if (base::i18n::IsRTL()) |
- std::swap(builtin_leading_padding_, builtin_trailing_padding_); |
- } |
} |
bool IconLabelBubbleView::ShouldShowBackground() const { |
@@ -189,15 +179,10 @@ void IconLabelBubbleView::OnNativeThemeChanged( |
const ui::NativeTheme* native_theme) { |
label_->SetEnabledColor(GetTextColor()); |
- if (!ui::MaterialDesignController::IsModeMaterial()) |
- return; |
- |
- bool inverted = color_utils::IsDark(GetParentBackgroundColor()); |
- SkColor border_color = inverted ? SK_ColorWHITE : GetBorderColor(); |
- SkColor background_color = |
- inverted ? SK_ColorWHITE : SkColorSetA(border_color, 0x13); |
- set_background(new BackgroundWith1PxBorder(background_color, border_color)); |
- SetLabelBackgroundColor(background_color); |
+ if (ui::MaterialDesignController::IsModeMaterial()) { |
+ label_->SetBackgroundColor(GetParentBackgroundColor()); |
+ SchedulePaint(); |
+ } |
} |
void IconLabelBubbleView::AddInkDropLayer(ui::Layer* ink_drop_layer) { |
@@ -236,6 +221,9 @@ gfx::Size IconLabelBubbleView::GetSizeForLabelWidth(int label_width) const { |
// is necessary to animate |total_width| even when the background is hidden |
// as long as the animation is still shrinking. |
if (ShouldShowBackground() || shrinking) { |
+ // Extra padding to place at the end of the label (right side in LTR). |
+ const int extra_trailing_padding = |
+ ui::MaterialDesignController::IsModeMaterial() ? 7 : 0; |
Evan Stade
2016/05/19 21:20:45
these constants are basically trial and error. The
Peter Kasting
2016/05/20 20:54:11
The mock is 8 dp between text and divider line, ri
Evan Stade
2016/05/20 22:33:53
The way the mocks measure the padding abuts the re
Peter Kasting
2016/05/20 22:44:56
Is that because that same constant is used as the
Evan Stade
2016/05/23 21:24:13
OK, I tried that, but it messed up the blocked pop
|
// |multiplier| grows from zero to one, stays equal to one and then shrinks |
// to zero again. The view width should correspondingly grow from zero to |
// fully showing both label and icon, stay there, then shrink to just large |
@@ -243,7 +231,8 @@ gfx::Size IconLabelBubbleView::GetSizeForLabelWidth(int label_width) const { |
// zero, since this would mean the view would completely disappear and then |
// pop back to an icon after the animation finishes. |
const int max_width = MinimumWidthForImageWithBackgroundShown() + |
- GetInternalSpacing() + label_width; |
+ GetInternalSpacing() + label_width + |
+ extra_trailing_padding; |
const int current_width = WidthMultiplier() * max_width; |
size.set_width(shrinking ? std::max(current_width, size.width()) |
: current_width); |
@@ -276,8 +265,7 @@ int IconLabelBubbleView::GetOuterPadding(bool leading) const { |
// subtract the amount of built-in padding in the image. This will mean |
// that the actual padding + the padding inside the image add up to the same |
// amount of padding as on the trailing edge of the bubble. |
- return GetLayoutConstant(ICON_LABEL_VIEW_TRAILING_PADDING) - |
- (leading ? builtin_leading_padding_ : 0); |
+ return GetLayoutConstant(ICON_LABEL_VIEW_TRAILING_PADDING); |
} |
return GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING) - |
@@ -288,8 +276,7 @@ int IconLabelBubbleView::GetOuterPadding(bool leading) const { |
int IconLabelBubbleView::GetInternalSpacing() const { |
return image_->GetPreferredSize().IsEmpty() |
? 0 |
- : (GetLayoutConstant(ICON_LABEL_VIEW_INTERNAL_SPACING) - |
- builtin_trailing_padding_); |
+ : GetLayoutConstant(ICON_LABEL_VIEW_INTERNAL_SPACING); |
} |
const char* IconLabelBubbleView::GetClassName() const { |
@@ -303,6 +290,19 @@ void IconLabelBubbleView::OnPaint(gfx::Canvas* canvas) { |
views::Painter::PaintPainterAt(canvas, background_painter_.get(), |
GetContentsBounds()); |
} |
- if (background()) |
- background()->Paint(canvas, this); |
+ if (ui::MaterialDesignController::IsModeMaterial()) { |
+ gfx::Rect bounds(GetLocalBounds()); |
+ // Amount of space to leave after the separator (dp). |
+ const int kPostSeparatorSpacing = 4; |
Peter Kasting
2016/05/20 20:54:11
This is calculated so that with the additional spa
Evan Stade
2016/05/20 22:33:53
well, not quite. The mocks seem to assume less int
Peter Kasting
2016/05/20 22:44:56
Totally agree that calculations can be confusing a
Evan Stade
2016/05/23 21:24:13
made this mathy, ptal?
|
+ // Height of the separator (dp). |
+ const int kSeparatorHeight = 16; |
Peter Kasting
2016/05/20 20:54:11
I wonder if this should be calculated based on the
Evan Stade
2016/05/20 22:33:53
Not sure what you want me to do at the moment. Sha
Peter Kasting
2016/05/20 22:44:57
I'd just ask Max, and assume until he answers that
Evan Stade
2016/05/23 21:24:13
he seems to want exactly 16
|
+ bounds.Inset(kPostSeparatorSpacing, |
+ (bounds.height() - kSeparatorHeight) / 2); |
+ SkColor separator_color = |
+ SkColorSetA(GetNativeTheme()->GetSystemColor( |
+ ui::NativeTheme::kColorId_TextfieldDefaultColor), |
+ 0x26); |
+ canvas->DrawLine(bounds.top_right(), bounds.bottom_right(), |
+ separator_color); |
Peter Kasting
2016/05/20 20:54:11
Will this draw a 2 px-thick line on scale factor 2
Evan Stade
2016/05/20 22:33:53
I think we want 1dp, i.e. 2px on scale factor 2. T
Evan Stade
2016/05/23 21:24:13
ok, 1px it is
|
+ } |
} |