From ab07b53ea140b6386702145f65775166b2d369d6 Mon Sep 17 00:00:00 2001
From: ykhung <ykhung@google.com>
Date: Tue, 18 Apr 2023 18:05:08 +0800
Subject: [PATCH] Fix banner tip buttons visibility and update incompatible tip

Fix the battery settings banner tip buttons visibility not reset issue
and update the incompatible charging tip into the colored primary button

Bug: 246960554
Test: make RunSettingsRoboTests ROBOTEST_FILTER=com.android.settings.fuelgauge.*
Test: make RunSettingsRoboTests ROBOTEST_FILTER=com.android.settings.widget.CardPreferenceTest
Change-Id: I9b72fccc7838d974d3b60d0f9684780841623cbb
---
 res/layout/card_preference.xml                | 11 ++--
 .../fuelgauge/batterytip/tips/BatteryTip.java |  4 ++
 .../tips/IncompatibleChargerTip.java          |  8 +--
 .../settings/widget/CardPreference.java       | 19 +++++--
 .../batterytip/tips/BatteryTipTest.java       | 18 +++++++
 .../tips/IncompatibleChargerTipTest.java      |  4 +-
 .../settings/widget/CardPreferenceTest.java   | 53 +++++++++++--------
 7 files changed, 81 insertions(+), 36 deletions(-)

diff --git a/res/layout/card_preference.xml b/res/layout/card_preference.xml
index e02933560d2..83c6eaf6a7c 100644
--- a/res/layout/card_preference.xml
+++ b/res/layout/card_preference.xml
@@ -67,12 +67,15 @@
             android:maxLines="10"
             style="@style/PreferenceSummaryTextStyle"/>
 
-        <RelativeLayout
+        <LinearLayout
             android:id="@+id/card_preference_buttons"
             android:layout_width="match_parent"
             android:layout_height="wrap_content"
             android:layout_marginTop="10dp"
             android:layout_below="@android:id/summary"
+            android:gravity="center_vertical|end"
+            android:orientation="horizontal"
+            android:paddingEnd="20dp"
             android:visibility="gone">
             <Button
                 android:id="@android:id/button1"
@@ -80,17 +83,15 @@
                 android:layout_height="wrap_content"
                 android:layout_width="wrap_content"
                 android:layout_marginStart="20dp"
-                android:layout_toStartOf="@android:id/button2"
                 android:visibility="gone"/>
             <Button
                 android:id="@android:id/button2"
                 style="@style/CardPreferenceBorderlessButton"
                 android:layout_height="wrap_content"
                 android:layout_width="wrap_content"
-                android:layout_marginHorizontal="20dp"
-                android:layout_alignParentEnd="true"
+                android:layout_marginStart="20dp"
                 android:visibility="gone"/>
-        </RelativeLayout>
+        </LinearLayout>
     </RelativeLayout>
 
     <!-- Preference should place its actual preference widget here. -->
diff --git a/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTip.java b/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTip.java
index c869b9265f4..8aabc37c6dc 100644
--- a/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTip.java
+++ b/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTip.java
@@ -166,6 +166,10 @@ public abstract class BatteryTip implements Comparable<BatteryTip>, Parcelable {
         if (iconTintColorId != View.NO_ID) {
             preference.getIcon().setTint(context.getColor(iconTintColorId));
         }
+        final CardPreference cardPreference = castToCardPreferenceSafely(preference);
+        if (cardPreference != null) {
+            cardPreference.resetLayoutState();
+        }
     }
 
     /** Returns the color resid for tinting {@link #getIconId()} or {@link View#NO_ID} if none. */
diff --git a/src/com/android/settings/fuelgauge/batterytip/tips/IncompatibleChargerTip.java b/src/com/android/settings/fuelgauge/batterytip/tips/IncompatibleChargerTip.java
index 8a8cc5cff9f..c1616eadb12 100644
--- a/src/com/android/settings/fuelgauge/batterytip/tips/IncompatibleChargerTip.java
+++ b/src/com/android/settings/fuelgauge/batterytip/tips/IncompatibleChargerTip.java
@@ -73,15 +73,15 @@ public final class IncompatibleChargerTip extends BatteryTip {
         }
 
         cardPreference.setSelectable(false);
