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

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

Issue 2433733002: Revert "Reusing Ok/Cancel buttons for intent picker" (Closed)
Patch Set: 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 18f8127a957d45e993a11c3fb6219f79060d989c..f805497fdfcbe26a0f4d2c9f66b2797e7893ca6e 100644
--- a/chrome/browser/ui/views/intent_picker_bubble_view.cc
+++ b/chrome/browser/ui/views/intent_picker_bubble_view.cc
@@ -15,9 +15,7 @@
#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"
@@ -31,81 +29,46 @@ 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);
-constexpr char kInvalidPackageName[] = "";
+// 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 };
} // 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<AppInfo>& app_info,
- const IntentPickerResponse& intent_picker_cb) {
+ const std::vector<NameAndIcon>& app_info,
+ const ThrottleCallback& throttle_cb) {
Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
- if (!browser || !BrowserView::GetBrowserViewForBrowser(browser)) {
- intent_picker_cb.Run(kInvalidPackageName,
- arc::ArcNavigationThrottle::CloseReason::ERROR);
+ if (!browser) {
+ throttle_cb.Run(kAppTagNoneSelected,
+ 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, 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));
+ new IntentPickerBubbleView(app_info, throttle_cb, web_contents);
+ delegate->set_margins(gfx::Insets());
delegate->set_parent_window(browser_view->GetNativeWindow());
views::Widget* widget =
views::BubbleDialogDelegateView::CreateBubble(delegate);
@@ -122,65 +85,104 @@ 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<AppInfo>& app_info,
- const IntentPickerResponse& intent_picker_cb,
+ const std::vector<NameAndIcon>& app_info,
+ const ThrottleCallback& throttle_cb,
content::WebContents* web_contents) {
std::unique_ptr<IntentPickerBubbleView> bubble(
- new IntentPickerBubbleView(app_info, intent_picker_cb, web_contents));
+ new IntentPickerBubbleView(app_info, throttle_cb, web_contents));
bubble->Init();
return bubble;
}
-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() {
+ SkColor button_text_color = SkColorSetRGB(0x42, 0x85, 0xf4);
+
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) {
- 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);
+ 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);
}
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.
@@ -195,28 +197,34 @@ 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);
-}
-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);
+ // 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);
}
IntentPickerBubbleView::IntentPickerBubbleView(
- const std::vector<AppInfo>& app_info,
- IntentPickerResponse intent_picker_cb,
+ const std::vector<NameAndIcon>& app_info,
+ ThrottleCallback throttle_cb,
content::WebContents* web_contents)
: views::BubbleDialogDelegateView(nullptr /* anchor_view */,
views::BubbleBorder::TOP_CENTER),
WebContentsObserver(web_contents),
- intent_picker_cb_(intent_picker_cb),
- selected_app_tag_(0),
+ was_callback_run_(false),
+ throttle_cb_(throttle_cb),
+ selected_app_tag_(kAppTagNoneSelected),
+ always_button_(nullptr),
+ just_once_button_(nullptr),
scroll_view_(nullptr),
app_info_(app_info) {}
@@ -227,18 +235,47 @@ 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) {
- RunCallback(kInvalidPackageName,
- arc::ArcNavigationThrottle::CloseReason::DIALOG_DEACTIVATED);
+ 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;
}
void IntentPickerBubbleView::ButtonPressed(views::Button* sender,
const ui::Event& event) {
- // 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);
+ 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);
+ }
}
gfx::Size IntentPickerBubbleView::GetPreferredSize() const {
@@ -252,48 +289,29 @@ gfx::Size IntentPickerBubbleView::GetPreferredSize() const {
} else {
apps_height *= kRowHeight;
}
- ps.set_height(apps_height + kDialogDelegateInsets);
+ ps.set_height(apps_height + kHeaderHeight + kFooterHeight);
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();
}
-IntentPickerLabelButton* IntentPickerBubbleView::GetIntentPickerLabelButtonAt(
- size_t index) {
+views::LabelButton* IntentPickerBubbleView::GetLabelButtonAt(size_t index) {
views::View* temp_contents = scroll_view_->contents();
- 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();
+ return static_cast<views::LabelButton*>(temp_contents->child_at(index));
}
-void IntentPickerBubbleView::PressButtonForTesting(size_t index,
- const ui::Event& event) {
- views::Button* button =
- static_cast<views::Button*>(GetIntentPickerLabelButtonAt(index));
- ButtonPressed(button, event);
+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();
}
« 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