Commit fb3cd665 authored by pkasting@chromium.org's avatar pkasting@chromium.org

ContentSettingImageView cleanup, phase 3.

Swapping between BoxLayouts when the background toggles is confusing, and
requires us to also add and remove children in order to get the padding right.

Instead, just manually Layout(), which isn't hard, and allows us to create the
text label in the constructor and just toggle its visibility instead of actually
adding and removing it.

This CL contains a few minor changes:
* The label now obeys not only the passed-in font but also the color and
  y-position.
* The label is now ALIGN_LEFT, which probably doesn't matter with NO_ELIDE but
  is more logical in principle (since we think of the label as being pinned to
  the icon on its left side).
* The size calculations now cause the animation to smoothly grow from the size
  of the icon all the way out (and vice versa), rather than having the starting
  state of the animation be a jump (compared to the non-animating state) to
  immediately showing the background already expanded out to the margins around
  the icon.  This is almost imperceptible at full speed but makes the animation
  slightly smoother.

BUG=none
TEST=none
R=sky@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@203023 0039d316-1c4b-4281-b951-d872f2087c98
parent 5bc7d2ed
...@@ -45,17 +45,23 @@ ContentSettingImageView::ContentSettingImageView( ...@@ -45,17 +45,23 @@ ContentSettingImageView::ContentSettingImageView(
content_type)), content_type)),
background_painter_(new views::HorizontalPainter(background_images)), background_painter_(new views::HorizontalPainter(background_images)),
icon_(new views::ImageView), icon_(new views::ImageView),
text_label_(NULL), text_label_(new views::Label),
slide_animator_(this), slide_animator_(this),
pause_animation_(false), pause_animation_(false),
pause_animation_state_(0.0), pause_animation_state_(0.0),
bubble_widget_(NULL), bubble_widget_(NULL) {
font_(font),
text_size_(0) {
SetLayoutManager(new views::BoxLayout(
views::BoxLayout::kHorizontal, 0, 0, 0));
icon_->SetHorizontalAlignment(views::ImageView::LEADING); icon_->SetHorizontalAlignment(views::ImageView::LEADING);
AddChildView(icon_); AddChildView(icon_);
text_label_->SetVisible(false);
text_label_->set_border(
views::Border::CreateEmptyBorder(font_y_offset, 0, 0, 0));
text_label_->SetFont(font);
text_label_->SetEnabledColor(font_color);
text_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
text_label_->SetElideBehavior(views::Label::NO_ELIDE);
AddChildView(text_label_);
TouchableLocationBarView::Init(this); TouchableLocationBarView::Init(this);
slide_animator_.SetSlideDuration(kAnimationDurationMS); slide_animator_.SetSlideDuration(kAnimationDurationMS);
...@@ -99,19 +105,8 @@ void ContentSettingImageView::Update(content::WebContents* web_contents) { ...@@ -99,19 +105,8 @@ void ContentSettingImageView::Update(content::WebContents* web_contents) {
// mechanism to show one after the other, but it doesn't seem important now. // mechanism to show one after the other, but it doesn't seem important now.
int string_id = content_setting_image_model_->explanatory_string_id(); int string_id = content_setting_image_model_->explanatory_string_id();
if (string_id && !background_showing()) { if (string_id && !background_showing()) {
// Initialize animated string. It will be cleared when animation is
// completed.
if (!text_label_) {
text_label_ = new views::Label;
text_label_->SetElideBehavior(views::Label::NO_ELIDE);
text_label_->SetFont(font_);
SetLayoutManager(new views::BoxLayout(
views::BoxLayout::kHorizontal, kHorizMargin, 0, kIconLabelSpacing));
AddChildView(text_label_);
}
text_label_->SetText(l10n_util::GetStringUTF16(string_id)); text_label_->SetText(l10n_util::GetStringUTF16(string_id));
text_size_ = font_.GetStringWidth(text_label_->text()); text_label_->SetVisible(true);
text_size_ += kHorizMargin;
slide_animator_.Show(); slide_animator_.Show();
} }
...@@ -122,10 +117,7 @@ void ContentSettingImageView::Update(content::WebContents* web_contents) { ...@@ -122,10 +117,7 @@ void ContentSettingImageView::Update(content::WebContents* web_contents) {
void ContentSettingImageView::AnimationEnded(const ui::Animation* animation) { void ContentSettingImageView::AnimationEnded(const ui::Animation* animation) {
slide_animator_.Reset(); slide_animator_.Reset();
if (!pause_animation_) { if (!pause_animation_) {
SetLayoutManager(new views::BoxLayout( text_label_->SetVisible(false);
views::BoxLayout::kHorizontal, 0, 0, 0));
RemoveChildView(text_label_); // will also delete the view.
text_label_ = NULL;
parent_->Layout(); parent_->Layout();
parent_->SchedulePaint(); parent_->SchedulePaint();
} }
...@@ -146,11 +138,7 @@ void ContentSettingImageView::AnimationCanceled( ...@@ -146,11 +138,7 @@ void ContentSettingImageView::AnimationCanceled(
gfx::Size ContentSettingImageView::GetPreferredSize() { gfx::Size ContentSettingImageView::GetPreferredSize() {
// Height will be ignored by the LocationBarView. // Height will be ignored by the LocationBarView.
gfx::Size preferred_size(views::View::GetPreferredSize()); gfx::Size size(icon_->GetPreferredSize());
int non_label_width = preferred_size.width() -
(text_label_ ? text_label_->GetPreferredSize().width() : 0);
int visible_text_size = 0;
if (background_showing()) { if (background_showing()) {
double state = slide_animator_.GetCurrentValue(); double state = slide_animator_.GetCurrentValue();
// The fraction of the animation we'll spend animating the string into view, // The fraction of the animation we'll spend animating the string into view,
...@@ -163,11 +151,23 @@ gfx::Size ContentSettingImageView::GetPreferredSize() { ...@@ -163,11 +151,23 @@ gfx::Size ContentSettingImageView::GetPreferredSize() {
size_fraction = state / kOpenFraction; size_fraction = state / kOpenFraction;
if (state > (1.0 - kOpenFraction)) if (state > (1.0 - kOpenFraction))
size_fraction = (1.0 - state) / kOpenFraction; size_fraction = (1.0 - state) / kOpenFraction;
visible_text_size = size_fraction * text_size_; size.Enlarge(
size_fraction * (text_label_->GetPreferredSize().width() +
GetTotalSpacingWhileAnimating()), 0);
size.ClampToMin(background_painter_->GetMinimumSize());
} }
return size;
}
preferred_size.set_width(non_label_width + visible_text_size); void ContentSettingImageView::Layout() {
return preferred_size; const int icon_width = icon_->GetPreferredSize().width();
icon_->SetBounds(
std::min((width() - icon_width) / 2, kHorizMargin), 0,
icon_width, height());
text_label_->SetBounds(
icon_->bounds().right() + kIconLabelSpacing, 0,
std::max(width() - GetTotalSpacingWhileAnimating() - icon_width, 0),
text_label_->GetPreferredSize().height());
} }
bool ContentSettingImageView::OnMousePressed(const ui::MouseEvent& event) { bool ContentSettingImageView::OnMousePressed(const ui::MouseEvent& event) {
...@@ -206,6 +206,10 @@ void ContentSettingImageView::OnWidgetDestroying(views::Widget* widget) { ...@@ -206,6 +206,10 @@ void ContentSettingImageView::OnWidgetDestroying(views::Widget* widget) {
} }
} }
int ContentSettingImageView::GetTotalSpacingWhileAnimating() const {
return kHorizMargin * 2 + kIconLabelSpacing;
}
void ContentSettingImageView::OnClick() { void ContentSettingImageView::OnClick() {
if (slide_animator_.is_animating()) { if (slide_animator_.is_animating()) {
if (!pause_animation_) { if (!pause_animation_) {
......
...@@ -67,6 +67,7 @@ class ContentSettingImageView : public TouchableLocationBarView, ...@@ -67,6 +67,7 @@ class ContentSettingImageView : public TouchableLocationBarView,
// views::View: // views::View:
virtual gfx::Size GetPreferredSize() OVERRIDE; virtual gfx::Size GetPreferredSize() OVERRIDE;
virtual void Layout() OVERRIDE;
virtual bool OnMousePressed(const ui::MouseEvent& event) OVERRIDE; virtual bool OnMousePressed(const ui::MouseEvent& event) OVERRIDE;
virtual void OnMouseReleased(const ui::MouseEvent& event) OVERRIDE; virtual void OnMouseReleased(const ui::MouseEvent& event) OVERRIDE;
virtual void OnGestureEvent(ui::GestureEvent* event) OVERRIDE; virtual void OnGestureEvent(ui::GestureEvent* event) OVERRIDE;
...@@ -79,7 +80,7 @@ class ContentSettingImageView : public TouchableLocationBarView, ...@@ -79,7 +80,7 @@ class ContentSettingImageView : public TouchableLocationBarView,
return slide_animator_.is_animating() || pause_animation_; return slide_animator_.is_animating() || pause_animation_;
} }
// Invoked when the user clicks on the control. int GetTotalSpacingWhileAnimating() const;
void OnClick(); void OnClick();
LocationBarView* parent_; // Weak, owns us. LocationBarView* parent_; // Weak, owns us.
...@@ -92,10 +93,6 @@ class ContentSettingImageView : public TouchableLocationBarView, ...@@ -92,10 +93,6 @@ class ContentSettingImageView : public TouchableLocationBarView,
double pause_animation_state_; double pause_animation_state_;
views::Widget* bubble_widget_; views::Widget* bubble_widget_;
// TODO(pkasting): Eliminate these.
gfx::Font font_;
int text_size_;
DISALLOW_COPY_AND_ASSIGN(ContentSettingImageView); DISALLOW_COPY_AND_ASSIGN(ContentSettingImageView);
}; };
......
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