-        cardPreference.setSecondaryButtonText(context.getString(R.string.learn_more));
-        cardPreference.setSecondaryButtonClickListener(
+        cardPreference.setPrimaryButtonText(context.getString(R.string.learn_more));
+        cardPreference.setPrimaryButtonClickListener(
                 button -> button.startActivityForResult(
                         HelpUtils.getHelpIntent(
                                 context,
                                 context.getString(R.string.help_url_incompatible_charging),
                                 /* backupContext */ ""), /* requestCode */ 0));
-        cardPreference.setSecondaryButtonVisible(true);
-        cardPreference.setSecondaryButtonContentDescription(context.getString(
+        cardPreference.setPrimaryButtonVisible(true);
+        cardPreference.setPrimaryButtonContentDescription(context.getString(
                 R.string.battery_tip_incompatible_charging_content_description));
     }
 }
diff --git a/src/com/android/settings/widget/CardPreference.java b/src/com/android/settings/widget/CardPreference.java
index 862a943f5b2..61114d959f2 100644
--- a/src/com/android/settings/widget/CardPreference.java
+++ b/src/com/android/settings/widget/CardPreference.java
@@ -30,9 +30,7 @@ import com.google.android.material.card.MaterialCardView;
 
 import java.util.Optional;
 
-/**
- * Preference that wrapped by {@link MaterialCardView}, only support to set icon, title and summary
- */
+/** Preference that wrapped by {@link MaterialCardView} */
 public class CardPreference extends Preference {
 
     private View.OnClickListener mPrimaryBtnClickListener = null;
@@ -76,6 +74,12 @@ public class CardPreference extends Preference {
         setSecondaryButtonVisible(mSecondaryButtonVisible);
     }
 
+    /** Clear layout state if needed */
+    public void resetLayoutState() {
+        setPrimaryButtonVisible(false);
+        setSecondaryButtonVisible(false);
+    }
+
     /**
      * Register a callback to be invoked when the primary button is clicked.
      *
@@ -140,6 +144,15 @@ public class CardPreference extends Preference {
         updateButtonGroupsVisibility();
     }
 
+    /**
+     * Sets the text of content description on primary button.
+     *
+     * @param text text for the content description
+     */
+    public void setPrimaryButtonContentDescription(String text) {
+        mPrimaryButton.ifPresent(button -> button.setContentDescription(text));
+    }
+
     /**
      * Sets the text of content description on secondary button.
      *
diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTipTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTipTest.java
index 87fc0dddb77..35131688b02 100644
--- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTipTest.java
+++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/BatteryTipTest.java
@@ -18,13 +18,16 @@ package com.android.settings.fuelgauge.batterytip.tips;
 import static com.google.common.truth.Truth.assertThat;
 
 import android.content.Context;
+import android.view.View;
 import android.os.Parcel;
 import android.os.Parcelable;
 
 import androidx.annotation.IdRes;
 import androidx.preference.Preference;
+import androidx.preference.PreferenceViewHolder;
 
 import com.android.settings.R;
+import com.android.settings.widget.CardPreference;
 import com.android.settingslib.core.instrumentation.MetricsFeatureProvider;
 import com.android.settingslib.testutils.DrawableTestHelper;
 
@@ -80,6 +83,21 @@ public class BatteryTipTest {
         assertThat(parcelTip.needUpdate()).isTrue();
     }
 
+    @Test
+    public void updatePreference_resetLayoutState() {
+        mContext.setTheme(R.style.Theme_Settings);
+        PreferenceViewHolder holder = PreferenceViewHolder.createInstanceForTests(
+                View.inflate(mContext, R.layout.card_preference_layout, /* parent= */ null));
+        CardPreference cardPreference = new CardPreference(mContext);
+        cardPreference.onBindViewHolder(holder);
+        cardPreference.setPrimaryButtonVisible(true);
+
+        mBatteryTip.updatePreference(cardPreference);
+
+        View view = holder.findViewById(R.id.card_preference_buttons);
+        assertThat(view.getVisibility()).isEqualTo(View.GONE);
+    }
+
     @Test
     public void tipOrder_orderUnique() {
         final List<Integer> orders = new ArrayList<>();
diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/IncompatibleChargerTipTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/IncompatibleChargerTipTest.java
index 3aad06132a2..9f74e1b32a4 100644
--- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/IncompatibleChargerTipTest.java
+++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/IncompatibleChargerTipTest.java
@@ -109,13 +109,13 @@ public final class IncompatibleChargerTipTest {
 
         mIncompatibleChargerTip.updatePreference(mCardPreference);
 
-        verify(mCardPreference).setSecondaryButtonText(expected);
+        verify(mCardPreference).setPrimaryButtonText(expected);
     }
 
     @Test
     public void updatePreference_shouldSetSecondaryButtonVisible() {
         mIncompatibleChargerTip.updatePreference(mCardPreference);
-        verify(mCardPreference).setSecondaryButtonVisible(true);
+        verify(mCardPreference).setPrimaryButtonVisible(true);
     }
 
     private String getLastErrorLog() {
diff --git a/tests/robotests/src/com/android/settings/widget/CardPreferenceTest.java b/tests/robotests/src/com/android/settings/widget/CardPreferenceTest.java
index eba447b83f8..e26643fdfc4 100644
--- a/tests/robotests/src/com/android/settings/widget/CardPreferenceTest.java
+++ b/tests/robotests/src/com/android/settings/widget/CardPreferenceTest.java
@@ -6,7 +6,7 @@
  * You may obtain a copy of the License at
  *
  *      http://www.apache.org/licenses/LICENSE-2.0
- *
+ *Visibility_setGoneForPrimaryButton_buttonGroupIsGone
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -48,8 +48,8 @@ public class CardPreferenceTest {
         context.setTheme(R.style.Theme_Settings);
         mCardPreference = new CardPreference(context);
 
-        View rootView = View.inflate(context, R.layout.card_preference_layout, /* parent= */ null);
-        mHolder = PreferenceViewHolder.createInstanceForTests(rootView);
+        mHolder = PreferenceViewHolder.createInstanceForTests(
+                View.inflate(context, R.layout.card_preference_layout, /* parent= */ null));
     }
 
     @Test
@@ -63,14 +63,14 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void onBindViewHolder_noButtonVisible_buttonsLayoutShouldBeGone() {
+    public void onBindViewHolder_noButtonVisible_buttonsLayoutIsGone() {
         mCardPreference.onBindViewHolder(mHolder);
 
         assertThat(getCardPreferenceButtonsView().getVisibility()).isEqualTo(GONE);
     }
 
     @Test
-    public void onBindViewHolder_setPrimaryButtonVisibility_buttonsLayoutShouldBeVisible() {
+    public void onBindViewHolder_setPrimaryButtonVisibility_buttonsLayoutIsVisible() {
         mCardPreference.setPrimaryButtonVisible(true);
 
         mCardPreference.onBindViewHolder(mHolder);
@@ -79,7 +79,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void onBindViewHolder_setPrimaryButtonVisibility_shouldApplyToPrimaryButton() {
+    public void onBindViewHolder_setPrimaryButtonVisibilityToVisible() {
         mCardPreference.setPrimaryButtonVisible(true);
 
         mCardPreference.onBindViewHolder(mHolder);
@@ -88,7 +88,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void onBindViewHolder_setSecondaryButtonVisibility_buttonsLayoutShouldBeVisible() {
+    public void onBindViewHolder_setSecondaryButtonVisibility_buttonsLayoutIsVisible() {
         mCardPreference.setSecondaryButtonVisible(true);
 
         mCardPreference.onBindViewHolder(mHolder);
@@ -97,7 +97,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void onBindViewHolder_setSecondaryButtonVisibility_shouldApplyToSecondaryButton() {
+    public void onBindViewHolder_setSecondaryButtonVisibilityToVisible() {
         mCardPreference.setSecondaryButtonVisible(true);
 
         mCardPreference.onBindViewHolder(mHolder);
@@ -106,7 +106,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void onBindViewHolder_setPrimaryButtonText_shouldApplyToPrimaryButton() {
+    public void onBindViewHolder_setPrimaryButtonTextToExpectedText() {
         String expectedText = "primary-button";
         mCardPreference.setPrimaryButtonText(expectedText);
 
@@ -116,7 +116,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void onBindViewHolder_setSecondaryButtonText_shouldApplyToSecondaryButton() {
+    public void onBindViewHolder_setSecondaryButtonTextToExpectedText() {
         String expectedText = "secondary-button";
         mCardPreference.setSecondaryButtonText(expectedText);
 
@@ -140,7 +140,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void performClickOnPrimaryButton_shouldCalledClickListener() {
+    public void performClickOnPrimaryButton_callClickListener() {
         final boolean[] hasCalled = {false};
         View.OnClickListener clickListener = v -> hasCalled[0] = true;
         mCardPreference.setPrimaryButtonClickListener(clickListener);
@@ -152,7 +152,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void performClickOnSecondaryButton_shouldCalledClickListener() {
+    public void performClickOnSecondaryButton_callClickListener() {
         final boolean[] hasCalled = {false};
         View.OnClickListener clickListener = v -> hasCalled[0] = true;
         mCardPreference.setSecondaryButtonClickListener(clickListener);
@@ -178,7 +178,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void setPrimaryButtonVisibility_setTrueAfterBindViewHolder_shouldBeVisible() {
+    public void setPrimaryButtonVisibility_setTrueAfterBindViewHolder_isVisible() {
         mCardPreference.setPrimaryButtonVisible(false);
         mCardPreference.onBindViewHolder(mHolder);
 
@@ -198,7 +198,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void setPrimaryButtonText_setNull_shouldBeEmptyText() {
+    public void setPrimaryButtonText_setNull_isEmptyText() {
         final String emptyString = "";
         mCardPreference.setPrimaryButtonText("1234");
         mCardPreference.onBindViewHolder(mHolder);
@@ -222,7 +222,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void setPrimaryButtonClickListener_setNull_shouldClearTheOnClickListener() {
+    public void setPrimaryButtonClickListener_setNull_clearTheOnClickListener() {
         final String[] hasCalled = {"not called"};
         View.OnClickListener clickListener = v -> hasCalled[0] = "called once";
         mCardPreference.setPrimaryButtonClickListener(clickListener);
@@ -235,7 +235,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void setSecondaryButtonVisibility_setTrueAfterBindViewHolder_shouldBeVisible() {
+    public void setSecondaryButtonVisibility_setTrueAfterBindViewHolder_isVisible() {
         mCardPreference.setSecondaryButtonVisible(false);
         mCardPreference.onBindViewHolder(mHolder);
 
@@ -255,7 +255,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void setSecondaryButtonText_setNull_shouldBeEmptyText() {
+    public void setSecondaryButtonText_setNull_isEmptyText() {
         String emptyString = "";
         mCardPreference.setSecondaryButtonText("1234");
         mCardPreference.onBindViewHolder(mHolder);
@@ -279,7 +279,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void setSecondaryButtonClickListener_setNull_shouldClearTheOnClickListener() {
+    public void setSecondaryButtonClickListener_setNull_clearTheOnClickListener() {
         final String[] hasCalled = {"not called"};
         View.OnClickListener clickListener = v -> hasCalled[0] = "called once";
         mCardPreference.setSecondaryButtonClickListener(clickListener);
@@ -292,8 +292,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void
-            setPrimaryButtonVisibility_onlyPrimaryButtonVisible_setGone_buttonGroupShouldBeGone() {
+    public void setPrimaryButtonVisibility_setGoneForSecondaryButton_buttonGroupIsGone() {
         mCardPreference.setPrimaryButtonVisible(true);
         mCardPreference.setSecondaryButtonVisible(false);
         mCardPreference.onBindViewHolder(mHolder);
@@ -307,8 +306,7 @@ public class CardPreferenceTest {
     }
 
     @Test
-    public void
-            setSecondaryButtonVisibility_only2ndButtonVisible_setGone_buttonGroupShouldBeGone() {
+    public void setSecondaryButtonVisibility_setGoneForPrimaryButton_buttonGroupIsGone() {
         mCardPreference.setPrimaryButtonVisible(false);
         mCardPreference.setSecondaryButtonVisible(true);
         mCardPreference.onBindViewHolder(mHolder);
@@ -321,6 +319,17 @@ public class CardPreferenceTest {
         assertThat(getCardPreferenceButtonsView().getVisibility()).isEqualTo(GONE);
     }
 
+    @Test
+    public void resetLayoutState_buttonGroupIsGone() {
+        mCardPreference.setPrimaryButtonVisible(true);
+        mCardPreference.setSecondaryButtonVisible(true);
+        mCardPreference.onBindViewHolder(mHolder);
+
+        mCardPreference.resetLayoutState();
+
+        assertThat(getCardPreferenceButtonsView().getVisibility()).isEqualTo(GONE);
+    }
+
     private View getCardPreferenceButtonsView() {
         return mHolder.findViewById(R.id.card_preference_buttons);
     }