Merge "Add illustration to final screen in PS setup and update existing illustrations." into main
diff --git a/res/values/dimens.xml b/res/values/dimens.xml
index 3e0b8d9..4b96486 100755
--- a/res/values/dimens.xml
+++ b/res/values/dimens.xml
@@ -473,4 +473,8 @@
 
     <!-- An arbitrarily large number to make the max size fit the parent -->
     <dimen name="animation_max_size">1000dp</dimen>
+
+    <!-- Credential Manager settings dimensions -->
+    <dimen name="credman_primary_provider_pref_left_padding">80dp</dimen>
+    <dimen name="credman_primary_provider_pref_left_padding_compact">24dp</dimen>
 </resources>
diff --git a/res/values/strings.xml b/res/values/strings.xml
index 17569e0..836a27f 100644
--- a/res/values/strings.xml
+++ b/res/values/strings.xml
@@ -10802,6 +10802,8 @@
     <string name="credman_button_change">Change</string>
     <!-- Button for opening credman service settings. [CHAR LIMIT=40] -->
     <string name="credman_button_open">Open</string>
+    <!-- Label for None item in Credential Manager service selection [CHAR LIMIT=40] -->
+    <string name="credman_app_list_preference_none">None selected</string>
 
     <!-- Message of the warning dialog for setting the auto-fill app. [CHAR_LIMIT=NONE] -->
     <string name="autofill_confirmation_message">
