Commit 5f16f81a authored by Alice Wang's avatar Alice Wang Committed by Commit Bot

[Signin][Android] Replace boolean with enum states in web sign-in flow

This CL replaces the boolean flag IS_ACCOUNT_LIST_EXPANDED with enum
state class to represent different states of the account picker bottom
sheet. This refactoring will facilitate our future tasks of adding new
screens like |sign-in in progress| to the bottom sheet.

Bug: 1106442
Change-Id: I072693a393e14987c66888549956fdc49c386d36
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2311577Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Reviewed-by: default avatarTanmoy Mollik <triploblastic@chromium.org>
Commit-Queue: Alice Wang <aliceywang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795370}
parent b34ee645
...@@ -12,6 +12,7 @@ import androidx.annotation.Nullable; ...@@ -12,6 +12,7 @@ import androidx.annotation.Nullable;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.signin.ProfileDataCache; import org.chromium.chrome.browser.signin.ProfileDataCache;
import org.chromium.chrome.browser.signin.account_picker.AccountPickerBottomSheetProperties.AccountPickerBottomSheetState;
import org.chromium.components.signin.AccountManagerFacade; import org.chromium.components.signin.AccountManagerFacade;
import org.chromium.components.signin.AccountManagerFacadeProvider; import org.chromium.components.signin.AccountManagerFacadeProvider;
import org.chromium.components.signin.AccountUtils; import org.chromium.components.signin.AccountUtils;
...@@ -57,9 +58,10 @@ class AccountPickerBottomSheetMediator implements AccountPickerCoordinator.Liste ...@@ -57,9 +58,10 @@ class AccountPickerBottomSheetMediator implements AccountPickerCoordinator.Liste
*/ */
@Override @Override
public void onAccountSelected(String accountName, boolean isDefaultAccount) { public void onAccountSelected(String accountName, boolean isDefaultAccount) {
// Click on one account in the account list when the account list is expanded // Clicking on one account in the account list when the account list is expanded
// will collapse it to the selected account // will collapse it to the selected account
mModel.set(AccountPickerBottomSheetProperties.IS_ACCOUNT_LIST_EXPANDED, false); mModel.set(AccountPickerBottomSheetProperties.ACCOUNT_PICKER_BOTTOM_SHEET_STATE,
AccountPickerBottomSheetState.COLLAPSED_ACCOUNT_LIST);
setSelectedAccountName(accountName); setSelectedAccountName(accountName);
} }
...@@ -97,28 +99,31 @@ class AccountPickerBottomSheetMediator implements AccountPickerCoordinator.Liste ...@@ -97,28 +99,31 @@ class AccountPickerBottomSheetMediator implements AccountPickerCoordinator.Liste
private void onAccountListUpdated() { private void onAccountListUpdated() {
List<Account> accounts = mAccountManagerFacade.tryGetGoogleAccounts(); List<Account> accounts = mAccountManagerFacade.tryGetGoogleAccounts();
if (accounts.isEmpty()) { if (accounts.isEmpty()) {
// If all accounts disappeared when the account list is collapsed, we will // If all accounts disappeared, no matter if the account list is collapsed or expanded,
// go to the zero account screen. If the account list is expanded, we will // we will go to the zero account screen.
// first set the account list state to collapsed then move to the zero mModel.set(AccountPickerBottomSheetProperties.ACCOUNT_PICKER_BOTTOM_SHEET_STATE,
// account collapsed screen. AccountPickerBottomSheetState.NO_ACCOUNTS);
mModel.set(AccountPickerBottomSheetProperties.IS_ACCOUNT_LIST_EXPANDED, false);
mSelectedAccountName = null; mSelectedAccountName = null;
mModel.set(AccountPickerBottomSheetProperties.SELECTED_ACCOUNT_DATA, null); mModel.set(AccountPickerBottomSheetProperties.SELECTED_ACCOUNT_DATA, null);
} else if (!mModel.get(AccountPickerBottomSheetProperties.IS_ACCOUNT_LIST_EXPANDED) return;
&& !isSelectedAccountInAccountList(accounts)) {
// The selected account is _only_ updated when the account list is collapsed and
// the current selected account name is not in the new account list
// (or there is no selected account).
setSelectedAccountName(accounts.get(0).name);
} }
}
/** @AccountPickerBottomSheetState
* Returns true if there is selected account and it is in the account list. int state =
*/ mModel.get(AccountPickerBottomSheetProperties.ACCOUNT_PICKER_BOTTOM_SHEET_STATE);
private boolean isSelectedAccountInAccountList(List<Account> accounts) { if (state == AccountPickerBottomSheetState.NO_ACCOUNTS) {
return mSelectedAccountName != null // When a non-empty account list appears while it is currently zero-account screen,
&& AccountUtils.findAccountByName(accounts, mSelectedAccountName) != null; // we should change the screen to collapsed account list and set the selected account
// to the first account of the account list
mModel.set(AccountPickerBottomSheetProperties.ACCOUNT_PICKER_BOTTOM_SHEET_STATE,
AccountPickerBottomSheetState.COLLAPSED_ACCOUNT_LIST);
setSelectedAccountName(accounts.get(0).name);
} else if (state == AccountPickerBottomSheetState.COLLAPSED_ACCOUNT_LIST
&& AccountUtils.findAccountByName(accounts, mSelectedAccountName) == null) {
// When it is already collapsed account list, we update the selected account only
// when the current selected account name is no longer in the new account list
setSelectedAccountName(accounts.get(0).name);
}
} }
private void setSelectedAccountName(String accountName) { private void setSelectedAccountName(String accountName) {
...@@ -144,7 +149,8 @@ class AccountPickerBottomSheetMediator implements AccountPickerCoordinator.Liste ...@@ -144,7 +149,8 @@ class AccountPickerBottomSheetMediator implements AccountPickerCoordinator.Liste
private void onSelectedAccountClicked() { private void onSelectedAccountClicked() {
// Clicking on the selected account when the account list is collapsed will expand the // Clicking on the selected account when the account list is collapsed will expand the
// account list and make the account list visible // account list and make the account list visible
mModel.set(AccountPickerBottomSheetProperties.IS_ACCOUNT_LIST_EXPANDED, true); mModel.set(AccountPickerBottomSheetProperties.ACCOUNT_PICKER_BOTTOM_SHEET_STATE,
AccountPickerBottomSheetState.EXPANDED_ACCOUNT_LIST);
} }
/** /**
......
...@@ -4,17 +4,71 @@ ...@@ -4,17 +4,71 @@
package org.chromium.chrome.browser.signin.account_picker; package org.chromium.chrome.browser.signin.account_picker;
import androidx.annotation.IntDef;
import org.chromium.chrome.browser.signin.DisplayableProfileData; import org.chromium.chrome.browser.signin.DisplayableProfileData;
import org.chromium.ui.modelutil.PropertyKey; import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel; import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModel.ReadableObjectPropertyKey; import org.chromium.ui.modelutil.PropertyModel.ReadableObjectPropertyKey;
import org.chromium.ui.modelutil.PropertyModel.WritableBooleanPropertyKey; import org.chromium.ui.modelutil.PropertyModel.WritableIntPropertyKey;
import org.chromium.ui.modelutil.PropertyModel.WritableObjectPropertyKey; import org.chromium.ui.modelutil.PropertyModel.WritableObjectPropertyKey;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
/** /**
* Properties of account picker bottom sheet. * Properties of account picker bottom sheet.
*/ */
class AccountPickerBottomSheetProperties { class AccountPickerBottomSheetProperties {
/**
* States of account picker.
* Different account picker state correspond to different account picker bottom sheet
* configuration.
*/
@IntDef({AccountPickerBottomSheetState.NO_ACCOUNTS,
AccountPickerBottomSheetState.COLLAPSED_ACCOUNT_LIST,
AccountPickerBottomSheetState.EXPANDED_ACCOUNT_LIST,
AccountPickerBottomSheetState.SIGNIN_IN_PROGRESS})
@Retention(RetentionPolicy.SOURCE)
@interface AccountPickerBottomSheetState {
/**
* When there is no account on device, the user sees only one blue button
* |Add account to device|.
*
* The bottom sheet starts with this state when there is zero account on device.
*/
int NO_ACCOUNTS = 0;
/**
* When the account list is collapsed with exactly one account shown as the selected
* account and a blue |Continue as| button to sign in with the selected account.
*
* The bottom sheet starts with this state when there is at least one account on device.
*
* This state can also be reached from EXPANDED_ACCOUNT_LIST by clicking on one of
* the accounts in the expanded account list.
*/
int COLLAPSED_ACCOUNT_LIST = 1;
/**
* When the account list is expanded, the user sees the account list of all the accounts
* on device and some additional rows like |Add account to device| and |Go incognito mode|.
*
* This state is reached from COLLAPSED_ACCOUNT_LIST by clicking the selected account of
* the collapsed account list.
*/
int EXPANDED_ACCOUNT_LIST = 2;
/**
* When the user is in the sign-in process, no account or button will be visible, the user
* sees mainly a spinner in the bottom sheet.
*
* This state can only be reached from COLLAPSED_ACCOUNT_LIST, when the button
* |Continue as| is clicked. This state does not lead to any other state.
*/
int SIGNIN_IN_PROGRESS = 3;
}
// PropertyKeys for the selected account view when the account list is collapsed. // PropertyKeys for the selected account view when the account list is collapsed.
// The selected account view is replaced by account list view when the // The selected account view is replaced by account list view when the
// account list is expanded. // account list is expanded.
...@@ -28,20 +82,26 @@ class AccountPickerBottomSheetProperties { ...@@ -28,20 +82,26 @@ class AccountPickerBottomSheetProperties {
static final ReadableObjectPropertyKey<Runnable> ON_CONTINUE_AS_CLICKED = static final ReadableObjectPropertyKey<Runnable> ON_CONTINUE_AS_CLICKED =
new ReadableObjectPropertyKey<>("on_continue_as_clicked"); new ReadableObjectPropertyKey<>("on_continue_as_clicked");
// PropertyKey indicates if the account list is expanded // PropertyKey indicates the state of the account picker bottom sheet
static final WritableBooleanPropertyKey IS_ACCOUNT_LIST_EXPANDED = static final WritableIntPropertyKey ACCOUNT_PICKER_BOTTOM_SHEET_STATE =
new WritableBooleanPropertyKey("is_account_list_expanded"); new WritableIntPropertyKey("account_picker_bottom_sheet_state");
static final PropertyKey[] ALL_KEYS = new PropertyKey[] {ON_SELECTED_ACCOUNT_CLICKED, static final PropertyKey[] ALL_KEYS = new PropertyKey[] {ON_SELECTED_ACCOUNT_CLICKED,
SELECTED_ACCOUNT_DATA, ON_CONTINUE_AS_CLICKED, IS_ACCOUNT_LIST_EXPANDED}; SELECTED_ACCOUNT_DATA, ON_CONTINUE_AS_CLICKED, ACCOUNT_PICKER_BOTTOM_SHEET_STATE};
/**
* Creates a default model for the AccountPickerBottomSheet.
*
* In the default model, as the selected account data is null, the bottom sheet is in the
* state {@link AccountPickerBottomSheetState#NO_ACCOUNTS}.
*/
static PropertyModel createModel( static PropertyModel createModel(
Runnable onSelectedAccountClicked, Runnable onContinueAsClicked) { Runnable onSelectedAccountClicked, Runnable onContinueAsClicked) {
return new PropertyModel.Builder(ALL_KEYS) return new PropertyModel.Builder(ALL_KEYS)
.with(ON_SELECTED_ACCOUNT_CLICKED, onSelectedAccountClicked) .with(ON_SELECTED_ACCOUNT_CLICKED, onSelectedAccountClicked)
.with(SELECTED_ACCOUNT_DATA, null) .with(SELECTED_ACCOUNT_DATA, null)
.with(ON_CONTINUE_AS_CLICKED, onContinueAsClicked) .with(ON_CONTINUE_AS_CLICKED, onContinueAsClicked)
.with(IS_ACCOUNT_LIST_EXPANDED, false) .with(ACCOUNT_PICKER_BOTTOM_SHEET_STATE, AccountPickerBottomSheetState.NO_ACCOUNTS)
.build(); .build();
} }
......
...@@ -74,35 +74,39 @@ class AccountPickerBottomSheetView implements BottomSheetContent { ...@@ -74,35 +74,39 @@ class AccountPickerBottomSheetView implements BottomSheetContent {
} }
/** /**
* Collapses the account list. * Collapses the account list to the selected account.
* If there is a non null selected account, the account list will collapse to that account,
* otherwise, the account list will just collapse the remaining.
*
* @param isSelectedAccountNonNull Flag indicates if the selected profile data exists
* in model.
*/ */
void collapseAccountList(boolean isSelectedAccountNonNull) { void collapseAccountList() {
mAccountListView.setVisibility(View.GONE); mAccountListView.setVisibility(View.GONE);
mSelectedAccountView.setVisibility(isSelectedAccountNonNull ? View.VISIBLE : View.GONE); mSelectedAccountView.setVisibility(View.VISIBLE);
mContinueAsButton.setVisibility(View.VISIBLE); mContinueAsButton.setVisibility(View.VISIBLE);
} }
/** /**
* Updates the view of the collapsed account list. * Collapses the account list to the no account view.
*/ */
void updateCollapsedAccountList(DisplayableProfileData accountProfileData) { void collapseToNoAccountView() {
if (accountProfileData == null) { mAccountListView.setVisibility(View.GONE);
mContinueAsButton.setText(R.string.signin_add_account_to_device); mSelectedAccountView.setVisibility(View.GONE);
} else { mContinueAsButton.setVisibility(View.VISIBLE);
ExistingAccountRowViewBinder.bindAccountView(accountProfileData, mSelectedAccountView); mContinueAsButton.setText(R.string.signin_add_account_to_device);
}
ImageView rowEndImage = mSelectedAccountView.findViewById(R.id.account_selection_mark);
rowEndImage.setImageResource(R.drawable.ic_expand_more_in_circle_24dp); /**
* Updates the views related to the selected account.
String continueAsButtonText = mContext.getString(R.string.signin_promo_continue_as, *
accountProfileData.getGivenNameOrFullNameOrEmail()); * This method only updates the UI elements like text related to the selected account, it
mContinueAsButton.setText(continueAsButtonText); * does not change the visibility.
} */
void updateSelectedAccount(DisplayableProfileData accountProfileData) {
ExistingAccountRowViewBinder.bindAccountView(accountProfileData, mSelectedAccountView);
ImageView rowEndImage = mSelectedAccountView.findViewById(R.id.account_selection_mark);
rowEndImage.setImageResource(R.drawable.ic_expand_more_in_circle_24dp);
String continueAsButtonText = mContext.getString(R.string.signin_promo_continue_as,
accountProfileData.getGivenNameOrFullNameOrEmail());
mContinueAsButton.setText(continueAsButtonText);
} }
@Override @Override
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
package org.chromium.chrome.browser.signin.account_picker; package org.chromium.chrome.browser.signin.account_picker;
import org.chromium.chrome.browser.signin.DisplayableProfileData; import org.chromium.chrome.browser.signin.DisplayableProfileData;
import org.chromium.chrome.browser.signin.account_picker.AccountPickerBottomSheetProperties.AccountPickerBottomSheetState;
import org.chromium.ui.modelutil.PropertyKey; import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel; import org.chromium.ui.modelutil.PropertyModel;
...@@ -18,22 +19,17 @@ class AccountPickerBottomSheetViewBinder { ...@@ -18,22 +19,17 @@ class AccountPickerBottomSheetViewBinder {
view.getSelectedAccountView().setOnClickListener(v -> { view.getSelectedAccountView().setOnClickListener(v -> {
model.get(AccountPickerBottomSheetProperties.ON_SELECTED_ACCOUNT_CLICKED).run(); model.get(AccountPickerBottomSheetProperties.ON_SELECTED_ACCOUNT_CLICKED).run();
}); });
} else if (propertyKey == AccountPickerBottomSheetProperties.IS_ACCOUNT_LIST_EXPANDED) { } else if (propertyKey
if (model.get(AccountPickerBottomSheetProperties.IS_ACCOUNT_LIST_EXPANDED)) { == AccountPickerBottomSheetProperties.ACCOUNT_PICKER_BOTTOM_SHEET_STATE) {
view.expandAccountList(); @AccountPickerBottomSheetState
} else { int state =
boolean isSelectedAccountNonNull = model.get(AccountPickerBottomSheetProperties.ACCOUNT_PICKER_BOTTOM_SHEET_STATE);
model.get(AccountPickerBottomSheetProperties.SELECTED_ACCOUNT_DATA) != null; switchToState(view, state);
view.collapseAccountList(isSelectedAccountNonNull);
}
} else if (propertyKey == AccountPickerBottomSheetProperties.SELECTED_ACCOUNT_DATA) { } else if (propertyKey == AccountPickerBottomSheetProperties.SELECTED_ACCOUNT_DATA) {
if (!model.get(AccountPickerBottomSheetProperties.IS_ACCOUNT_LIST_EXPANDED)) { DisplayableProfileData profileData =
// Selected account data (which can be null) is only updated model.get(AccountPickerBottomSheetProperties.SELECTED_ACCOUNT_DATA);
// when the account list is collapsed. if (profileData != null) {
DisplayableProfileData profileData = view.updateSelectedAccount(profileData);
model.get(AccountPickerBottomSheetProperties.SELECTED_ACCOUNT_DATA);
view.collapseAccountList(profileData != null);
view.updateCollapsedAccountList(profileData);
} }
} else if (propertyKey == AccountPickerBottomSheetProperties.ON_CONTINUE_AS_CLICKED) { } else if (propertyKey == AccountPickerBottomSheetProperties.ON_CONTINUE_AS_CLICKED) {
view.getContinueAsButton().setOnClickListener(v -> { view.getContinueAsButton().setOnClickListener(v -> {
...@@ -42,5 +38,30 @@ class AccountPickerBottomSheetViewBinder { ...@@ -42,5 +38,30 @@ class AccountPickerBottomSheetViewBinder {
} }
} }
/**
* Sets up the configuration of account picker bottom sheet according to the given state.
*/
private static void switchToState(AccountPickerBottomSheetView view,
@AccountPickerBottomSheetState int accountPickerBottomSheetState) {
switch (accountPickerBottomSheetState) {
case AccountPickerBottomSheetState.NO_ACCOUNTS:
view.collapseToNoAccountView();
break;
case AccountPickerBottomSheetState.COLLAPSED_ACCOUNT_LIST:
view.collapseAccountList();
break;
case AccountPickerBottomSheetState.EXPANDED_ACCOUNT_LIST:
view.expandAccountList();
break;
case AccountPickerBottomSheetState.SIGNIN_IN_PROGRESS:
// TODO(https://crbug.com/1102784): Implement UI update when sign-in in progress
break;
default:
throw new IllegalArgumentException(
"Cannot bind AccountPickerBottomSheetView for the state:"
+ accountPickerBottomSheetState);
}
}
private AccountPickerBottomSheetViewBinder() {} private AccountPickerBottomSheetViewBinder() {}
} }
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