Skip to content

Commit

Permalink
Adjusts content bubble animation with respect to icon size
Browse files Browse the repository at this point in the history 
This CL corrects several issues with the icon bubble animation:
1. The bubble is not clipping its children, so the image draws outside the bubble bounds. This is MD-specific.
2. The image is being centered at the bubble center when the size is too low, instead of always being positioned based on the leading edge of the bubble, so it doesn't "slide in" correctly shifting during the initial portion of the animation.
3. The animation "closes" down to zero width (MD) or smaller than necessary width (non-MD) and then snaps the image back into place; it should animate down to become just large enough to show the image.
4. Removes the clamp in non-MD size that prevents animation from starting at zero.
5. Avoids clipping icon at the end of the animation (MD) by hiding the border even before the animation ends.
6. Reduces padding at the end of the MD version of the text label.
7. Corrects RTL layout by showing trailing edge of the icon first.

BUG=591411

Review URL: https://codereview.chromium.org/1763713004

Cr-Commit-Position: refs/heads/master@{#380729}
  • Loading branch information
varkha authored and Commit bot committed Mar 11, 2016
1 parent a8b8995 commit a2a4fab
Show file tree
Hide file tree
Showing 7 changed files with 342 additions and 51 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/ui/layout_constants.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
int GetLayoutConstant(LayoutConstant constant) {
const int kFindBarVerticalOffset[] = {1, 6, 6};
const int kIconLabelViewInternalPadding[] = {3, 2, 2};
const int kIconLabelViewTrailingPadding[] = {2, 8, 8};
const int kIconLabelViewTrailingPadding[] = {2, 3, 3};
const int kLocationBarBorderThickness[] = {2, 1, 1};
const int kLocationBarBubbleFontVerticalPadding[] = {1, 2, 4};
const int kLocationBarBubbleHorizontalPadding[] = {1, 4, 4};
Expand Down
47 changes: 33 additions & 14 deletions chrome/browser/ui/views/location_bar/content_setting_image_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ ContentSettingImageView::ContentSettingImageView(
SetBackgroundImageGrid(kBackgroundImages);
}

image()->SetHorizontalAlignment(views::ImageView::LEADING);
image()->SetHorizontalAlignment(base::i18n::IsRTL()
? views::ImageView::TRAILING
: views::ImageView::LEADING);
image()->set_interactive(true);
image()->EnableCanvasFlippingForRTLUI(true);
image()->SetAccessibilityFocusable(true);
Expand Down Expand Up @@ -94,7 +96,8 @@ void ContentSettingImageView::Update(content::WebContents* web_contents) {
// the user. If this becomes a problem, we could design some sort of queueing
// mechanism to show one after the other, but it doesn't seem important now.
int string_id = content_setting_image_model_->explanatory_string_id();
if (string_id && !ShouldShowBackground()) {
if (string_id && !label()->visible()) {
ink_drop_delegate_->OnAction(views::InkDropState::HIDDEN);
SetLabel(l10n_util::GetStringUTF16(string_id));
label()->SetVisible(true);
slide_animator_.Show();
Expand All @@ -113,7 +116,8 @@ SkColor ContentSettingImageView::GetBorderColor() const {
}

bool ContentSettingImageView::ShouldShowBackground() const {
return slide_animator_.is_animating() || pause_animation_;
return (!IsShrinking() || label()->width() > 0) &&
(slide_animator_.is_animating() || pause_animation_);
}

double ContentSettingImageView::WidthMultiplier() const {
Expand All @@ -132,6 +136,13 @@ double ContentSettingImageView::WidthMultiplier() const {
return size_fraction;
}

bool ContentSettingImageView::IsShrinking() const {
const double kOpenFraction =
static_cast<double>(kOpenTimeMS) / kAnimationDurationMS;
return (!pause_animation_ && slide_animator_.is_animating() &&
slide_animator_.GetCurrentValue() > (1.0 - kOpenFraction));
}

void ContentSettingImageView::AnimationEnded(const gfx::Animation* animation) {
slide_animator_.Reset();
if (!pause_animation_) {
Expand Down Expand Up @@ -184,10 +195,8 @@ void ContentSettingImageView::OnMouseReleased(const ui::MouseEvent& event) {
return;
}
const bool activated = HitTestPoint(event.location());
if (!label()->visible()) {
ink_drop_delegate_->OnAction(activated ? views::InkDropState::ACTIVATED
: views::InkDropState::HIDDEN);
}
if (!label()->visible() && !activated)
ink_drop_delegate_->OnAction(views::InkDropState::HIDDEN);
if (activated)
OnClick();
}
Expand All @@ -196,17 +205,13 @@ bool ContentSettingImageView::OnKeyPressed(const ui::KeyEvent& event) {
if (event.key_code() != ui::VKEY_SPACE && event.key_code() != ui::VKEY_RETURN)
return false;

ink_drop_delegate_->OnAction(views::InkDropState::ACTIVATED);
OnClick();
return true;
}

void ContentSettingImageView::OnGestureEvent(ui::GestureEvent* event) {
if (event->type() == ui::ET_GESTURE_TAP) {
if (!label()->visible())
ink_drop_delegate_->OnAction(views::InkDropState::ACTIVATED);
if (event->type() == ui::ET_GESTURE_TAP)
OnClick();
}
if ((event->type() == ui::ET_GESTURE_TAP) ||
(event->type() == ui::ET_GESTURE_TAP_DOWN))
event->SetHandled();
Expand Down Expand Up @@ -242,7 +247,19 @@ void ContentSettingImageView::OnWidgetVisibilityChanged(views::Widget* widget,

void ContentSettingImageView::OnClick() {
if (slide_animator_.is_animating()) {
if (!pause_animation_) {
// If the user clicks while we're animating, the bubble arrow will be
// pointing to the image, and if we allow the animation to keep running, the
// image will move away from the arrow (or we'll have to move the bubble,
// which is even worse). So we want to stop the animation. We have two
// choices: jump to the final post-animation state (no label visible), or
// pause the animation where we are and continue running after the bubble
// closes. The former looks more jerky, so we avoid it unless the animation
// hasn't even fully exposed the image yet, in which case pausing with half
// an image visible will look broken.
const int final_width = image()->GetPreferredSize().width() +
GetBubbleOuterPadding(true) +
GetBubbleOuterPadding(false);
if (!pause_animation_ && ShouldShowBackground() && width() > final_width) {
pause_animation_ = true;
pause_animation_state_ = slide_animator_.GetCurrentValue();
}
Expand All @@ -264,8 +281,10 @@ void ContentSettingImageView::OnClick() {
// bubble doesn't need an arrow. If the user clicks during an animation,
// the animation simply pauses and no other visible state change occurs, so
// show the arrow in this case.
if (ui::MaterialDesignController::IsModeMaterial() && !pause_animation_)
if (ui::MaterialDesignController::IsModeMaterial() && !pause_animation_) {
ink_drop_delegate_->OnAction(views::InkDropState::ACTIVATED);
bubble_view_->SetArrowPaintType(views::BubbleBorder::PAINT_TRANSPARENT);
}
bubble_widget->Show();
}
}
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/views/location_bar/content_setting_image_view.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class ContentSettingImageView : public IconLabelBubbleView,
SkColor GetBorderColor() const override;
bool ShouldShowBackground() const override;
double WidthMultiplier() const override;
bool IsShrinking() const override;

// gfx::AnimationDelegate:
void AnimationEnded(const gfx::Animation* animation) override;
Expand Down
109 changes: 79 additions & 30 deletions chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ double IconLabelBubbleView::WidthMultiplier() const {
return 1.0;
}

bool IconLabelBubbleView::IsShrinking() const {
return false;
}

int IconLabelBubbleView::GetImageAndPaddingWidth() const {
const int image_width = image_->GetPreferredSize().width();
return image_width
Expand All @@ -118,20 +122,53 @@ gfx::Size IconLabelBubbleView::GetPreferredSize() const {
}

void IconLabelBubbleView::Layout() {
// In MD mode, both extension icons and Chrome-provided icons are 16px,
// so it's not necessary to handle them differently. TODO(estade): clean
// this up when MD is on by default.
bool icon_has_enough_padding =
// Compute the label bounds. The label gets whatever size is left over after
// accounting for the preferred image width and padding amounts. Note that if
// the label has zero size it doesn't actually matter what we compute its X
// value to be, since it won't be visible, so the X value can be "wrong"
// compared to where the right edge of the image is computed to be below.
// This means doing this layout doesn't doesn't depend on any of the layout
// below. That layout, however, may need for this layout to have already
// happened, since the value of ShouldShowBackground() we read below may
// depend on whether the label has nonzero size. Therefore, we do this first.
const int label_x = GetBubbleOuterPadding(true) + GetImageAndPaddingWidth();
const int label_width =
std::max(0, width() - label_x - GetBubbleOuterPadding(false));
label_->SetBounds(label_x, 0, label_width, height());

// Now compute the image bounds. In non-MD, the leading padding depends on
// whether this is an extension icon, since extension icons and
// Chrome-provided icons are different sizes. In MD, these sizes are the
// same, so it's not necessary to handle the two types differently.
const bool icon_has_enough_padding =
!is_extension_icon_ || ui::MaterialDesignController::IsModeMaterial();
const int image_width = image_->GetPreferredSize().width();
image_->SetBounds(std::min((width() - image_width) / 2,
GetBubbleOuterPadding(icon_has_enough_padding)),
0, image_->GetPreferredSize().width(), height());
int image_x = GetBubbleOuterPadding(icon_has_enough_padding);
const int image_preferred_width = image_->GetPreferredSize().width();
int bubble_trailing_padding = GetBubbleOuterPadding(false);

// If ShouldShowBackground() is true, then either we show a background in the
// steady state, or we're not yet in the last portion of the animation. In
// these cases, we leave the leading and trailing padding alone; we don't want
// to let the image overlap the edge of the background, as this looks glitchy.
// If this is false, however, then we're only showing the image, and either
// the view width is the image width, or it's animating downwards and getting
// close to it. In these cases, we want to shrink the trailing padding first,
// so the image slides all the way to the trailing edge before slowing or
// stopping; then we want to shrink the leading padding down to zero.
if (!ShouldShowBackground()) {
image_x = std::min(image_x, width() - image_preferred_width);
bubble_trailing_padding = std::min(
bubble_trailing_padding, width() - image_preferred_width - image_x);
}

int pre_label_width = GetBubbleOuterPadding(true) + GetImageAndPaddingWidth();
label_->SetBounds(pre_label_width, 0,
width() - pre_label_width - GetBubbleOuterPadding(false),
height());
// Now that we've computed the padding values, give the image all the
// remaining width. This will be less than the image's preferred width during
// the first portion of the animation; during the very beginning there may not
// be enough room to show the image at all.
const int image_width =
std::min(image_preferred_width,
std::max(0, width() - image_x - bubble_trailing_padding));
image_->SetBounds(image_x, 0, image_width, height());
}

void IconLabelBubbleView::OnNativeThemeChanged(
Expand Down Expand Up @@ -177,20 +214,41 @@ SkColor IconLabelBubbleView::GetParentBackgroundColor() const {
: parent_background_color_;
}

gfx::Size IconLabelBubbleView::GetSizeForLabelWidth(int width) const {
gfx::Size IconLabelBubbleView::GetSizeForLabelWidth(int label_width) const {
gfx::Size size(image_->GetPreferredSize());
if (ShouldShowBackground()) {
const int non_label_width = GetBubbleOuterPadding(true) +
GetImageAndPaddingWidth() +
GetBubbleOuterPadding(false);
size = gfx::Size(WidthMultiplier() * (width + non_label_width), 0);
if (!ui::MaterialDesignController::IsModeMaterial())
size.SetToMax(background_painter_->GetMinimumSize());
bool shrinking = IsShrinking();
// Animation continues for the last few pixels even after the label is not
// visible in order to slide the icon into its final position. Therefore it
// is necessary to animate |total_width| even when the background is hidden
// as long as the animation is still shrinking.
if (ShouldShowBackground() || shrinking) {
const int image_width = size.width();
const int padding = GetLayoutConstant(ICON_LABEL_VIEW_INTERNAL_PADDING) +
GetBubbleOuterPadding(true) +
GetBubbleOuterPadding(false);
// |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
// enough to show the icon. We don't want to shrink all the way back to
// zero, since this would mean the view would completely disappear and then
// pop back to an icon after the animation finishes.
int total_width = WidthMultiplier() * (label_width + image_width + padding);
if (shrinking)
total_width = std::max(total_width, image_width);
size.set_width(total_width);
}

return size;
}

int IconLabelBubbleView::GetBubbleOuterPadding(bool leading) const {
if (ui::MaterialDesignController::IsModeMaterial())
return GetBubbleOuterPaddingMd(leading);

return GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING) -
GetLayoutConstant(LOCATION_BAR_BUBBLE_HORIZONTAL_PADDING) +
(leading ? 0 : GetLayoutConstant(ICON_LABEL_VIEW_TRAILING_PADDING));
}

void IconLabelBubbleView::SetLabelBackgroundColor(
SkColor chip_background_color) {
// The background images are painted atop |parent_background_color_|.
Expand All @@ -205,15 +263,6 @@ void IconLabelBubbleView::SetLabelBackgroundColor(
SkColorGetA(chip_background_color)));
}

int IconLabelBubbleView::GetBubbleOuterPadding(bool leading) const {
if (ui::MaterialDesignController::IsModeMaterial())
return GetBubbleOuterPaddingMd(leading);

return GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING) -
GetLayoutConstant(LOCATION_BAR_BUBBLE_HORIZONTAL_PADDING) +
(leading ? 0 : GetLayoutConstant(ICON_LABEL_VIEW_TRAILING_PADDING));
}

int IconLabelBubbleView::GetBubbleOuterPaddingMd(bool leading) const {
// When the image is empty, leading and trailing padding are equal.
if (image_->GetPreferredSize().IsEmpty() || !leading)
Expand Down
16 changes: 10 additions & 6 deletions chrome/browser/ui/views/location_bar/icon_label_bubble_view.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class IconLabelBubbleView : public views::InkDropHostView {
protected:
views::ImageView* image() { return image_; }
views::Label* label() { return label_; }
const views::Label* label() const { return label_; }

// Gets the color for displaying text.
virtual SkColor GetTextColor() const = 0;
Expand All @@ -70,6 +71,9 @@ class IconLabelBubbleView : public views::InkDropHostView {
// full-width view and can be used to animate the width of the view.
virtual double WidthMultiplier() const;

// Returns true when animation is in progress and is shrinking.
virtual bool IsShrinking() const;

// Returns the amount of horizontal space needed to draw the image and its
// padding before the label.
virtual int GetImageAndPaddingWidth() const;
Expand All @@ -87,18 +91,18 @@ class IconLabelBubbleView : public views::InkDropHostView {

SkColor GetParentBackgroundColor() const;

gfx::Size GetSizeForLabelWidth(int width) const;
gfx::Size GetSizeForLabelWidth(int label_width) const;

// Amount of padding from the edge of the icon / label to the outer edge of
// the bubble view. If |leading| is true, this is the padding at the
// beginning of the bubble (left in LTR), otherwise it's the trailing padding.
int GetBubbleOuterPadding(bool leading) const;

private:
// Sets a background color on |label_| based on |chip_background_color| and
// the parent's bg color.
void SetLabelBackgroundColor(SkColor chip_background_color);

// Amount of padding at the edges of the bubble. If |leading| is true, this
// is the padding at the beginning of the bubble (left in LTR), otherwise it's
// the end padding.
int GetBubbleOuterPadding(bool leading) const;

// As above, but for Material Design. TODO(estade): remove/replace the above.
int GetBubbleOuterPaddingMd(bool leading) const;

Expand Down
Loading

0 comments on commit a2a4fab

Please sign in to comment.