Commit 39eded94 authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

[Passwords] Update header image for different states in Save Bubble

As per the attached bug, here is the logic for changing the header image

- The illustration should change when the user switches between "Save to device" and "Save to account".
- In the Update bubble, the illustration should change based on where the update will go.
- If the update will go to both locations, use the "account" image.

Screencast:
Light mode: https://screencast.googleplex.com/cast/NjQxNjYzNjU3MzY0Njg0OHw0NTg2MTgzYy1kMw
Dark mode: https://screencast.googleplex.com/cast/NTY4MTQ3NTYxMTUyNTEyMHwzOTI4Y2JkYy1mZQ

Bug: 1115936
Change-Id: Iaa3256fa1ff6dde3019da6a6da96940e577697b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2421674
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809389}
parent fe861948
...@@ -266,8 +266,10 @@ ...@@ -266,8 +266,10 @@
<structure type="chrome_scaled_image" name="IDR_SAVED_PASSWORDS_SAFE_STATE_DARK" file="common/passwords_safe_state_dark.png" /> <structure type="chrome_scaled_image" name="IDR_SAVED_PASSWORDS_SAFE_STATE_DARK" file="common/passwords_safe_state_dark.png" />
<structure type="chrome_scaled_image" name="IDR_SAVED_PASSWORDS_WARNING_STATE" file="common/passwords_warning_state.png" /> <structure type="chrome_scaled_image" name="IDR_SAVED_PASSWORDS_WARNING_STATE" file="common/passwords_warning_state.png" />
<structure type="chrome_scaled_image" name="IDR_SAVED_PASSWORDS_WARNING_STATE_DARK" file="common/passwords_warning_state_dark.png" /> <structure type="chrome_scaled_image" name="IDR_SAVED_PASSWORDS_WARNING_STATE_DARK" file="common/passwords_warning_state_dark.png" />
<structure type="chrome_scaled_image" name="IDR_SAVE_PASSWORD" file="common/save_password.png" /> <structure type="chrome_scaled_image" name="IDR_SAVE_PASSWORD_MULTI_DEVICE" file="common/save_password_multi_device.png" />
<structure type="chrome_scaled_image" name="IDR_SAVE_PASSWORD_DARK" file="common/save_password_dark.png" /> <structure type="chrome_scaled_image" name="IDR_SAVE_PASSWORD_MULTI_DEVICE_DARK" file="common/save_password_multi_device_dark.png" />
<structure type="chrome_scaled_image" name="IDR_SAVE_PASSWORD_ONE_DEVICE" file="common/save_password_one_device.png" />
<structure type="chrome_scaled_image" name="IDR_SAVE_PASSWORD_ONE_DEVICE_DARK" file="common/save_password_one_device_dark.png" />
</if> </if>
<structure type="chrome_scaled_image" name="IDR_SCREEN_CAPTURE_NOTIFICATION_GRIP" file="screen_capture_notification_grip.png" /> <structure type="chrome_scaled_image" name="IDR_SCREEN_CAPTURE_NOTIFICATION_GRIP" file="screen_capture_notification_grip.png" />
<if expr="chromeos"> <if expr="chromeos">
......
...@@ -198,6 +198,26 @@ bool SaveUpdateWithAccountStoreBubbleController::IsCurrentStateUpdate() const { ...@@ -198,6 +198,26 @@ bool SaveUpdateWithAccountStoreBubbleController::IsCurrentStateUpdate() const {
}); });
} }
bool SaveUpdateWithAccountStoreBubbleController::
IsCurrentStateAffectingTheAccountStore() {
DCHECK(state_ == password_manager::ui::PENDING_PASSWORD_UPDATE_STATE ||
state_ == password_manager::ui::PENDING_PASSWORD_STATE);
bool is_update = false;
bool is_update_in_account_store = false;
for (const autofill::PasswordForm& form : existing_credentials_) {
if (form.username_value == pending_password_.username_value) {
is_update = true;
if (form.IsUsingAccountStore())
is_update_in_account_store = true;
}
}
if (!is_update)
return IsUsingAccountStore();
return is_update_in_account_store;
}
bool SaveUpdateWithAccountStoreBubbleController::RevealPasswords() { bool SaveUpdateWithAccountStoreBubbleController::RevealPasswords() {
bool reveal_immediately = !password_revealing_requires_reauth_ || bool reveal_immediately = !password_revealing_requires_reauth_ ||
(delegate_ && delegate_->AuthenticateUser()); (delegate_ && delegate_->AuthenticateUser());
......
...@@ -51,6 +51,12 @@ class SaveUpdateWithAccountStoreBubbleController ...@@ -51,6 +51,12 @@ class SaveUpdateWithAccountStoreBubbleController
// creation. This method returns true iff the current state is "update". // creation. This method returns true iff the current state is "update".
bool IsCurrentStateUpdate() const; bool IsCurrentStateUpdate() const;
// The password bubble header image can switch its state between "save" and
// "update" depending on the user input. |state_| only captures the correct
// state on creation. This method returns true iff the current state is
// "save" or "update" to a password in the account store.
bool IsCurrentStateAffectingTheAccountStore();
// Returns true if passwords revealing is not locked or re-authentication is // Returns true if passwords revealing is not locked or re-authentication is
// not available on the given platform. Otherwise, the method schedules // not available on the given platform. Otherwise, the method schedules
// re-authentication and bubble reopen (the current bubble will be destroyed), // re-authentication and bubble reopen (the current bubble will be destroyed),
......
...@@ -688,3 +688,73 @@ TEST_F(SaveUpdateWithAccountStoreBubbleControllerTest, DisableEditing) { ...@@ -688,3 +688,73 @@ TEST_F(SaveUpdateWithAccountStoreBubbleControllerTest, DisableEditing) {
PretendPasswordWaiting(); PretendPasswordWaiting();
EXPECT_FALSE(controller()->enable_editing()); EXPECT_FALSE(controller()->enable_editing());
} }
TEST_F(SaveUpdateWithAccountStoreBubbleControllerTest,
UpdateAccountStoreAffectsTheAccountStore) {
EXPECT_CALL(*delegate(), GetPendingPassword())
.WillOnce(ReturnRef(pending_password()));
std::vector<std::unique_ptr<autofill::PasswordForm>> forms;
auto form = std::make_unique<autofill::PasswordForm>(pending_password());
form->password_value = base::ASCIIToUTF16("old_password");
form->in_store = autofill::PasswordForm::Store::kAccountStore;
forms.push_back(std::move(form));
EXPECT_CALL(*delegate(), GetCurrentForms()).WillOnce(ReturnRef(forms));
SetUpWithState(password_manager::ui::PENDING_PASSWORD_UPDATE_STATE,
PasswordBubbleControllerBase::DisplayReason::kAutomatic);
EXPECT_TRUE(controller()->IsCurrentStateAffectingTheAccountStore());
}
TEST_F(SaveUpdateWithAccountStoreBubbleControllerTest,
UpdateProfileStoreDoesnotAffectTheAccountStore) {
EXPECT_CALL(*delegate(), GetPendingPassword())
.WillOnce(ReturnRef(pending_password()));
std::vector<std::unique_ptr<autofill::PasswordForm>> forms;
auto form = std::make_unique<autofill::PasswordForm>(pending_password());
form->password_value = base::ASCIIToUTF16("old_password");
form->in_store = autofill::PasswordForm::Store::kProfileStore;
forms.push_back(std::move(form));
EXPECT_CALL(*delegate(), GetCurrentForms()).WillOnce(ReturnRef(forms));
SetUpWithState(password_manager::ui::PENDING_PASSWORD_UPDATE_STATE,
PasswordBubbleControllerBase::DisplayReason::kAutomatic);
EXPECT_FALSE(controller()->IsCurrentStateAffectingTheAccountStore());
}
TEST_F(SaveUpdateWithAccountStoreBubbleControllerTest,
UpdateBothStoresAffectsTheAccountStore) {
EXPECT_CALL(*delegate(), GetPendingPassword())
.WillOnce(ReturnRef(pending_password()));
std::vector<std::unique_ptr<autofill::PasswordForm>> forms;
auto profile_form =
std::make_unique<autofill::PasswordForm>(pending_password());
profile_form->password_value = base::ASCIIToUTF16("old_password");
profile_form->in_store = autofill::PasswordForm::Store::kProfileStore;
forms.push_back(std::move(profile_form));
auto account_form =
std::make_unique<autofill::PasswordForm>(pending_password());
account_form->password_value = base::ASCIIToUTF16("old_password");
account_form->in_store = autofill::PasswordForm::Store::kAccountStore;
forms.push_back(std::move(account_form));
EXPECT_CALL(*delegate(), GetCurrentForms()).WillOnce(ReturnRef(forms));
SetUpWithState(password_manager::ui::PENDING_PASSWORD_UPDATE_STATE,
PasswordBubbleControllerBase::DisplayReason::kAutomatic);
EXPECT_TRUE(controller()->IsCurrentStateAffectingTheAccountStore());
}
TEST_F(SaveUpdateWithAccountStoreBubbleControllerTest,
SaveInAccountStoreAffectsTheAccountStore) {
ON_CALL(*password_feature_manager(), GetDefaultPasswordStore)
.WillByDefault(Return(autofill::PasswordForm::Store::kAccountStore));
PretendPasswordWaiting();
EXPECT_TRUE(controller()->IsCurrentStateAffectingTheAccountStore());
}
TEST_F(SaveUpdateWithAccountStoreBubbleControllerTest,
SaveInProfileStoreDoesntAffectTheAccountStore) {
ON_CALL(*password_feature_manager(), GetDefaultPasswordStore)
.WillByDefault(Return(autofill::PasswordForm::Store::kProfileStore));
PretendPasswordWaiting();
EXPECT_FALSE(controller()->IsCurrentStateAffectingTheAccountStore());
}
...@@ -273,8 +273,8 @@ void MoveToAccountStoreBubbleView::OnThemeChanged() { ...@@ -273,8 +273,8 @@ void MoveToAccountStoreBubbleView::OnThemeChanged() {
PasswordBubbleViewBase::OnThemeChanged(); PasswordBubbleViewBase::OnThemeChanged();
GetBubbleFrameView()->SetHeaderView(CreateHeaderImage( GetBubbleFrameView()->SetHeaderView(CreateHeaderImage(
color_utils::IsDark(GetBubbleFrameView()->GetBackgroundColor()) color_utils::IsDark(GetBubbleFrameView()->GetBackgroundColor())
? IDR_SAVE_PASSWORD_DARK ? IDR_SAVE_PASSWORD_MULTI_DEVICE_DARK
: IDR_SAVE_PASSWORD)); : IDR_SAVE_PASSWORD_MULTI_DEVICE));
} }
gfx::Size MoveToAccountStoreBubbleView::CalculatePreferredSize() const { gfx::Size MoveToAccountStoreBubbleView::CalculatePreferredSize() const {
......
...@@ -388,8 +388,8 @@ void PasswordSaveUpdateView::AddedToWidget() { ...@@ -388,8 +388,8 @@ void PasswordSaveUpdateView::AddedToWidget() {
void PasswordSaveUpdateView::OnThemeChanged() { void PasswordSaveUpdateView::OnThemeChanged() {
PasswordBubbleViewBase::OnThemeChanged(); PasswordBubbleViewBase::OnThemeChanged();
int id = color_utils::IsDark(GetBubbleFrameView()->GetBackgroundColor()) int id = color_utils::IsDark(GetBubbleFrameView()->GetBackgroundColor())
? IDR_SAVE_PASSWORD_DARK ? IDR_SAVE_PASSWORD_MULTI_DEVICE_DARK
: IDR_SAVE_PASSWORD; : IDR_SAVE_PASSWORD_MULTI_DEVICE;
GetBubbleFrameView()->SetHeaderView(CreateHeaderImage(id)); GetBubbleFrameView()->SetHeaderView(CreateHeaderImage(id));
if (password_view_button_) { if (password_view_button_) {
auto* theme = GetNativeTheme(); auto* theme = GetNativeTheme();
......
...@@ -522,6 +522,8 @@ void PasswordSaveUpdateWithAccountStoreView::DestinationChanged() { ...@@ -522,6 +522,8 @@ void PasswordSaveUpdateWithAccountStoreView::DestinationChanged() {
bool is_account_store_selected = bool is_account_store_selected =
destination_dropdown_->GetSelectedIndex() == 0; destination_dropdown_->GetSelectedIndex() == 0;
controller_.OnToggleAccountStore(is_account_store_selected); controller_.OnToggleAccountStore(is_account_store_selected);
// Saving in account and local stores have different header images.
UpdateHeaderImage();
// If the user explicitly switched to "save on this device only", record this // If the user explicitly switched to "save on this device only", record this
// with the IPH tracker (so it can decide not to show the IPH again). // with the IPH tracker (so it can decide not to show the IPH again).
if (!is_account_store_selected) { if (!is_account_store_selected) {
...@@ -602,10 +604,7 @@ void PasswordSaveUpdateWithAccountStoreView::AddedToWidget() { ...@@ -602,10 +604,7 @@ void PasswordSaveUpdateWithAccountStoreView::AddedToWidget() {
void PasswordSaveUpdateWithAccountStoreView::OnThemeChanged() { void PasswordSaveUpdateWithAccountStoreView::OnThemeChanged() {
PasswordBubbleViewBase::OnThemeChanged(); PasswordBubbleViewBase::OnThemeChanged();
int id = color_utils::IsDark(GetBubbleFrameView()->GetBackgroundColor()) UpdateHeaderImage();
? IDR_SAVE_PASSWORD_DARK
: IDR_SAVE_PASSWORD;
GetBubbleFrameView()->SetHeaderView(CreateHeaderImage(id));
if (password_view_button_) { if (password_view_button_) {
auto* theme = GetNativeTheme(); auto* theme = GetNativeTheme();
const SkColor icon_color = const SkColor icon_color =
...@@ -683,6 +682,9 @@ void PasswordSaveUpdateWithAccountStoreView::UpdateBubbleUIElements() { ...@@ -683,6 +682,9 @@ void PasswordSaveUpdateWithAccountStoreView::UpdateBubbleUIElements() {
if (!destination_dropdown_) if (!destination_dropdown_)
return; return;
// Saving and updating are using different headers depending on the affected
// store.
UpdateHeaderImage();
// If it's not a save bubble anymore, close the IPH because the account picker // If it's not a save bubble anymore, close the IPH because the account picker
// will disappear. If it has become a save bubble, the IPH will get triggered // will disappear. If it has become a save bubble, the IPH will get triggered
// after the animation finishes. // after the animation finishes.
...@@ -692,6 +694,17 @@ void PasswordSaveUpdateWithAccountStoreView::UpdateBubbleUIElements() { ...@@ -692,6 +694,17 @@ void PasswordSaveUpdateWithAccountStoreView::UpdateBubbleUIElements() {
destination_dropdown_->SetVisible(!controller_.IsCurrentStateUpdate()); destination_dropdown_->SetVisible(!controller_.IsCurrentStateUpdate());
} }
void PasswordSaveUpdateWithAccountStoreView::UpdateHeaderImage() {
bool is_dark_mode =
color_utils::IsDark(GetBubbleFrameView()->GetBackgroundColor());
int id = controller_.IsCurrentStateAffectingTheAccountStore()
? (is_dark_mode ? IDR_SAVE_PASSWORD_MULTI_DEVICE_DARK
: IDR_SAVE_PASSWORD_MULTI_DEVICE)
: (is_dark_mode ? IDR_SAVE_PASSWORD_ONE_DEVICE_DARK
: IDR_SAVE_PASSWORD_ONE_DEVICE);
GetBubbleFrameView()->SetHeaderView(CreateHeaderImage(id));
}
bool PasswordSaveUpdateWithAccountStoreView::ShouldShowRegularIPH() { bool PasswordSaveUpdateWithAccountStoreView::ShouldShowRegularIPH() {
// IPH is shown only where the destination dropdown is shown (i.e. only for // IPH is shown only where the destination dropdown is shown (i.e. only for
// Save bubble). // Save bubble).
......
...@@ -84,6 +84,7 @@ class PasswordSaveUpdateWithAccountStoreView ...@@ -84,6 +84,7 @@ class PasswordSaveUpdateWithAccountStoreView
void TogglePasswordVisibility(); void TogglePasswordVisibility();
void UpdateUsernameAndPasswordInModel(); void UpdateUsernameAndPasswordInModel();
void UpdateBubbleUIElements(); void UpdateBubbleUIElements();
void UpdateHeaderImage();
void DestinationChanged(); void DestinationChanged();
......
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