Skip to content

Commit

Permalink
[IBAN local save] Create chrome icon to reopen IBAN bubble view.
Browse files Browse the repository at this point in the history
This CL introduce chrome IBAN icon which can reopen hidden bubble (if
user clicks on [X] to close the dialog, or window is timeout) when the
user clicks on the icon.

Screenshot:
https://screenshot.googleplex.com/5zS54E7MizLEfgH.png

Bug: 1349109
Change-Id: I8cc91ea72cba7d8ced8fda889e1f22e20238a777
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4169150
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Qihui Zhao <qihuizhao@google.com>
Reviewed-by: Vinny Persky <vinnypersky@google.com>
Reviewed-by: Siyu An <siyua@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/main@{#1100811}
  • Loading branch information
Qihui Zhao authored and Chromium LUCI CQ committed Feb 3, 2023
1 parent 9936c68 commit 5c383b5
Show file tree
Hide file tree
Showing 23 changed files with 266 additions and 54 deletions.
1 change: 1 addition & 0 deletions chrome/app/chrome_command_ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
#define IDC_VIRTUAL_CARD_ENROLL 35032
#define IDC_FOLLOW 35033
#define IDC_UNFOLLOW 35034
#define IDC_SAVE_IBAN_FOR_PAGE 35035

// Page-manipulation commands that target a specified tab, which may not be the
// active one.
Expand Down
3 changes: 3 additions & 0 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -7653,6 +7653,9 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_TOOLTIP_SAVE_CREDIT_CARD_FAILURE" desc="The tooltip for the icon that shows the credit card is not successfully uploaded">
Can't save card right now
</message>
<message name="IDS_TOOLTIP_SAVE_IBAN" desc="The tooltip for the icon that shows the save IBAN (International Bank Account Number) bubble">
Save IBAN
</message>
<message name="IDS_TOOLTIP_MIGRATE_LOCAL_CARD" desc="Tooltip for the credit card icon that appears in the address bar. Icon is shown when a user has a credit card stored locally in Chrome but not in Google Pay.">
Save payment method
</message>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
f6c473730d7b0772ed6ac9682d72a106d3c90136
Original file line number Diff line number Diff line change
Expand Up @@ -470,11 +470,12 @@ std::u16string SaveCardBubbleControllerImpl::GetSavePaymentIconTooltipText()
}
}

