Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1582)

Unified Diff: chrome/browser/ui/views/intent_picker_bubble_view.cc

Issue 2436543003: reland: Reusing Ok/Cancel buttons for intent picker (Closed)
Patch Set: Modifying the way I create a dummy event for the testing. Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/views/intent_picker_bubble_view.cc
diff --git a/chrome/browser/ui/views/intent_picker_bubble_view.cc b/chrome/browser/ui/views/intent_picker_bubble_view.cc
index f805497fdfcbe26a0f4d2c9f66b2797e7893ca6e..18f8127a957d45e993a11c3fb6219f79060d989c 100644
--- a/chrome/browser/ui/views/intent_picker_bubble_view.cc
+++ b/chrome/browser/ui/views/intent_picker_bubble_view.cc
@@ -15,7 +15,9 @@
#include "content/public/browser/navigation_handle.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/material_design/material_design_controller.h"
#include "ui/gfx/canvas.h"
+#include "ui/views/animation/ink_drop_host_view.h"
#include "ui/views/border.h"
#include "ui/views/controls/button/image_button.h"
#include "ui/views/controls/scroll_view.h"
@@ -29,46 +31,81 @@ namespace {
// Using |kMaxAppResults| as a measure of how many apps we want to show.
constexpr size_t kMaxAppResults = arc::ArcNavigationThrottle::kMaxAppResults;
// Main components sizes
+constexpr int kDialogDelegateInsets = 16;
constexpr int kRowHeight = 40;
constexpr int kMaxWidth = 320;
-constexpr int kHeaderHeight = 60;
-constexpr int kFooterHeight = 68;
-// Inter components padding
-constexpr int kButtonSeparation = 8;
-constexpr int kLabelImageSeparation = 12;
// UI position wrt the Top Container
constexpr int kTopContainerMerge = 3;
-constexpr size_t kAppTagNoneSelected = static_cast<size_t>(-1);
-// Arbitrary negative values to use as tags on the |always_button_| and
-// |just_once_button_|. These are negative to differentiate from the app's tags
-// which are always >= 0.
-enum class Option : int { ALWAYS = -2, JUST_ONCE };
+constexpr char kInvalidPackageName[] = "";
} // namespace
+// IntentPickerLabelButton
+
+// A button that represents a candidate intent handler.
+class IntentPickerLabelButton : public views::LabelButton {
+ public:
+ IntentPickerLabelButton(views::ButtonListener* listener,
+ gfx::Image* icon,
+ const std::string& package_name,
+ const std::string& activity_name)
+ : LabelButton(listener,
+ base::UTF8ToUTF16(base::StringPiece(activity_name))),
+ package_name_(package_name) {
+ SetHorizontalAlignment(gfx::ALIGN_LEFT);
+ SetFocusBehavior(View::FocusBehavior::ALWAYS);
+ SetMinSize(gfx::Size(kMaxWidth, kRowHeight));
+ SetInkDropMode(InkDropMode::ON);
+ if (!icon->IsEmpty())
+ SetImage(views::ImageButton::STATE_NORMAL, *icon->ToImageSkia());
+ SetBorder(views::Border::CreateEmptyBorder(10, 16, 10, 0));
+ }
+
+ SkColor GetInkDropBaseColor() const override { return SK_ColorBLACK; }
+
+ void MarkAsUnselected(const ui::Event* event) {
+ AnimateInkDrop(views::InkDropState::DEACTIVATED,
+ ui::LocatedEvent::FromIfValid(event));
+ }
+
+ void MarkAsSelected(const ui::Event* event) {
+ AnimateInkDrop(views::InkDropState::ACTIVATED,
+ ui::LocatedEvent::FromIfValid(event));
+ }
+
+ views::InkDropState GetTargetInkDropState() {
+ return ink_drop()->GetTargetInkDropState();
+ }
+
+ private:
+ std::string package_name_;
+
+ DISALLOW_COPY_AND_ASSIGN(IntentPickerLabelButton);
+};
+
// static
void IntentPickerBubbleView::ShowBubble(
content::WebContents* web_contents,
- const std::vector<NameAndIcon>& app_info,
- const ThrottleCallback& throttle_cb) {
+ const std::vector<AppInfo>& app_info,
+ const IntentPickerResponse& intent_picker_cb) {
Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
- if (!browser) {
- throttle_cb.Run(kAppTagNoneSelected,
- arc::ArcNavigationThrottle::CloseReason::ERROR);
+ if (!browser || !BrowserView::GetBrowserViewForBrowser(browser)) {
+ intent_picker_cb.Run(kInvalidPackageName,
+ arc::ArcNavigationThrottle::CloseReason::ERROR);
return;
}
BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
- if (!browser_view) {
- throttle_cb.Run(kAppTagNoneSelected,
- arc::ArcNavigationThrottle::CloseReason::ERROR);
- return;
- }
-
IntentPickerBubbleView* delegate =
- new IntentPickerBubbleView(app_info, throttle_cb, web_contents);
- delegate->set_margins(gfx::Insets());
+ new IntentPickerBubbleView(app_info, intent_picker_cb, web_contents);
+ // TODO(djacobo): Remove the left and right insets when
+ // http://crbug.com/656662 gets fixed.
+ // Add a 1-pixel extra boundary left and right when using secondary UI.
+ if (ui::MaterialDesignController::IsSecondaryUiMaterial())
+ delegate->set_margins(gfx::Insets(16, 1, 0, 1));
+ else
+ delegate->set_margins(gfx::Insets(16, 0, 0, 0));
delegate->set_parent_window(browser_view->GetNativeWindow());
views::Widget* widget =
views::BubbleDialogDelegateView::CreateBubble(delegate);
@@ -85,104 +122,65 @@ void IntentPickerBubbleView::ShowBubble(
browser_view->GetTopContainerBoundsInScreen().width(),
browser_view->GetTopContainerBoundsInScreen().height() -
kTopContainerMerge));
+ delegate->GetDialogClientView()->set_button_row_insets(
+ gfx::Insets(kDialogDelegateInsets));
+ delegate->GetDialogClientView()->Layout();
+ delegate->GetIntentPickerLabelButtonAt(0)->MarkAsSelected(nullptr);
widget->Show();
}
// static
std::unique_ptr<IntentPickerBubbleView>
IntentPickerBubbleView::CreateBubbleView(
- const std::vector<NameAndIcon>& app_info,
- const ThrottleCallback& throttle_cb,
+ const std::vector<AppInfo>& app_info,
+ const IntentPickerResponse& intent_picker_cb,
content::WebContents* web_contents) {
std::unique_ptr<IntentPickerBubbleView> bubble(
- new IntentPickerBubbleView(app_info, throttle_cb, web_contents));
+ new IntentPickerBubbleView(app_info, intent_picker_cb, web_contents));
bubble->Init();
return bubble;
}
-void IntentPickerBubbleView::Init() {
- SkColor button_text_color = SkColorSetRGB(0x42, 0x85, 0xf4);
+bool IntentPickerBubbleView::Accept() {
+ RunCallback(app_info_[selected_app_tag_].package_name,
+ arc::ArcNavigationThrottle::CloseReason::JUST_ONCE_PRESSED);
+ return true;
+}
+
+bool IntentPickerBubbleView::Cancel() {
+ RunCallback(app_info_[selected_app_tag_].package_name,
+ arc::ArcNavigationThrottle::CloseReason::ALWAYS_PRESSED);
+ return true;
+}
+
+bool IntentPickerBubbleView::Close() {
+ // Whenever closing the bubble without pressing |Just once| or |Always| we
+ // need to report back that the user didn't select anything.
+ RunCallback(kInvalidPackageName,
+ arc::ArcNavigationThrottle::CloseReason::DIALOG_DEACTIVATED);
+ return true;
+}
+void IntentPickerBubbleView::Init() {
views::BoxLayout* general_layout =
new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0);
SetLayoutManager(general_layout);
- // Create a view for the upper part of the UI, managed by a GridLayout to
- // allow precise padding.
- View* header = new View();
- views::GridLayout* header_layout = new views::GridLayout(header);
- header->SetLayoutManager(header_layout);
- views::Label* open_with = new views::Label(
- l10n_util::GetStringUTF16(IDS_INTENT_PICKER_BUBBLE_VIEW_OPEN_WITH));
- open_with->SetHorizontalAlignment(gfx::ALIGN_LEFT);
- open_with->SetFontList(gfx::FontList("Roboto-medium, 15px"));
-
- views::ColumnSet* column_set = header_layout->AddColumnSet(0);
- column_set->AddPaddingColumn(0, 16);
- column_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 1,
- views::GridLayout::FIXED, kMaxWidth, kMaxWidth);
- column_set->AddPaddingColumn(0, 16);
- // Tell the GridLayout where to start, then proceed to place the views.
- header_layout->AddPaddingRow(0.0, 21);
- header_layout->StartRow(0, 0);
- header_layout->AddView(open_with);
- header_layout->AddPaddingRow(0.0, 24);
-
- always_button_ = new views::LabelButton(
- this, l10n_util::GetStringUTF16(IDS_INTENT_PICKER_BUBBLE_VIEW_ALWAYS));
- always_button_->SetFocusBehavior(View::FocusBehavior::ALWAYS);
- always_button_->SetFontList(gfx::FontList("Roboto-medium, 13px"));
- always_button_->set_tag(static_cast<int>(Option::ALWAYS));
- always_button_->SetMinSize(gfx::Size(80, 32));
- always_button_->SetTextColor(views::Button::STATE_DISABLED, SK_ColorGRAY);
- always_button_->SetTextColor(views::Button::STATE_NORMAL, button_text_color);
- always_button_->SetTextColor(views::Button::STATE_HOVERED, button_text_color);
- always_button_->SetHorizontalAlignment(gfx::ALIGN_CENTER);
- always_button_->SetState(views::Button::STATE_DISABLED);
- always_button_->SetBorder(views::Border::CreateEmptyBorder(gfx::Insets(16)));
-
- just_once_button_ = new views::LabelButton(
- this, l10n_util::GetStringUTF16(IDS_INTENT_PICKER_BUBBLE_VIEW_JUST_ONCE));
- just_once_button_->SetFocusBehavior(View::FocusBehavior::ALWAYS);
- just_once_button_->SetFontList(gfx::FontList("Roboto-medium, 13px"));
- just_once_button_->set_tag(static_cast<int>(Option::JUST_ONCE));
- just_once_button_->SetMinSize(gfx::Size(80, 32));
- just_once_button_->SetState(views::Button::STATE_DISABLED);
- just_once_button_->SetTextColor(views::Button::STATE_DISABLED, SK_ColorGRAY);
- just_once_button_->SetTextColor(views::Button::STATE_NORMAL,
- button_text_color);
- just_once_button_->SetTextColor(views::Button::STATE_HOVERED,
- button_text_color);
- just_once_button_->SetHorizontalAlignment(gfx::ALIGN_CENTER);
- just_once_button_->SetBorder(
- views::Border::CreateEmptyBorder(gfx::Insets(16)));
-
- header_layout->StartRow(0, 0);
- AddChildView(header);
-
// Creates a view to hold the views for each app.
views::View* scrollable_view = new views::View();
views::BoxLayout* scrollable_layout =
new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0);
scrollable_view->SetLayoutManager(scrollable_layout);
for (size_t i = 0; i < app_info_.size(); ++i) {
- views::LabelButton* tmp_label = new views::LabelButton(
- this, base::UTF8ToUTF16(base::StringPiece(app_info_[i].first)));
- tmp_label->SetFocusBehavior(View::FocusBehavior::ALWAYS);
- tmp_label->SetFontList(gfx::FontList("Roboto-regular, 13px"));
- tmp_label->SetImageLabelSpacing(kLabelImageSeparation);
- tmp_label->SetMinSize(gfx::Size(kMaxWidth, kRowHeight));
- tmp_label->SetMaxSize(gfx::Size(kMaxWidth, kRowHeight));
- tmp_label->set_tag(i);
- if (!app_info_[i].second.IsEmpty()) {
- tmp_label->SetImage(views::ImageButton::STATE_NORMAL,
- *app_info_[i].second.ToImageSkia());
- }
- tmp_label->SetBorder(views::Border::CreateEmptyBorder(10, 16, 10, 0));
- scrollable_view->AddChildViewAt(tmp_label, i);
+ IntentPickerLabelButton* app_button = new IntentPickerLabelButton(
+ this, &app_info_[i].icon, app_info_[i].package_name,
+ app_info_[i].activity_name);
+ app_button->set_tag(i);
+ scrollable_view->AddChildViewAt(app_button, i);
}
scroll_view_ = new views::ScrollView();
+ scroll_view_->EnableViewPortLayer();
scroll_view_->SetContents(scrollable_view);
// Setting a customized ScrollBar which is shown only when the mouse pointer
// is inside the ScrollView.
@@ -197,34 +195,28 @@ void IntentPickerBubbleView::Init() {
scroll_view_->ClipHeightTo(kRowHeight, (kMaxAppResults + 0.5) * kRowHeight);
}
AddChildView(scroll_view_);
+}
+
+base::string16 IntentPickerBubbleView::GetWindowTitle() const {
+ return l10n_util::GetStringUTF16(IDS_INTENT_PICKER_BUBBLE_VIEW_OPEN_WITH);
+}
- // The lower part of the Picker contains only the 2 buttons
- // |just_once_button_| and |always_button_|.
- View* footer = new View();
- footer->SetBorder(views::Border::CreateEmptyBorder(24, 0, 12, 12));
- views::BoxLayout* buttons_layout = new views::BoxLayout(
- views::BoxLayout::kHorizontal, 0, 0, kButtonSeparation);
- footer->SetLayoutManager(buttons_layout);
-
- buttons_layout->set_main_axis_alignment(
- views::BoxLayout::MAIN_AXIS_ALIGNMENT_END);
- footer->AddChildView(just_once_button_);
- footer->AddChildView(always_button_);
- AddChildView(footer);
+base::string16 IntentPickerBubbleView::GetDialogButtonLabel(
+ ui::DialogButton button) const {
+ return l10n_util::GetStringUTF16(button == ui::DIALOG_BUTTON_OK
+ ? IDS_INTENT_PICKER_BUBBLE_VIEW_JUST_ONCE
+ : IDS_INTENT_PICKER_BUBBLE_VIEW_ALWAYS);
}
IntentPickerBubbleView::IntentPickerBubbleView(
- const std::vector<NameAndIcon>& app_info,
- ThrottleCallback throttle_cb,
+ const std::vector<AppInfo>& app_info,
+ IntentPickerResponse intent_picker_cb,
content::WebContents* web_contents)
: views::BubbleDialogDelegateView(nullptr /* anchor_view */,
views::BubbleBorder::TOP_CENTER),
WebContentsObserver(web_contents),
- was_callback_run_(false),
- throttle_cb_(throttle_cb),
- selected_app_tag_(kAppTagNoneSelected),
- always_button_(nullptr),
- just_once_button_(nullptr),
+ intent_picker_cb_(intent_picker_cb),
+ selected_app_tag_(0),
scroll_view_(nullptr),
app_info_(app_info) {}
@@ -235,47 +227,18 @@ IntentPickerBubbleView::~IntentPickerBubbleView() {
// If the widget gets closed without an app being selected we still need to use
// the callback so the caller can Resume the navigation.
void IntentPickerBubbleView::OnWidgetDestroying(views::Widget* widget) {
- if (!was_callback_run_) {
- was_callback_run_ = true;
- throttle_cb_.Run(
- kAppTagNoneSelected,
- arc::ArcNavigationThrottle::CloseReason::DIALOG_DEACTIVATED);
- }
-}
-
-int IntentPickerBubbleView::GetDialogButtons() const {
- return ui::DIALOG_BUTTON_NONE;
+ RunCallback(kInvalidPackageName,
+ arc::ArcNavigationThrottle::CloseReason::DIALOG_DEACTIVATED);
}
void IntentPickerBubbleView::ButtonPressed(views::Button* sender,
const ui::Event& event) {
- switch (sender->tag()) {
- case static_cast<int>(Option::ALWAYS):
- was_callback_run_ = true;
- throttle_cb_.Run(selected_app_tag_,
- arc::ArcNavigationThrottle::CloseReason::ALWAYS_PRESSED);
- GetWidget()->Close();
- break;
- case static_cast<int>(Option::JUST_ONCE):
- was_callback_run_ = true;
- throttle_cb_.Run(
- selected_app_tag_,
- arc::ArcNavigationThrottle::CloseReason::JUST_ONCE_PRESSED);
- GetWidget()->Close();
- break;
- default:
- // The selected app must be a value in the range [0, app_info_.size()-1].
- DCHECK(static_cast<size_t>(sender->tag()) < app_info_.size());
- // The user cannot click on the |always_button_| or |just_once_button_|
- // unless an explicit app has been selected.
- if (selected_app_tag_ != kAppTagNoneSelected)
- SetLabelButtonBackgroundColor(selected_app_tag_, SK_ColorWHITE);
- selected_app_tag_ = sender->tag();
- SetLabelButtonBackgroundColor(selected_app_tag_,
- SkColorSetRGB(0xeb, 0xeb, 0xeb));
- always_button_->SetState(views::Button::STATE_NORMAL);
- just_once_button_->SetState(views::Button::STATE_NORMAL);
- }
+ // The selected app must be a value in the range [0, app_info_.size()-1].
+ DCHECK_LT(static_cast<size_t>(sender->tag()), app_info_.size());
+ GetIntentPickerLabelButtonAt(selected_app_tag_)->MarkAsUnselected(&event);
+
+ selected_app_tag_ = sender->tag();
+ GetIntentPickerLabelButtonAt(selected_app_tag_)->MarkAsSelected(&event);
}
gfx::Size IntentPickerBubbleView::GetPreferredSize() const {
@@ -289,29 +252,48 @@ gfx::Size IntentPickerBubbleView::GetPreferredSize() const {
} else {
apps_height *= kRowHeight;
}
- ps.set_height(apps_height + kHeaderHeight + kFooterHeight);
+ ps.set_height(apps_height + kDialogDelegateInsets);
return ps;
}
// If the actual web_contents gets destroyed in the middle of the process we
// should inform the caller about this error.
void IntentPickerBubbleView::WebContentsDestroyed() {
- if (!was_callback_run_) {
- was_callback_run_ = true;
- throttle_cb_.Run(kAppTagNoneSelected,
- arc::ArcNavigationThrottle::CloseReason::ERROR);
- }
GetWidget()->Close();
}
-views::LabelButton* IntentPickerBubbleView::GetLabelButtonAt(size_t index) {
+IntentPickerLabelButton* IntentPickerBubbleView::GetIntentPickerLabelButtonAt(
+ size_t index) {
views::View* temp_contents = scroll_view_->contents();
- return static_cast<views::LabelButton*>(temp_contents->child_at(index));
+ return static_cast<IntentPickerLabelButton*>(temp_contents->child_at(index));
+}
+
+void IntentPickerBubbleView::RunCallback(
+ std::string package,
+ arc::ArcNavigationThrottle::CloseReason close_reason) {
+ if (!intent_picker_cb_.is_null()) {
+ // We must ensure |intent_picker_cb_| is only Run() once, this is why we
+ // have a temporary |callback| helper, so we can set the original callback
+ // to null and still report back to whoever started the UI.
+ auto callback = intent_picker_cb_;
+ intent_picker_cb_.Reset();
+ callback.Run(package, close_reason);
+ }
+}
+
+gfx::ImageSkia IntentPickerBubbleView::GetAppImageForTesting(size_t index) {
+ return GetIntentPickerLabelButtonAt(index)->GetImage(
+ views::Button::ButtonState::STATE_NORMAL);
+}
+
+views::InkDropState IntentPickerBubbleView::GetInkDropStateForTesting(
+ size_t index) {
+ return GetIntentPickerLabelButtonAt(index)->GetTargetInkDropState();
}
-void IntentPickerBubbleView::SetLabelButtonBackgroundColor(size_t index,
- SkColor color) {
- views::LabelButton* temp_lb = GetLabelButtonAt(index);
- temp_lb->set_background(views::Background::CreateSolidBackground(color));
- temp_lb->SchedulePaint();
+void IntentPickerBubbleView::PressButtonForTesting(size_t index,
+ const ui::Event& event) {
+ views::Button* button =
+ static_cast<views::Button*>(GetIntentPickerLabelButtonAt(index));
+ ButtonPressed(button, event);
}
« no previous file with comments | « chrome/browser/ui/views/intent_picker_bubble_view.h ('k') | chrome/browser/ui/views/intent_picker_bubble_view_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698