Commit e8b6973c authored by ellyjones's avatar ellyjones Committed by Commit bot

Revert of BubbleFrameView: add top padding even when close button is hidden...

Revert of BubbleFrameView: add top padding even when close button is hidden (patchset #7 id:120001 of https://codereview.chromium.org/2148963002/ )

Reason for revert:
Caused https://crbug.com/630539

Original issue's description:
> BubbleFrameView: add top padding even when close button is hidden
>
> If there's no title or close button present, BubbleFrameView still needs some
> top padding to avoid the content being hard up against the top edge. Always pad
> as though the close button was visible, even if it's not, unless there's a
> title.
>
> BUG=622859
>
> Committed: https://crrev.com/f06ef4ac6ac3dd2f3a76486d2efde9ec35583ae6
> Cr-Commit-Position: refs/heads/master@{#406856}

TBR=msw@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=622859

Review-Url: https://codereview.chromium.org/2177533002
Cr-Commit-Position: refs/heads/master@{#407161}
parent 70009066
...@@ -19,7 +19,7 @@ ...@@ -19,7 +19,7 @@
#include "ui/views/border.h" #include "ui/views/border.h"
#include "ui/views/bubble/bubble_border.h" #include "ui/views/bubble/bubble_border.h"
#include "ui/views/bubble/bubble_frame_view.h" #include "ui/views/bubble/bubble_frame_view.h"
#include "ui/views/controls/button/image_button.h" #include "ui/views/controls/button/label_button.h"
#include "ui/views/layout/fill_layout.h" #include "ui/views/layout/fill_layout.h"
#include "ui/views/widget/widget.h" #include "ui/views/widget/widget.h"
#include "ui/views/window/client_view.h" #include "ui/views/window/client_view.h"
...@@ -168,7 +168,7 @@ class AppListDialogContainer : public BaseDialogContainer, ...@@ -168,7 +168,7 @@ class AppListDialogContainer : public BaseDialogContainer,
} }
} }
views::ImageButton* close_button_; views::LabelButton* close_button_;
DISALLOW_COPY_AND_ASSIGN(AppListDialogContainer); DISALLOW_COPY_AND_ASSIGN(AppListDialogContainer);
}; };
......
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
#include "ui/base/hit_test.h" #include "ui/base/hit_test.h"
#include "ui/events/event_utils.h" #include "ui/events/event_utils.h"
#include "ui/views/bubble/bubble_frame_view.h" #include "ui/views/bubble/bubble_frame_view.h"
#include "ui/views/controls/button/image_button.h" #include "ui/views/controls/button/label_button.h"
#include "ui/views/test/test_widget_observer.h" #include "ui/views/test/test_widget_observer.h"
#include "ui/views/test/views_test_base.h" #include "ui/views/test/views_test_base.h"
#include "ui/views/widget/widget.h" #include "ui/views/widget/widget.h"
...@@ -321,7 +321,7 @@ TEST_F(BubbleDialogDelegateTest, CloseMethods) { ...@@ -321,7 +321,7 @@ TEST_F(BubbleDialogDelegateTest, CloseMethods) {
BubbleDialogDelegateView::CreateBubble(bubble_delegate); BubbleDialogDelegateView::CreateBubble(bubble_delegate);
bubble_widget->Show(); bubble_widget->Show();
BubbleFrameView* frame_view = bubble_delegate->GetBubbleFrameView(); BubbleFrameView* frame_view = bubble_delegate->GetBubbleFrameView();
ImageButton* close_button = frame_view->close_; LabelButton* close_button = frame_view->close_;
ASSERT_TRUE(close_button); ASSERT_TRUE(close_button);
frame_view->ButtonPressed( frame_view->ButtonPressed(
close_button, close_button,
......
...@@ -99,15 +99,15 @@ BubbleFrameView::BubbleFrameView(const gfx::Insets& title_margins, ...@@ -99,15 +99,15 @@ BubbleFrameView::BubbleFrameView(const gfx::Insets& title_margins,
BubbleFrameView::~BubbleFrameView() {} BubbleFrameView::~BubbleFrameView() {}
// static // static
ImageButton* BubbleFrameView::CreateCloseButton(ButtonListener* listener) { LabelButton* BubbleFrameView::CreateCloseButton(ButtonListener* listener) {
ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
ImageButton* close = new ImageButton(listener); LabelButton* close = new LabelButton(listener, base::string16());
close->SetImage(CustomButton::STATE_NORMAL, close->SetImage(CustomButton::STATE_NORMAL,
rb.GetImageNamed(IDR_CLOSE_DIALOG).ToImageSkia()); *rb.GetImageNamed(IDR_CLOSE_DIALOG).ToImageSkia());
close->SetImage(CustomButton::STATE_HOVERED, close->SetImage(CustomButton::STATE_HOVERED,
rb.GetImageNamed(IDR_CLOSE_DIALOG_H).ToImageSkia()); *rb.GetImageNamed(IDR_CLOSE_DIALOG_H).ToImageSkia());
close->SetImage(CustomButton::STATE_PRESSED, close->SetImage(CustomButton::STATE_PRESSED,
rb.GetImageNamed(IDR_CLOSE_DIALOG_P).ToImageSkia()); *rb.GetImageNamed(IDR_CLOSE_DIALOG_P).ToImageSkia());
close->SetBorder(nullptr); close->SetBorder(nullptr);
close->SetSize(close->GetPreferredSize()); close->SetSize(close->GetPreferredSize());
#if !defined(OS_WIN) #if !defined(OS_WIN)
...@@ -245,9 +245,7 @@ gfx::Insets BubbleFrameView::GetInsets() const { ...@@ -245,9 +245,7 @@ gfx::Insets BubbleFrameView::GetInsets() const {
const int title_padding = has_title ? title_margins_.height() : 0; const int title_padding = has_title ? title_margins_.height() : 0;
const int title_height = std::max(icon_height, label_height) + title_padding; const int title_height = std::max(icon_height, label_height) + title_padding;
const int close_height = close_->visible() ? close_->height() : 0; const int close_height = close_->visible() ? close_->height() : 0;
const int min_height = !has_title ? close_->height() : 0; insets += gfx::Insets(std::max(title_height, close_height), 0, 0, 0);
insets +=
gfx::Insets(std::max({title_height, close_height, min_height}), 0, 0, 0);
return insets; return insets;
} }
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include "base/macros.h" #include "base/macros.h"
#include "ui/gfx/geometry/insets.h" #include "ui/gfx/geometry/insets.h"
#include "ui/views/controls/button/button.h" #include "ui/views/controls/button/button.h"
#include "ui/views/controls/button/image_button.h"
#include "ui/views/window/non_client_view.h" #include "ui/views/window/non_client_view.h"
namespace gfx { namespace gfx {
...@@ -36,7 +35,7 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView, ...@@ -36,7 +35,7 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView,
~BubbleFrameView() override; ~BubbleFrameView() override;
// Creates a close button used in the corner of the dialog. // Creates a close button used in the corner of the dialog.
static ImageButton* CreateCloseButton(ButtonListener* listener); static LabelButton* CreateCloseButton(ButtonListener* listener);
// NonClientFrameView overrides: // NonClientFrameView overrides:
gfx::Rect GetBoundsForClientView() const override; gfx::Rect GetBoundsForClientView() const override;
...@@ -86,7 +85,7 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView, ...@@ -86,7 +85,7 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView,
bool close_button_clicked() const { return close_button_clicked_; } bool close_button_clicked() const { return close_button_clicked_; }
ImageButton* GetCloseButtonForTest() { return close_; } LabelButton* GetCloseButtonForTest() { return close_; }
protected: protected:
// Returns the available screen bounds if the frame were to show in |rect|. // Returns the available screen bounds if the frame were to show in |rect|.
...@@ -126,7 +125,7 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView, ...@@ -126,7 +125,7 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView,
// The optional title icon, title, and (x) close button. // The optional title icon, title, and (x) close button.
views::ImageView* title_icon_; views::ImageView* title_icon_;
Label* title_; Label* title_;
ImageButton* close_; LabelButton* close_;
// A view to contain the footnote view, if it exists. // A view to contain the footnote view, if it exists.
View* footnote_container_; View* footnote_container_;
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
#include "ui/views/bubble/bubble_border.h" #include "ui/views/bubble/bubble_border.h"
#include "ui/views/controls/button/image_button.h"
#include "ui/views/test/test_views.h" #include "ui/views/test/test_views.h"
#include "ui/views/test/views_test_base.h" #include "ui/views/test/views_test_base.h"
#include "ui/views/widget/widget.h" #include "ui/views/widget/widget.h"
...@@ -37,8 +36,7 @@ const int kPreferredClientHeight = 250; ...@@ -37,8 +36,7 @@ const int kPreferredClientHeight = 250;
// These account for non-client areas like the title bar, footnote etc. However // These account for non-client areas like the title bar, footnote etc. However
// these do not take the bubble border into consideration. // these do not take the bubble border into consideration.
const int kExpectedAdditionalWidth = 12; const int kExpectedAdditionalWidth = 12;
// 12 for the builtin margin, 14 for the close button margin const int kExpectedAdditionalHeight = 12;
const int kExpectedAdditionalHeight = 12 + 14;
class TestBubbleFrameViewWidgetDelegate : public WidgetDelegate { class TestBubbleFrameViewWidgetDelegate : public WidgetDelegate {
public: public:
...@@ -121,11 +119,9 @@ TEST_F(BubbleFrameViewTest, GetBoundsForClientView) { ...@@ -121,11 +119,9 @@ TEST_F(BubbleFrameViewTest, GetBoundsForClientView) {
int margin_x = frame.content_margins().left(); int margin_x = frame.content_margins().left();
int margin_y = frame.content_margins().top(); int margin_y = frame.content_margins().top();
int close_y = frame.GetCloseButtonForTest()->height();
gfx::Insets insets = frame.bubble_border()->GetInsets(); gfx::Insets insets = frame.bubble_border()->GetInsets();
EXPECT_EQ(insets.left() + margin_x, frame.GetBoundsForClientView().x()); EXPECT_EQ(insets.left() + margin_x, frame.GetBoundsForClientView().x());
EXPECT_EQ(insets.top() + margin_y + close_y, EXPECT_EQ(insets.top() + margin_y, frame.GetBoundsForClientView().y());
frame.GetBoundsForClientView().y());
} }
// Tests that the arrow is mirrored as needed to better fit the screen. // Tests that the arrow is mirrored as needed to better fit the screen.
...@@ -441,9 +437,6 @@ TEST_F(BubbleFrameViewTest, GetMinimumSize) { ...@@ -441,9 +437,6 @@ TEST_F(BubbleFrameViewTest, GetMinimumSize) {
// Expect that a border has been added to the minimum size. // Expect that a border has been added to the minimum size.
minimum_rect.Inset(frame.bubble_border()->GetInsets()); minimum_rect.Inset(frame.bubble_border()->GetInsets());
ImageButton* button = frame.GetCloseButtonForTest();
EXPECT_EQ(button->height(), 14);
EXPECT_EQ(nullptr, frame.GetCloseButtonForTest()->border());
gfx::Size expected_size(kMinimumClientWidth + kExpectedAdditionalWidth, gfx::Size expected_size(kMinimumClientWidth + kExpectedAdditionalWidth,
kMinimumClientHeight + kExpectedAdditionalHeight); kMinimumClientHeight + kExpectedAdditionalHeight);
EXPECT_EQ(expected_size, minimum_rect.size()); EXPECT_EQ(expected_size, minimum_rect.size());
......
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