bool SaveCardBubbleControllerImpl::ShouldShowSavingCardAnimation() const {
bool SaveCardBubbleControllerImpl::ShouldShowSavingPaymentAnimation() const {
return current_bubble_type_ == BubbleType::UPLOAD_IN_PROGRESS;
}

bool SaveCardBubbleControllerImpl::ShouldShowCardSavedLabelAnimation() const {
bool SaveCardBubbleControllerImpl::ShouldShowPaymentSavedLabelAnimation()
const {
return should_show_card_saved_label_animation_;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ class SaveCardBubbleControllerImpl

// SavePaymentIconController:
std::u16string GetSavePaymentIconTooltipText() const override;
bool ShouldShowSavingCardAnimation() const override;
bool ShouldShowCardSavedLabelAnimation() const override;
bool ShouldShowSavingPaymentAnimation() const override;
bool ShouldShowPaymentSavedLabelAnimation() const override;
bool ShouldShowSaveFailureBadge() const override;
void OnAnimationEnded() override;
bool IsIconVisible() const override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,9 @@ class SaveCardBubbleControllerImplTest : public BrowserWithTestWindowTest {
void ClickSaveButton() {
controller()->OnSaveButton({});
controller()->OnBubbleClosed(PaymentsBubbleClosedReason::kAccepted);
if (controller()->ShouldShowCardSavedLabelAnimation())
if (controller()->ShouldShowPaymentSavedLabelAnimation()) {
controller()->OnAnimationEnded();
}
}

protected:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace autofill {

class AutofillBubbleBase;
class IBAN;
enum class IBANBubbleType;
enum class IbanBubbleType;

// Interface that exposes controller functionality to save IBAN bubbles.
class SaveIbanBubbleController {
Expand Down Expand Up @@ -47,6 +47,9 @@ class SaveIbanBubbleController {
virtual void OnSaveButton(const std::u16string& nickname) = 0;
virtual void OnCancelButton() = 0;
virtual void OnBubbleClosed(PaymentsBubbleClosedReason closed_reason) = 0;

// Returns the current state of the bubble.
virtual IbanBubbleType GetBubbleType() const = 0;
};

} // namespace autofill
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/page_action/page_action_icon_type.h"
#include "chrome/grit/generated_resources.h"
#include "components/autofill/core/browser/personal_data_manager.h"
#include "components/strings/grit/components_strings.h"
#include "ui/base/l10n/l10n_util.h"
Expand Down Expand Up @@ -49,8 +50,18 @@ void SaveIbanBubbleControllerImpl::OfferLocalSave(

if (should_show_prompt) {
ShowBubble();
} else {
ShowIconOnly();
}
// TODO(crbug.com/1349109): Add else block for show Icon only.
}

void SaveIbanBubbleControllerImpl::EnsureBubbleShown() {
// Don't show the bubble if it's already visible.
if (bubble_view()) {
return;
}

ShowBubble();
}

std::u16string SaveIbanBubbleControllerImpl::GetWindowTitle() const {
Expand Down Expand Up @@ -88,15 +99,14 @@ const IBAN& SaveIbanBubbleControllerImpl::GetIBAN() const {
return iban_;
}

AutofillBubbleBase* SaveIbanBubbleControllerImpl::GetSaveBubbleView() const {
return bubble_view();
}

void SaveIbanBubbleControllerImpl::OnSaveButton(
const std::u16string& nickname) {
switch (current_bubble_type_) {
case IbanBubbleType::kLocalSave:
DCHECK(!local_save_iban_prompt_callback_.is_null());
// Show an animated IBAN saved confirmation message next time
// UpdatePageActionIcon() is called.
should_show_iban_saved_label_animation_ = true;
std::move(local_save_iban_prompt_callback_)
.Run(AutofillClient::SaveIBANOfferUserDecision::kAccepted, nickname);
break;
Expand All @@ -117,7 +127,18 @@ void SaveIbanBubbleControllerImpl::OnBubbleClosed(
PaymentsBubbleClosedReason closed_reason) {
set_bubble_view(nullptr);

current_bubble_type_ = IbanBubbleType::kInactive;
// Handles `current_bubble_type_` change according to its current type and the
// `closed_reason`.
if (closed_reason == PaymentsBubbleClosedReason::kAccepted) {
if (current_bubble_type_ == IbanBubbleType::kLocalSave) {
// TODO(crbug.com/1349109): Add kManageIban type to open IBAN bubble for
// managing the saved IBAN.
} else {
current_bubble_type_ = IbanBubbleType::kInactive;
}
} else if (closed_reason == PaymentsBubbleClosedReason::kCancelled) {
current_bubble_type_ = IbanBubbleType::kInactive;
}

UpdatePageActionIcon();
}
Expand All @@ -131,6 +152,48 @@ SaveIbanBubbleControllerImpl::SaveIbanBubbleControllerImpl(
Profile::FromBrowserContext(web_contents->GetBrowserContext()))) {
}

IbanBubbleType SaveIbanBubbleControllerImpl::GetBubbleType() const {
return current_bubble_type_;
}

std::u16string SaveIbanBubbleControllerImpl::GetSavePaymentIconTooltipText()
const {
switch (current_bubble_type_) {
case IbanBubbleType::kLocalSave:
return l10n_util::GetStringUTF16(IDS_TOOLTIP_SAVE_IBAN);
case IbanBubbleType::kInactive:
return std::u16string();
}
}

bool SaveIbanBubbleControllerImpl::ShouldShowSavingPaymentAnimation() const {
return false;
}

bool SaveIbanBubbleControllerImpl::ShouldShowPaymentSavedLabelAnimation()
const {
return should_show_iban_saved_label_animation_;
}

bool SaveIbanBubbleControllerImpl::ShouldShowSaveFailureBadge() const {
return false;
}

void SaveIbanBubbleControllerImpl::OnAnimationEnded() {
// Do not repeat the animation next time UpdatePageActionIcon() is called,
// unless explicitly set somewhere else.
should_show_iban_saved_label_animation_ = false;
}

bool SaveIbanBubbleControllerImpl::IsIconVisible() const {
// If there is no bubble to show, then there should be no icon.
return current_bubble_type_ != IbanBubbleType::kInactive;
}

AutofillBubbleBase* SaveIbanBubbleControllerImpl::GetSaveBubbleView() const {
return bubble_view();
}

PageActionIconType SaveIbanBubbleControllerImpl::GetPageActionIconType() {
return PageActionIconType::kSaveIban;
}
Expand All @@ -156,6 +219,22 @@ void SaveIbanBubbleControllerImpl::ShowBubble() {
Show();
}

void SaveIbanBubbleControllerImpl::ShowIconOnly() {
DCHECK(current_bubble_type_ != IbanBubbleType::kInactive);
// Local save callback should not be null for LOCAL_SAVE state.
DCHECK(!local_save_iban_prompt_callback_.is_null() ||
current_bubble_type_ != IbanBubbleType::kLocalSave);
DCHECK(!bubble_view());

// Show the icon only. The bubble can still be displayed if the user
// explicitly clicks the icon.
UpdatePageActionIcon();

if (observer_for_testing_) {
observer_for_testing_->OnIconShown();
}
}

WEB_CONTENTS_USER_DATA_KEY_IMPL(SaveIbanBubbleControllerImpl);

} // namespace autofill
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "chrome/browser/ui/autofill/autofill_bubble_controller_base.h"
#include "chrome/browser/ui/autofill/payments/save_iban_bubble_controller.h"
#include "chrome/browser/ui/autofill/payments/save_iban_ui.h"
#include "chrome/browser/ui/autofill/payments/save_payment_icon_controller.h"
#include "components/autofill/core/browser/autofill_client.h"
#include "components/autofill/core/browser/data_model/iban.h"
#include "content/public/browser/web_contents_user_data.h"
Expand All @@ -22,13 +23,15 @@ enum class IbanBubbleType;
class SaveIbanBubbleControllerImpl
: public AutofillBubbleControllerBase,
public SaveIbanBubbleController,
public SavePaymentIconController,
public content::WebContentsUserData<SaveIbanBubbleControllerImpl> {
public:
// An observer class used by browsertests that gets notified whenever
// particular actions occur.
class ObserverForTest {
public:
virtual void OnBubbleShown() = 0;
virtual void OnIconShown() = 0;
};

SaveIbanBubbleControllerImpl(const SaveIbanBubbleControllerImpl&) = delete;
Expand All @@ -44,15 +47,27 @@ class SaveIbanBubbleControllerImpl
bool should_show_prompt,
AutofillClient::LocalSaveIBANPromptCallback save_iban_prompt_callback);

// No-op if the bubble is already shown, otherwise, shows the bubble.
void EnsureBubbleShown();

// SaveIbanBubbleController:
std::u16string GetWindowTitle() const override;
std::u16string GetAcceptButtonText() const override;
std::u16string GetDeclineButtonText() const override;
const IBAN& GetIBAN() const override;
AutofillBubbleBase* GetSaveBubbleView() const override;
void OnSaveButton(const std::u16string& nickname) override;
void OnCancelButton() override;
void OnBubbleClosed(PaymentsBubbleClosedReason closed_reason) override;
IbanBubbleType GetBubbleType() const override;

// SavePaymentIconController:
std::u16string GetSavePaymentIconTooltipText() const override;
bool ShouldShowSavingPaymentAnimation() const override;
bool ShouldShowPaymentSavedLabelAnimation() const override;
bool ShouldShowSaveFailureBadge() const override;
void OnAnimationEnded() override;
bool IsIconVisible() const override;
AutofillBubbleBase* GetSaveBubbleView() const override;

// For testing.
void SetEventObserverForTesting(ObserverForTest* observer) {
Expand All @@ -72,6 +87,9 @@ class SaveIbanBubbleControllerImpl
// Displays both the offer-to-save bubble and its associated omnibox icon.
void ShowBubble();

// Displays omnibox icon only.
void ShowIconOnly();

// Should outlive this object.
raw_ptr<PersonalDataManager> personal_data_manager_;

Expand All @@ -80,6 +98,9 @@ class SaveIbanBubbleControllerImpl

// Note: Below fields are set when IBAN save is offered.
//
// Is true only if the [IBAN saved] label animation should be shown.
bool should_show_iban_saved_label_animation_ = false;

// The type of bubble that is either currently being shown or would
// be shown when the IBAN save icon is clicked.
IbanBubbleType current_bubble_type_ = IbanBubbleType::kInactive;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ class SaveIbanBubbleControllerImplTest : public BrowserWithTestWindowTest {
void ClickSaveButton(const std::u16string& nickname) {
controller()->OnSaveButton(nickname);
controller()->OnBubbleClosed(PaymentsBubbleClosedReason::kAccepted);
if (controller()->ShouldShowPaymentSavedLabelAnimation()) {
controller()->OnAnimationEnded();
}
}

std::u16string saved_nickname() { return saved_nickname_; }
Expand All @@ -75,8 +78,7 @@ class SaveIbanBubbleControllerImplTest : public BrowserWithTestWindowTest {

TEST_F(SaveIbanBubbleControllerImplTest, LocalIbanSavedSuccessfully) {
std::u16string nickname = u"My doctor's IBAN";
IBAN iban = autofill::test::GetIBAN();
ShowLocalBubble(iban);
ShowLocalBubble(autofill::test::GetIBAN());
ClickSaveButton(nickname);

EXPECT_EQ(nickname, saved_nickname());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,24 @@
// found in the LICENSE file.

#include "chrome/browser/ui/autofill/payments/save_payment_icon_controller.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/ui/autofill/payments/save_card_bubble_controller_impl.h"
#include "chrome/browser/ui/autofill/payments/save_iban_bubble_controller_impl.h"

namespace autofill {

// static
SavePaymentIconController* SavePaymentIconController::GetOrCreate(
content::WebContents* web_contents) {
if (!web_contents)
return nullptr;

SaveCardBubbleControllerImpl::CreateForWebContents(web_contents);
return SaveCardBubbleControllerImpl::FromWebContents(web_contents);
}

// static
SavePaymentIconController* SavePaymentIconController::Get(
content::WebContents* web_contents) {
content::WebContents* web_contents,
int command_id) {
if (!web_contents)
return nullptr;

return SaveCardBubbleControllerImpl::FromWebContents(web_contents);
if (command_id == IDC_SAVE_CREDIT_CARD_FOR_PAGE) {
return SaveCardBubbleControllerImpl::FromWebContents(web_contents);
}
DCHECK_EQ(command_id, IDC_SAVE_IBAN_FOR_PAGE);
return SaveIbanBubbleControllerImpl::FromWebContents(web_contents);
}

} // namespace autofill
19 changes: 7 additions & 12 deletions chrome/browser/ui/autofill/payments/save_payment_icon_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,10 @@ class SavePaymentIconController {
virtual ~SavePaymentIconController() = default;

// Returns a reference to the SavePaymentIconController associated with the
// given |web_contents|. If controller does not exist, this will create the
// controller from the |web_contents| then return the reference.
static SavePaymentIconController* GetOrCreate(
content::WebContents* web_contents);

// Returns a reference to the SavePaymentIconController associated with the
// given |web_contents|. If controller does not exist, this will return
// nullptr.
static SavePaymentIconController* Get(content::WebContents* web_contents);
// given `web_contents` and `command_id`. If controller does not exist, this
// returns nullptr.
static SavePaymentIconController* Get(content::WebContents* web_contents,
int command_id);

// Once the animation ends, it shows a new bubble if needed.
virtual void OnAnimationEnded() = 0;
Expand All @@ -37,16 +32,16 @@ class SavePaymentIconController {
virtual bool ShouldShowSaveFailureBadge() const = 0;

// Returns true iff the payment saved animation should be shown.
virtual bool ShouldShowCardSavedLabelAnimation() const = 0;
virtual bool ShouldShowPaymentSavedLabelAnimation() const = 0;

// Returns true iff upload save is in progress and the saving animation should
// be shown.
virtual bool ShouldShowSavingCardAnimation() const = 0;
virtual bool ShouldShowSavingPaymentAnimation() const = 0;

// Returns true iff the payment icon is visible.
virtual bool IsIconVisible() const = 0;

// Returns the currently active save card bubble view. Can be nullptr if no
// Returns the currently active save payment bubble view. Can be nullptr if no
// bubble is visible.
virtual AutofillBubbleBase* GetSaveBubbleView() const = 0;

Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ui/browser_command_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,9 @@ bool BrowserCommandController::ExecuteCommandWithDisposition(
case IDC_SAVE_CREDIT_CARD_FOR_PAGE:
SaveCreditCard(browser_);
break;
case IDC_SAVE_IBAN_FOR_PAGE:
SaveIBAN(browser_);
break;
case IDC_MIGRATE_LOCAL_CREDIT_CARD_FOR_PAGE:
MigrateLocalCards(browser_);
break;
Expand Down

0 comments on commit 5c383b5

Please sign in to comment.