Commit 90a73c37 authored by Tommy Martino's avatar Tommy Martino Committed by Commit Bot

[WebPayments] Clicking selected row goes back

Changing behavior when clicking on the currently selected item in an
ItemList. Was a no-op; now goes back to the main sheet.

Bug: 748552
Change-Id: I43f716145c059b6d95d5f01526b1224528fb0ad3
Reviewed-on: https://chromium-review.googlesource.com/585370Reviewed-by: default avatarAnthony Vallee-Dubois <anthonyvd@chromium.org>
Commit-Queue: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491751}
parent b3840f43
...@@ -186,7 +186,8 @@ PaymentMethodViewController::PaymentMethodViewController( ...@@ -186,7 +186,8 @@ PaymentMethodViewController::PaymentMethodViewController(
PaymentRequestSpec* spec, PaymentRequestSpec* spec,
PaymentRequestState* state, PaymentRequestState* state,
PaymentRequestDialogView* dialog) PaymentRequestDialogView* dialog)
: PaymentRequestSheetController(spec, state, dialog) { : PaymentRequestSheetController(spec, state, dialog),
payment_method_list_(dialog) {
const std::vector<std::unique_ptr<PaymentInstrument>>& available_instruments = const std::vector<std::unique_ptr<PaymentInstrument>>& available_instruments =
state->available_instruments(); state->available_instruments();
for (const auto& instrument : available_instruments) { for (const auto& instrument : available_instruments) {
......
...@@ -100,15 +100,11 @@ IN_PROC_BROWSER_TEST_F(PaymentMethodViewControllerTest, ...@@ -100,15 +100,11 @@ IN_PROC_BROWSER_TEST_F(PaymentMethodViewControllerTest,
OpenPaymentMethodScreen(); OpenPaymentMethodScreen();
list_view = dialog_view()->GetViewByID( list_view = dialog_view()->GetViewByID(
static_cast<int>(DialogViewID::PAYMENT_METHOD_SHEET_LIST_VIEW)); static_cast<int>(DialogViewID::PAYMENT_METHOD_SHEET_LIST_VIEW));
// Clicking on the second card again should not modify any state.
ClickOnDialogViewAndWait(list_view->child_at(1));
checkmark_view = list_view->child_at(0)->GetViewByID( ResetEventObserver(DialogEvent::BACK_NAVIGATION);
static_cast<int>(DialogViewID::CHECKMARK_VIEW)); // Clicking on the second card again should not modify any state, and should
checkmark_view2 = list_view->child_at(1)->GetViewByID( // return to the main payment sheet.
static_cast<int>(DialogViewID::CHECKMARK_VIEW)); ClickOnDialogViewAndWait(list_view->child_at(1));
EXPECT_FALSE(checkmark_view->visible());
EXPECT_TRUE(checkmark_view2->visible());
EXPECT_EQ(request->state()->available_instruments().back().get(), EXPECT_EQ(request->state()->available_instruments().back().get(),
request->state()->selected_instrument()); request->state()->selected_instrument());
......
...@@ -4,6 +4,9 @@ ...@@ -4,6 +4,9 @@
#include "chrome/browser/ui/views/payments/payment_request_item_list.h" #include "chrome/browser/ui/views/payments/payment_request_item_list.h"
#include <utility>
#include "chrome/browser/ui/views/payments/payment_request_dialog_view.h"
#include "chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h" #include "chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h"
#include "chrome/browser/ui/views/payments/payment_request_views_util.h" #include "chrome/browser/ui/views/payments/payment_request_views_util.h"
#include "components/payments/content/payment_request_state.h" #include "components/payments/content/payment_request_state.h"
...@@ -152,6 +155,10 @@ void PaymentRequestItemList::Item::ButtonPressed(views::Button* sender, ...@@ -152,6 +155,10 @@ void PaymentRequestItemList::Item::ButtonPressed(views::Button* sender,
const ui::Event& event) { const ui::Event& event) {
if (sender->id() == static_cast<int>(DialogViewID::EDIT_ITEM_BUTTON)) { if (sender->id() == static_cast<int>(DialogViewID::EDIT_ITEM_BUTTON)) {
EditButtonPressed(); EditButtonPressed();
} else if (selected_) {
// |dialog()| may be null in tests
if (list_->dialog())
list_->dialog()->GoBack();
} else if (CanBeSelected()) { } else if (CanBeSelected()) {
list()->SelectItem(this); list()->SelectItem(this);
} else { } else {
...@@ -170,7 +177,8 @@ void PaymentRequestItemList::Item::UpdateAccessibleName() { ...@@ -170,7 +177,8 @@ void PaymentRequestItemList::Item::UpdateAccessibleName() {
SetAccessibleName(accessible_content); SetAccessibleName(accessible_content);
} }
PaymentRequestItemList::PaymentRequestItemList() : selected_item_(nullptr) {} PaymentRequestItemList::PaymentRequestItemList(PaymentRequestDialogView* dialog)
: selected_item_(nullptr), dialog_(dialog) {}
PaymentRequestItemList::~PaymentRequestItemList() {} PaymentRequestItemList::~PaymentRequestItemList() {}
......
...@@ -19,6 +19,7 @@ class View; ...@@ -19,6 +19,7 @@ class View;
namespace payments { namespace payments {
class PaymentRequestDialogView;
class PaymentRequestSpec; class PaymentRequestSpec;
class PaymentRequestState; class PaymentRequestState;
...@@ -123,7 +124,7 @@ class PaymentRequestItemList { ...@@ -123,7 +124,7 @@ class PaymentRequestItemList {
DISALLOW_COPY_AND_ASSIGN(Item); DISALLOW_COPY_AND_ASSIGN(Item);
}; };
PaymentRequestItemList(); explicit PaymentRequestItemList(PaymentRequestDialogView* dialog);
virtual ~PaymentRequestItemList(); virtual ~PaymentRequestItemList();
// Adds an item to this list. |item->list()| should return this object. // Adds an item to this list. |item->list()| should return this object.
...@@ -140,6 +141,8 @@ class PaymentRequestItemList { ...@@ -140,6 +141,8 @@ class PaymentRequestItemList {
// Deselects the currently selected item and selects |item| instead. // Deselects the currently selected item and selects |item| instead.
void SelectItem(Item* item); void SelectItem(Item* item);
PaymentRequestDialogView* dialog() { return dialog_; }
private: private:
// Unselects the currently selected item. This is private so that the list can // Unselects the currently selected item. This is private so that the list can
// use it when selecting a new item while avoiding consumers of this class // use it when selecting a new item while avoiding consumers of this class
...@@ -148,6 +151,7 @@ class PaymentRequestItemList { ...@@ -148,6 +151,7 @@ class PaymentRequestItemList {
std::vector<std::unique_ptr<Item>> items_; std::vector<std::unique_ptr<Item>> items_;
Item* selected_item_; Item* selected_item_;
PaymentRequestDialogView* dialog_;
DISALLOW_COPY_AND_ASSIGN(PaymentRequestItemList); DISALLOW_COPY_AND_ASSIGN(PaymentRequestItemList);
}; };
......
...@@ -59,7 +59,7 @@ class TestListItem : public PaymentRequestItemList::Item { ...@@ -59,7 +59,7 @@ class TestListItem : public PaymentRequestItemList::Item {
} // namespace } // namespace
TEST(PaymentRequestItemListTest, TestAddItem) { TEST(PaymentRequestItemListTest, TestAddItem) {
PaymentRequestItemList list; PaymentRequestItemList list(nullptr);
std::unique_ptr<views::View> list_view = list.CreateListView(); std::unique_ptr<views::View> list_view = list.CreateListView();
EXPECT_FALSE(list_view->has_children()); EXPECT_FALSE(list_view->has_children());
...@@ -90,7 +90,7 @@ TEST(PaymentRequestItemListTest, TestAddItem) { ...@@ -90,7 +90,7 @@ TEST(PaymentRequestItemListTest, TestAddItem) {
} }
TEST(PaymentRequestItemListTest, TestSelectItemResultsInSingleItemSelected) { TEST(PaymentRequestItemListTest, TestSelectItemResultsInSingleItemSelected) {
PaymentRequestItemList list; PaymentRequestItemList list(nullptr);
std::vector<std::unique_ptr<TestListItem>> items; std::vector<std::unique_ptr<TestListItem>> items;
items.push_back(base::MakeUnique<TestListItem>(&list, false)); items.push_back(base::MakeUnique<TestListItem>(&list, false));
......
...@@ -368,7 +368,7 @@ ProfileListViewController::ProfileListViewController( ...@@ -368,7 +368,7 @@ ProfileListViewController::ProfileListViewController(
PaymentRequestSpec* spec, PaymentRequestSpec* spec,
PaymentRequestState* state, PaymentRequestState* state,
PaymentRequestDialogView* dialog) PaymentRequestDialogView* dialog)
: PaymentRequestSheetController(spec, state, dialog) {} : PaymentRequestSheetController(spec, state, dialog), list_(dialog) {}
ProfileListViewController::~ProfileListViewController() {} ProfileListViewController::~ProfileListViewController() {}
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
#include "chrome/browser/ui/views/payments/shipping_option_view_controller.h" #include "chrome/browser/ui/views/payments/shipping_option_view_controller.h"
#include <memory>
#include "chrome/browser/ui/views/payments/payment_request_dialog_view.h" #include "chrome/browser/ui/views/payments/payment_request_dialog_view.h"
#include "chrome/browser/ui/views/payments/payment_request_views_util.h" #include "chrome/browser/ui/views/payments/payment_request_views_util.h"
#include "components/payments/content/payment_request_spec.h" #include "components/payments/content/payment_request_spec.h"
...@@ -82,7 +84,8 @@ ShippingOptionViewController::ShippingOptionViewController( ...@@ -82,7 +84,8 @@ ShippingOptionViewController::ShippingOptionViewController(
PaymentRequestSpec* spec, PaymentRequestSpec* spec,
PaymentRequestState* state, PaymentRequestState* state,
PaymentRequestDialogView* dialog) PaymentRequestDialogView* dialog)
: PaymentRequestSheetController(spec, state, dialog) { : PaymentRequestSheetController(spec, state, dialog),
shipping_option_list_(dialog) {
spec->AddObserver(this); spec->AddObserver(this);
for (const auto& option : spec->GetShippingOptions()) { for (const auto& option : spec->GetShippingOptions()) {
shipping_option_list_.AddItem(base::MakeUnique<ShippingOptionItem>( shipping_option_list_.AddItem(base::MakeUnique<ShippingOptionItem>(
......
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