Commit 3f6a13da authored by Allen Bauer's avatar Allen Bauer Committed by Commit Bot

Harmonize "Ask Google for suggestions" dialog.

Change link to help (question mark) vector icon.
Removed close (X) button from ConfirmBubbleView dialog bubble.

TBR=lazyboy@chromium.org

Bug: 654139
Change-Id: I99e58f98d449d83b0aaea3aa49acfd71912a6ea5
Reviewed-on: https://chromium-review.googlesource.com/871183
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: default avatarAllen Bauer <kylixrd@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531895}
parent 0fc0e49c
...@@ -54,12 +54,12 @@ base::string16 SpellingBubbleModel::GetLinkText() const { ...@@ -54,12 +54,12 @@ base::string16 SpellingBubbleModel::GetLinkText() const {
return l10n_util::GetStringUTF16(IDS_LEARN_MORE); return l10n_util::GetStringUTF16(IDS_LEARN_MORE);
} }
GURL SpellingBubbleModel::GetLinkURL() const { GURL SpellingBubbleModel::GetHelpPageURL() const {
return GURL(chrome::kPrivacyLearnMoreURL); return GURL(chrome::kPrivacyLearnMoreURL);
} }
void SpellingBubbleModel::LinkClicked() { void SpellingBubbleModel::OpenHelpPage() {
OpenURLParams params(GetLinkURL(), Referrer(), OpenURLParams params(GetHelpPageURL(), Referrer(),
WindowOpenDisposition::NEW_FOREGROUND_TAB, WindowOpenDisposition::NEW_FOREGROUND_TAB,
ui::PAGE_TRANSITION_LINK, false); ui::PAGE_TRANSITION_LINK, false);
web_contents_->OpenURL(params); web_contents_->OpenURL(params);
......
...@@ -29,8 +29,8 @@ class SpellingBubbleModel : public ConfirmBubbleModel { ...@@ -29,8 +29,8 @@ class SpellingBubbleModel : public ConfirmBubbleModel {
void Accept() override; void Accept() override;
void Cancel() override; void Cancel() override;
base::string16 GetLinkText() const override; base::string16 GetLinkText() const override;
GURL GetLinkURL() const override; GURL GetHelpPageURL() const override;
void LinkClicked() override; void OpenHelpPage() override;
private: private:
// Set the profile preferences to enable or disable the feature. // Set the profile preferences to enable or disable the feature.
......
...@@ -32,15 +32,7 @@ class AskGoogleForSuggestionsDialogTest : public DialogBrowserTest { ...@@ -32,15 +32,7 @@ class AskGoogleForSuggestionsDialogTest : public DialogBrowserTest {
DISALLOW_COPY_AND_ASSIGN(AskGoogleForSuggestionsDialogTest); DISALLOW_COPY_AND_ASSIGN(AskGoogleForSuggestionsDialogTest);
}; };
#if !defined(OS_MACOSX)
// Initially disabled except on Mac due to http://crbug.com/683808.
#define MAYBE_InvokeUi_default DISABLED_InvokeUi_default
#else
#define MAYBE_InvokeUi_default InvokeUi_default
#endif
// Test that calls ShowUi("default"). // Test that calls ShowUi("default").
IN_PROC_BROWSER_TEST_F(AskGoogleForSuggestionsDialogTest, IN_PROC_BROWSER_TEST_F(AskGoogleForSuggestionsDialogTest, InvokeUi_default) {
MAYBE_InvokeUi_default) {
ShowAndVerifyUi(); ShowAndVerifyUi();
} }
...@@ -128,7 +128,7 @@ const int kButtonHEdgeMargin = 7; ...@@ -128,7 +128,7 @@ const int kButtonHEdgeMargin = 7;
- (BOOL)textView:(NSTextView*)textView - (BOOL)textView:(NSTextView*)textView
clickedOnLink:(id)link clickedOnLink:(id)link
atIndex:(NSUInteger)charIndex { atIndex:(NSUInteger)charIndex {
[controller_ linkClicked]; [controller_ openHelpPage];
[self closeBubble]; [self closeBubble];
return YES; return YES;
} }
...@@ -195,8 +195,7 @@ const int kButtonHEdgeMargin = 7; ...@@ -195,8 +195,7 @@ const int kButtonHEdgeMargin = 7;
base::scoped_nsobject<NSAttributedString> whiteSpace( base::scoped_nsobject<NSAttributedString> whiteSpace(
[[NSAttributedString alloc] initWithString:@" "]); [[NSAttributedString alloc] initWithString:@" "]);
[attributedMessage.get() appendAttributedString:whiteSpace.get()]; [attributedMessage.get() appendAttributedString:whiteSpace.get()];
[attributes setObject:[controller_ linkURL] [attributes setObject:[controller_ helpPageURL] forKey:NSLinkAttributeName];
forKey:NSLinkAttributeName];
base::scoped_nsobject<NSAttributedString> attributedLink( base::scoped_nsobject<NSAttributedString> attributedLink(
[[NSAttributedString alloc] initWithString:linkText [[NSAttributedString alloc] initWithString:linkText
attributes:attributes]); attributes:attributes]);
......
...@@ -35,7 +35,7 @@ class ConfirmBubbleModel; ...@@ -35,7 +35,7 @@ class ConfirmBubbleModel;
- (NSString*)title; - (NSString*)title;
- (NSString*)messageText; - (NSString*)messageText;
- (NSString*)linkText; - (NSString*)linkText;
- (NSString*)linkURL; - (NSString*)helpPageURL;
- (NSString*)okButtonText; - (NSString*)okButtonText;
- (NSString*)cancelButtonText; - (NSString*)cancelButtonText;
- (BOOL)hasOkButton; - (BOOL)hasOkButton;
...@@ -44,7 +44,7 @@ class ConfirmBubbleModel; ...@@ -44,7 +44,7 @@ class ConfirmBubbleModel;
// Handle actions from the ConfirmBubbleCocoa object. // Handle actions from the ConfirmBubbleCocoa object.
- (void)accept; - (void)accept;
- (void)cancel; - (void)cancel;
- (void)linkClicked; - (void)openHelpPage;
@end @end
......
...@@ -53,8 +53,8 @@ ...@@ -53,8 +53,8 @@
return base::SysUTF16ToNSString(model_->GetLinkText()); return base::SysUTF16ToNSString(model_->GetLinkText());
} }
- (NSString*)linkURL { - (NSString*)helpPageURL {
return base::SysUTF8ToNSString(model_->GetLinkURL().spec()); return base::SysUTF8ToNSString(model_->GetHelpPageURL().spec());
} }
- (NSString*)okButtonText { - (NSString*)okButtonText {
...@@ -84,8 +84,8 @@ ...@@ -84,8 +84,8 @@
model_->Cancel(); model_->Cancel();
} }
- (void)linkClicked { - (void)openHelpPage {
model_->LinkClicked(); model_->OpenHelpPage();
} }
@end @end
...@@ -32,7 +32,7 @@ class TestConfirmBubbleModel : public ConfirmBubbleModel { ...@@ -32,7 +32,7 @@ class TestConfirmBubbleModel : public ConfirmBubbleModel {
void Accept() override; void Accept() override;
void Cancel() override; void Cancel() override;
base::string16 GetLinkText() const override; base::string16 GetLinkText() const override;
void LinkClicked() override; void OpenHelpPage() override;
private: private:
bool* accept_clicked_; bool* accept_clicked_;
...@@ -77,7 +77,7 @@ base::string16 TestConfirmBubbleModel::GetLinkText() const { ...@@ -77,7 +77,7 @@ base::string16 TestConfirmBubbleModel::GetLinkText() const {
return base::ASCIIToUTF16("Link"); return base::ASCIIToUTF16("Link");
} }
void TestConfirmBubbleModel::LinkClicked() { void TestConfirmBubbleModel::OpenHelpPage() {
*link_clicked_ = true; *link_clicked_ = true;
} }
......
...@@ -32,9 +32,8 @@ base::string16 ConfirmBubbleModel::GetLinkText() const { ...@@ -32,9 +32,8 @@ base::string16 ConfirmBubbleModel::GetLinkText() const {
return base::string16(); return base::string16();
} }
GURL ConfirmBubbleModel::GetLinkURL() const { GURL ConfirmBubbleModel::GetHelpPageURL() const {
return GURL(); return GURL();
} }
void ConfirmBubbleModel::LinkClicked() { void ConfirmBubbleModel::OpenHelpPage() {}
}
...@@ -50,10 +50,10 @@ class ConfirmBubbleModel { ...@@ -50,10 +50,10 @@ class ConfirmBubbleModel {
virtual base::string16 GetLinkText() const; virtual base::string16 GetLinkText() const;
// Returns the URL of the link to be displayed. // Returns the URL of the link to be displayed.
virtual GURL GetLinkURL() const; virtual GURL GetHelpPageURL() const;
// Called when the link is clicked. // Called when the link is clicked.
virtual void LinkClicked(); virtual void OpenHelpPage();
private: private:
DISALLOW_COPY_AND_ASSIGN(ConfirmBubbleModel); DISALLOW_COPY_AND_ASSIGN(ConfirmBubbleModel);
......
...@@ -54,7 +54,7 @@ base::string16 TestConfirmBubbleModel::GetLinkText() const { ...@@ -54,7 +54,7 @@ base::string16 TestConfirmBubbleModel::GetLinkText() const {
return base::ASCIIToUTF16("Link"); return base::ASCIIToUTF16("Link");
} }
void TestConfirmBubbleModel::LinkClicked() { void TestConfirmBubbleModel::OpenHelpPage() {
if (link_clicked_) if (link_clicked_)
*link_clicked_ = true; *link_clicked_ = true;
} }
...@@ -27,7 +27,7 @@ class TestConfirmBubbleModel : public ConfirmBubbleModel { ...@@ -27,7 +27,7 @@ class TestConfirmBubbleModel : public ConfirmBubbleModel {
void Accept() override; void Accept() override;
void Cancel() override; void Cancel() override;
base::string16 GetLinkText() const override; base::string16 GetLinkText() const override;
void LinkClicked() override; void OpenHelpPage() override;
private: private:
bool* model_deleted_; bool* model_deleted_;
......
...@@ -11,15 +11,17 @@ ...@@ -11,15 +11,17 @@
#include "chrome/browser/ui/confirm_bubble_model.h" #include "chrome/browser/ui/confirm_bubble_model.h"
#include "chrome/browser/ui/views/harmony/chrome_layout_provider.h" #include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
#include "components/constrained_window/constrained_window_views.h" #include "components/constrained_window/constrained_window_views.h"
#include "components/vector_icons/vector_icons.h"
#include "ui/base/ui_features.h" #include "ui/base/ui_features.h"
#include "ui/views/controls/button/image_button.h"
#include "ui/views/controls/button/image_button_factory.h"
#include "ui/views/controls/label.h" #include "ui/views/controls/label.h"
#include "ui/views/controls/link.h"
#include "ui/views/layout/grid_layout.h" #include "ui/views/layout/grid_layout.h"
#include "ui/views/widget/widget.h" #include "ui/views/widget/widget.h"
ConfirmBubbleViews::ConfirmBubbleViews( ConfirmBubbleViews::ConfirmBubbleViews(
std::unique_ptr<ConfirmBubbleModel> model) std::unique_ptr<ConfirmBubbleModel> model)
: model_(std::move(model)), link_(NULL) { : model_(std::move(model)), help_button_(nullptr) {
set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType( set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType(
views::TEXT, views::TEXT)); views::TEXT, views::TEXT));
views::GridLayout* layout = views::GridLayout* layout =
...@@ -40,10 +42,9 @@ ConfirmBubbleViews::ConfirmBubbleViews( ...@@ -40,10 +42,9 @@ ConfirmBubbleViews::ConfirmBubbleViews(
layout->StartRow(0, 0); layout->StartRow(0, 0);
layout->AddView(label); layout->AddView(label);
// Initialize the link. // Initialize the help button.
link_ = new views::Link(model_->GetLinkText()); help_button_ = CreateVectorImageButton(this);
link_->set_listener(this); SetImageFromVectorIcon(help_button_, vector_icons::kHelpOutlineIcon);
link_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
chrome::RecordDialogCreation(chrome::DialogIdentifier::CONFIRM_BUBBLE); chrome::RecordDialogCreation(chrome::DialogIdentifier::CONFIRM_BUBBLE);
} }
...@@ -77,7 +78,7 @@ bool ConfirmBubbleViews::IsDialogButtonEnabled(ui::DialogButton button) const { ...@@ -77,7 +78,7 @@ bool ConfirmBubbleViews::IsDialogButtonEnabled(ui::DialogButton button) const {
} }
views::View* ConfirmBubbleViews::CreateExtraView() { views::View* ConfirmBubbleViews::CreateExtraView() {
return link_; return help_button_;
} }
bool ConfirmBubbleViews::Cancel() { bool ConfirmBubbleViews::Cancel() {
...@@ -98,9 +99,14 @@ base::string16 ConfirmBubbleViews::GetWindowTitle() const { ...@@ -98,9 +99,14 @@ base::string16 ConfirmBubbleViews::GetWindowTitle() const {
return model_->GetTitle(); return model_->GetTitle();
} }
void ConfirmBubbleViews::LinkClicked(views::Link* source, int event_flags) { bool ConfirmBubbleViews::ShouldShowCloseButton() const {
if (source == link_) { return false;
model_->LinkClicked(); }
void ConfirmBubbleViews::ButtonPressed(views::Button* sender,
const ui::Event& event) {
if (sender == help_button_) {
model_->OpenHelpPage();
GetWidget()->Close(); GetWidget()->Close();
} }
} }
......
...@@ -9,22 +9,26 @@ ...@@ -9,22 +9,26 @@
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/macros.h" #include "base/macros.h"
#include "ui/views/controls/link_listener.h" #include "ui/views/controls/button/button.h"
#include "ui/views/window/dialog_delegate.h" #include "ui/views/window/dialog_delegate.h"
class ConfirmBubbleModel; class ConfirmBubbleModel;
// A dialog (with the standard Title/(x)/[OK]/[Cancel] UI elements), as well as namespace views {
// a message Label and optional Link. The dialog ultimately appears like this: class ImageButton;
}
// A dialog (with the standard Title/[OK]/[Cancel] UI elements), as well as
// a message Label and help (?) button. The dialog ultimately appears like this:
// +------------------------+ // +------------------------+
// | Title (x) | // | Title |
// | Label | // | Label |
// | Link [OK] [Cancel] | // | (?) [OK] [Cancel] |
// +------------------------+ // +------------------------+
// //
// TODO(msw): Remove this class or merge it with DialogDelegateView. // TODO(msw): Remove this class or merge it with DialogDelegateView.
class ConfirmBubbleViews : public views::DialogDelegateView, class ConfirmBubbleViews : public views::DialogDelegateView,
public views::LinkListener { public views::ButtonListener {
public: public:
explicit ConfirmBubbleViews(std::unique_ptr<ConfirmBubbleModel> model); explicit ConfirmBubbleViews(std::unique_ptr<ConfirmBubbleModel> model);
...@@ -41,15 +45,16 @@ class ConfirmBubbleViews : public views::DialogDelegateView, ...@@ -41,15 +45,16 @@ class ConfirmBubbleViews : public views::DialogDelegateView,
// views::WidgetDelegate implementation. // views::WidgetDelegate implementation.
ui::ModalType GetModalType() const override; ui::ModalType GetModalType() const override;
base::string16 GetWindowTitle() const override; base::string16 GetWindowTitle() const override;
bool ShouldShowCloseButton() const override;
// views::LinkListener implementation. // views::ButtonListener implementation.
void LinkClicked(views::Link* source, int event_flags) override; void ButtonPressed(views::Button* sender, const ui::Event& event) override;
private: private:
// The model to customize this bubble view. // The model to customize this bubble view.
std::unique_ptr<ConfirmBubbleModel> model_; std::unique_ptr<ConfirmBubbleModel> model_;
views::Link* link_; views::ImageButton* help_button_;
DISALLOW_COPY_AND_ASSIGN(ConfirmBubbleViews); DISALLOW_COPY_AND_ASSIGN(ConfirmBubbleViews);
}; };
......
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