diff --git a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java
index d98bc51..be0658e 100644
--- a/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java
+++ b/src/com/android/settings/applications/credentials/CredentialManagerPreferenceController.java
@@ -568,7 +568,7 @@
      */
     @VisibleForTesting
     public boolean togglePackageNameEnabled(String packageName) {
-        if (mEnabledPackageNames.size() >= MAX_SELECTABLE_PROVIDERS) {
+        if (hasProviderLimitBeenReached()) {
             return false;
         } else {
             mEnabledPackageNames.add(packageName);
@@ -623,6 +623,19 @@
         return mIconResizer.createIconThumbnail(providerIcon);
     }
 
+    private boolean hasProviderLimitBeenReached() {
+        return hasProviderLimitBeenReached(mEnabledPackageNames.size());
+    }
+
+    @VisibleForTesting
+    public static boolean hasProviderLimitBeenReached(int enabledAdditionalProviderCount) {
+        // If the number of package names has reached the maximum limit then
+        // we should stop any new packages from being added. We will also
+        // reserve one place for the primary provider so if the max limit is
+        // five providers this will be four additional plus the primary.
+        return (enabledAdditionalProviderCount + 1) >= MAX_SELECTABLE_PROVIDERS;
+    }
+
     private CombiPreference addProviderPreference(
             @NonNull Context prefContext,
             @NonNull CharSequence title,
@@ -648,19 +661,18 @@
         pref.setPreferenceListener(
                 new CombiPreference.OnCombiPreferenceClickListener() {
                     @Override
-                    public void onCheckChanged(CombiPreference p, boolean isChecked) {
+                    public boolean onCheckChanged(CombiPreference p, boolean isChecked) {
                         if (isChecked) {
-                            if (mEnabledPackageNames.size() >= MAX_SELECTABLE_PROVIDERS) {
+                            if (hasProviderLimitBeenReached()) {
                                 // Show the error if too many enabled.
-                                pref.setChecked(false);
                                 final DialogFragment fragment = newErrorDialogFragment();
 
                                 if (fragment == null || mFragmentManager == null) {
-                                    return;
+                                    return false;
                                 }
 
                                 fragment.show(mFragmentManager, ErrorDialogFragment.TAG);
-                                return;
+                                return false;
                             }
 
                             togglePackageNameEnabled(packageName);
@@ -672,6 +684,8 @@
                         } else {
                             togglePackageNameDisabled(packageName);
                         }
+
+                        return true;
                     }
 
                     @Override
@@ -989,8 +1003,13 @@
             @Override
             public void onClick(View buttonView) {
                 // Forward the event.
-                if (mSwitch != null) {
-                    mOnClickListener.onCheckChanged(CombiPreference.this, mSwitch.isChecked());
+                if (mSwitch != null && mOnClickListener != null) {
+                    if (!mOnClickListener.onCheckChanged(CombiPreference.this, mSwitch.isChecked())) {
+                      // The update was not successful since there were too
+                      // many enabled providers to manually reset any state.
+                      mChecked = false;
+                      mSwitch.setChecked(false);
+                    }
                 }
             }
         }
@@ -1004,7 +1023,7 @@
 
         public interface OnCombiPreferenceClickListener {
             /** Called when the check is updated */
-            void onCheckChanged(CombiPreference p, boolean isChecked);
+            boolean onCheckChanged(CombiPreference p, boolean isChecked);
 
             /** Called when the left side is clicked. */
             void onLeftSideClicked();
diff --git a/src/com/android/settings/applications/credentials/DefaultCombinedPreferenceController.java b/src/com/android/settings/applications/credentials/DefaultCombinedPreferenceController.java
index 49dd7cd..032402f 100644
--- a/src/com/android/settings/applications/credentials/DefaultCombinedPreferenceController.java
+++ b/src/com/android/settings/applications/credentials/DefaultCombinedPreferenceController.java
@@ -114,7 +114,7 @@
             @Nullable CharSequence packageName,
             @Nullable CharSequence settingsActivity) {
         if (appName == null) {
-            preference.setTitle(R.string.app_list_preference_none);
+            preference.setTitle(R.string.credman_app_list_preference_none);
         } else {
             preference.setTitle(appName);
         }
@@ -144,7 +144,7 @@
 
             // Hide the open button if there is no defined settings activity.
             primaryPref.setOpenButtonVisible(!TextUtils.isEmpty(settingsActivity));
-            primaryPref.setButtonsVisible(appName != null);
+            primaryPref.setButtonsCompactMode(appName != null);
         }
     }
 
diff --git a/src/com/android/settings/applications/credentials/PrimaryProviderPreference.java b/src/com/android/settings/applications/credentials/PrimaryProviderPreference.java
index b8e2529..84459e0 100644
--- a/src/com/android/settings/applications/credentials/PrimaryProviderPreference.java
+++ b/src/com/android/settings/applications/credentials/PrimaryProviderPreference.java
@@ -46,7 +46,7 @@
     private @Nullable View mButtonFrameView = null;
     private @Nullable View mGearView = null;
     private @Nullable Delegate mDelegate = null;
-    private boolean mButtonsVisible = false;
+    private boolean mButtonsCompactMode = false;
     private boolean mOpenButtonVisible = false;
 
     /** Called to send messages back to the parent controller. */
@@ -141,26 +141,7 @@
                 });
 
         mButtonFrameView = holder.findViewById(R.id.credman_button_frame);
-        mButtonFrameView.setVisibility(mButtonsVisible ? View.VISIBLE : View.GONE);
-
-        // There is a special case where if the provider == none then we should
-        // hide the buttons and when the preference is tapped we can open the
-        // provider selection dialog.
-        setOnPreferenceClickListener(
-                new Preference.OnPreferenceClickListener() {
-                    public boolean onPreferenceClick(@NonNull Preference preference) {
-                        return handlePreferenceClickNewSettingsUi();
-                    }
-                });
-    }
-
-    private boolean handlePreferenceClickNewSettingsUi() {
-        if (mDelegate != null && !mButtonsVisible) {
-            mDelegate.onChangeButtonClicked();
-            return true;
-        }
-
-        return false;
+        updateButtonFramePadding();
     }
 
     public void setOpenButtonVisible(boolean isVisible) {
@@ -172,12 +153,27 @@
         mOpenButtonVisible = isVisible;
     }
 
-    public void setButtonsVisible(boolean isVisible) {
-        if (mButtonFrameView != null) {
-            setVisibility(mButtonFrameView, isVisible);
+    private void updateButtonFramePadding() {
+        if (mButtonFrameView == null) {
+          return;
         }
 
-        mButtonsVisible = isVisible;
+        int paddingLeft = mButtonsCompactMode ?
+            getContext().getResources().getDimensionPixelSize(
+                R.dimen.credman_primary_provider_pref_left_padding) :
+            getContext().getResources().getDimensionPixelSize(
+                R.dimen.credman_primary_provider_pref_left_padding_compact);
+
+        mButtonFrameView.setPadding(
+            paddingLeft,
+            mButtonFrameView.getPaddingTop(),
+            mButtonFrameView.getPaddingRight(),
+            mButtonFrameView.getPaddingBottom());
+    }
+
+    public void setButtonsCompactMode(boolean isCompactMode) {
+        mButtonsCompactMode = isCompactMode;
+        updateButtonFramePadding();
     }
 
     public void setDelegate(@NonNull Delegate delegate) {
diff --git a/tests/unit/src/com/android/settings/applications/credentials/CredentialManagerPreferenceControllerTest.java b/tests/unit/src/com/android/settings/applications/credentials/CredentialManagerPreferenceControllerTest.java
index b5aeac7..0a04a73 100644
--- a/tests/unit/src/com/android/settings/applications/credentials/CredentialManagerPreferenceControllerTest.java
+++ b/tests/unit/src/com/android/settings/applications/credentials/CredentialManagerPreferenceControllerTest.java
@@ -217,33 +217,33 @@
         assertThat(controller.isHiddenDueToNoProviderSet()).isFalse();
         assertThat(controller.getAvailabilityStatus()).isEqualTo(AVAILABLE);
 
-        // Ensure that we stay under 5 providers.
+        // Ensure that we stay under 5 providers (one is reserved for primary).
         assertThat(controller.togglePackageNameEnabled("com.android.provider1")).isTrue();
         assertThat(controller.togglePackageNameEnabled("com.android.provider2")).isTrue();
         assertThat(controller.togglePackageNameEnabled("com.android.provider3")).isTrue();
         assertThat(controller.togglePackageNameEnabled("com.android.provider4")).isTrue();
-        assertThat(controller.togglePackageNameEnabled("com.android.provider5")).isTrue();
+        assertThat(controller.togglePackageNameEnabled("com.android.provider5")).isFalse();
         assertThat(controller.togglePackageNameEnabled("com.android.provider6")).isFalse();
 
         // Check that they are all actually registered.
         Set<String> enabledProviders = controller.getEnabledProviders();
-        assertThat(enabledProviders.size()).isEqualTo(5);
+        assertThat(enabledProviders.size()).isEqualTo(4);
         assertThat(enabledProviders.contains("com.android.provider1")).isTrue();
         assertThat(enabledProviders.contains("com.android.provider2")).isTrue();
         assertThat(enabledProviders.contains("com.android.provider3")).isTrue();
         assertThat(enabledProviders.contains("com.android.provider4")).isTrue();
-        assertThat(enabledProviders.contains("com.android.provider5")).isTrue();
+        assertThat(enabledProviders.contains("com.android.provider5")).isFalse();
         assertThat(enabledProviders.contains("com.android.provider6")).isFalse();
 
         // Check that the settings string has the right component names.
         List<String> enabledServices = controller.getEnabledSettings();
-        assertThat(enabledServices.size()).isEqualTo(6);
+        assertThat(enabledServices.size()).isEqualTo(5);
         assertThat(enabledServices.contains("com.android.provider1/ClassA")).isTrue();
         assertThat(enabledServices.contains("com.android.provider1/ClassB")).isTrue();
         assertThat(enabledServices.contains("com.android.provider2/ClassA")).isTrue();
         assertThat(enabledServices.contains("com.android.provider3/ClassA")).isTrue();
         assertThat(enabledServices.contains("com.android.provider4/ClassA")).isTrue();
-        assertThat(enabledServices.contains("com.android.provider5/ClassA")).isTrue();
+        assertThat(enabledServices.contains("com.android.provider5/ClassA")).isFalse();
         assertThat(enabledServices.contains("com.android.provider6/ClassA")).isFalse();
 
         // Toggle the provider disabled.
@@ -251,22 +251,22 @@
 
         // Check that the provider was removed from the list of providers.
         Set<String> currentlyEnabledProviders = controller.getEnabledProviders();
-        assertThat(currentlyEnabledProviders.size()).isEqualTo(4);
+        assertThat(currentlyEnabledProviders.size()).isEqualTo(3);
         assertThat(currentlyEnabledProviders.contains("com.android.provider1")).isTrue();
         assertThat(currentlyEnabledProviders.contains("com.android.provider2")).isFalse();
         assertThat(currentlyEnabledProviders.contains("com.android.provider3")).isTrue();
         assertThat(currentlyEnabledProviders.contains("com.android.provider4")).isTrue();
-        assertThat(currentlyEnabledProviders.contains("com.android.provider5")).isTrue();
+        assertThat(currentlyEnabledProviders.contains("com.android.provider5")).isFalse();
         assertThat(currentlyEnabledProviders.contains("com.android.provider6")).isFalse();
 
         // Check that the provider was removed from the list of services stored in the setting.
         List<String> currentlyEnabledServices = controller.getEnabledSettings();
-        assertThat(currentlyEnabledServices.size()).isEqualTo(5);
+        assertThat(currentlyEnabledServices.size()).isEqualTo(4);
         assertThat(currentlyEnabledServices.contains("com.android.provider1/ClassA")).isTrue();
         assertThat(currentlyEnabledServices.contains("com.android.provider1/ClassB")).isTrue();
         assertThat(currentlyEnabledServices.contains("com.android.provider3/ClassA")).isTrue();
         assertThat(currentlyEnabledServices.contains("com.android.provider4/ClassA")).isTrue();
-        assertThat(currentlyEnabledServices.contains("com.android.provider5/ClassA")).isTrue();
+        assertThat(currentlyEnabledServices.contains("com.android.provider5/ClassA")).isFalse();
         assertThat(currentlyEnabledServices.contains("com.android.provider6/ClassA")).isFalse();
     }
 
@@ -528,6 +528,17 @@
         assertThat(thumbnail.getIntrinsicWidth()).isEqualTo(getIconSize());
     }
 
+    @Test
+    public void testProviderLimitReached() {
+        // The limit is 5 with one slot reserved for primary.
+        assertThat(CredentialManagerPreferenceController.hasProviderLimitBeenReached(0)).isFalse();
+        assertThat(CredentialManagerPreferenceController.hasProviderLimitBeenReached(1)).isFalse();
+        assertThat(CredentialManagerPreferenceController.hasProviderLimitBeenReached(2)).isFalse();
+        assertThat(CredentialManagerPreferenceController.hasProviderLimitBeenReached(3)).isFalse();
+        assertThat(CredentialManagerPreferenceController.hasProviderLimitBeenReached(4)).isTrue();
+        assertThat(CredentialManagerPreferenceController.hasProviderLimitBeenReached(5)).isTrue();
+    }
+
     private int getIconSize() {
         final Resources resources = mContext.getResources();
         return (int) resources.getDimension(android.R.dimen.app_icon_size);
diff --git a/tests/unit/src/com/android/settings/applications/credentials/PrimaryProviderPreferenceTest.java b/tests/unit/src/com/android/settings/applications/credentials/PrimaryProviderPreferenceTest.java
index 51a1fc4..9ea16bb 100644
--- a/tests/unit/src/com/android/settings/applications/credentials/PrimaryProviderPreferenceTest.java
+++ b/tests/unit/src/com/android/settings/applications/credentials/PrimaryProviderPreferenceTest.java
@@ -114,50 +114,21 @@
     }
 
     @Test
-    public void ensureButtonsClicksCallDelegate_newDesign_buttonsHidden() {
+    public void ensureButtonsClicksCallDelegate_newDesign_buttonsCompactMode() {
         if (!PrimaryProviderPreference.shouldUseNewSettingsUi()) {
             return;
         }
 
         PrimaryProviderPreference ppp = createTestPreferenceWithNewLayout();
+        int initialPaddingLeft = ppp.getButtonFrameView().getPaddingLeft();
 
-        // Test that the buttons are visible.
-        assertThat(ppp.getButtonFrameView()).isNotNull();
-        assertThat(ppp.getButtonFrameView().getVisibility()).isEqualTo(View.GONE);
-        assertThat(mReceivedChangeButtonClicked).isFalse();
+        // If we show the buttons the left padding should be updated.
+        ppp.setButtonsCompactMode(true);
+        assertThat(ppp.getButtonFrameView().getPaddingLeft()).isNotEqualTo(initialPaddingLeft);
 
-        // If we show the buttons the visiblility should be updated.
-        ppp.setButtonsVisible(true);
-        assertThat(ppp.getButtonFrameView().getVisibility()).isEqualTo(View.VISIBLE);
-
-        // If we hide the buttons the visibility should be updated.
-        ppp.setButtonsVisible(false);
-        assertThat(ppp.getButtonFrameView().getVisibility()).isEqualTo(View.GONE);
-    }
-
-    @Test
-    public void ensureButtonsClicksCallDelegate_oldDesign() {
-        if (PrimaryProviderPreference.shouldUseNewSettingsUi()) {
-            return;
-        }
-
-        PrimaryProviderPreference ppp = createTestPreference("preference_widget_gear");
-
-        // Test that clicking the preference results in the delegate being
-        // called.
-        assertThat(mReceivedOpenButtonClicked).isFalse();
-        ppp.getOnPreferenceClickListener().onPreferenceClick(ppp);
-        assertThat(mReceivedOpenButtonClicked).isTrue();
-
-        // Test that the gear button is present and visible.
-        assertThat(ppp.getGearView()).isNotNull();
-        assertThat(ppp.getGearView().getVisibility()).isEqualTo(View.VISIBLE);
-
-        // Test that clicking the gear button results in the delegate being
-        // called.
-        assertThat(mReceivedChangeButtonClicked).isFalse();
-        ppp.getGearView().performClick();
-        assertThat(mReceivedChangeButtonClicked).isTrue();
+        // If we hide the buttons the left padding should be updated.
+        ppp.setButtonsCompactMode(false);
+        assertThat(ppp.getButtonFrameView().getPaddingLeft()).isEqualTo(initialPaddingLeft);
     }
 
     private PrimaryProviderPreference createTestPreferenceWithNewLayout() {