Create a new visual design for action buttons

- Create a new style for two action buttons. - Change the layout of two_action_buttons. Since we use same style for positive/nagative buttons now, we only remain button1/button2 for this layout. - Create a setButtonIcon interface in ActionButtonPreference. So, user can set icon for each button in this preferece. Test: visual, robotest Bug: 116346008 Change-Id: I511272cba5fd961349b85cae6d30004ddabe2c8e
2018-11-12 17:42:31 +08:00
parent d6cdafc0d7
commit 8f65efa8f9
6 changed files with 139 additions and 119 deletions
--- a/res/layout/two_action_buttons.xml
+++ b/res/layout/two_action_buttons.xml
@@ -19,47 +19,28 @@
    xmlns:android="http://schemas.android.com/apk/res/android"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
-    android:gravity="center"
-    android:paddingTop="24dp"
-    android:paddingStart="68dp"
-    android:paddingEnd="24dp"
+    android:paddingStart="8dp"
+    android:paddingEnd="8dp"
    android:orientation="horizontal">

-    <FrameLayout
+    <Button
+        android:id="@+id/button1"
+        style="@style/SettingsActionButton"
        android:layout_width="0dp"
+        android:layout_height="wrap_content"
        android:layout_weight="1"
-        android:layout_height="wrap_content">
-        <Button
-            android:id="@+id/button1_positive"
-            style="@style/ActionPrimaryButton"
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content"
-            android:layout_marginBottom="4dp" />
-        <Button
-            android:id="@+id/button1_negative"
-            style="@style/ActionSecondaryButton"
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content"
-            android:layout_marginBottom="4dp" />
-    </FrameLayout>
+        android:paddingTop="20dp"
+        android:paddingBottom="20dp"/>
    <Space
-        android:layout_width="16dp"
+        android:layout_width="8dp"
        android:layout_height="wrap_content" />
-    <FrameLayout
+
+    <Button
+        android:id="@+id/button2"
+        style="@style/SettingsActionButton"
        android:layout_width="0dp"
+        android:layout_height="wrap_content"
        android:layout_weight="1"
-        android:layout_height="wrap_content">
-        <Button
-            android:id="@+id/button2_positive"
-            style="@style/ActionPrimaryButton"
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content"
-            android:layout_marginBottom="4dp" />
-        <Button
-            android:id="@+id/button2_negative"
-            style="@style/ActionSecondaryButton"
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content"
-            android:layout_marginBottom="4dp" />
-    </FrameLayout>
+        android:paddingTop="20dp"
+        android:paddingBottom="20dp"/>
 </LinearLayout>
--- a/res/values/styles.xml
+++ b/res/values/styles.xml
@@ -426,6 +426,11 @@

    <style name="ActionSecondaryButton" parent="android:Widget.DeviceDefault.Button"/>

+    <style name="SettingsActionButton" parent="android:Widget.DeviceDefault.Button.Borderless.Colored">
+        <item name="android:drawablePadding">4dp</item>
+        <item name="android:drawableTint">@*android:color/btn_colored_borderless_text_material</item>
+    </style>
+
    <style name="LockPatternContainerStyle">
        <item name="android:maxHeight">400dp</item>
        <item name="android:maxWidth">420dp</item>
--- a/src/com/android/settings/widget/ActionButtonPreference.java
+++ b/src/com/android/settings/widget/ActionButtonPreference.java
@@ -17,11 +17,15 @@
 package com.android.settings.widget;

 import android.content.Context;
+import android.content.res.Resources;
+import android.graphics.drawable.Drawable;
 import android.text.TextUtils;
 import android.util.AttributeSet;
+import android.util.Log;
 import android.view.View;
 import android.widget.Button;

+import androidx.annotation.DrawableRes;
 import androidx.annotation.StringRes;
 import androidx.preference.Preference;
 import androidx.preference.PreferenceViewHolder;
@@ -30,6 +34,7 @@ import com.android.settings.R;

 public class ActionButtonPreference extends Preference {

+    private final String TAG = "ActionButtonPreference";
    private final ButtonInfo mButton1Info = new ButtonInfo();
    private final ButtonInfo mButton2Info = new ButtonInfo();

@@ -62,12 +67,11 @@ public class ActionButtonPreference extends Preference {
    @Override
    public void onBindViewHolder(PreferenceViewHolder holder) {
        super.onBindViewHolder(holder);
-        holder.setDividerAllowedAbove(false);
-        holder.setDividerAllowedBelow(false);
-        mButton1Info.mPositiveButton = (Button) holder.findViewById(R.id.button1_positive);
-        mButton1Info.mNegativeButton = (Button) holder.findViewById(R.id.button1_negative);
-        mButton2Info.mPositiveButton = (Button) holder.findViewById(R.id.button2_positive);
-        mButton2Info.mNegativeButton = (Button) holder.findViewById(R.id.button2_negative);
+        holder.setDividerAllowedAbove(true);
+        holder.setDividerAllowedBelow(true);
+
+        mButton1Info.mButton = (Button) holder.findViewById(R.id.button1);
+        mButton2Info.mButton = (Button) holder.findViewById(R.id.button2);

        mButton1Info.setUpButton();
        mButton2Info.setUpButton();
@@ -82,6 +86,22 @@ public class ActionButtonPreference extends Preference {
        return this;
    }

+    public ActionButtonPreference setButton1Icon(@DrawableRes int iconResId) {
+        if (iconResId == 0) {
+            return this;
+        }
+
+        final Drawable icon;
+        try {
+            icon = getContext().getDrawable(iconResId);
+            mButton1Info.mIcon = icon;
+            notifyChanged();
+        } catch (Resources.NotFoundException exception) {
+            Log.e(TAG, "Resource does not exist: " + iconResId);
+        }
+        return this;
+    }
+
    public ActionButtonPreference setButton1Enabled(boolean isEnabled) {
        if (isEnabled != mButton1Info.mIsEnabled) {
            mButton1Info.mIsEnabled = isEnabled;
@@ -99,6 +119,22 @@ public class ActionButtonPreference extends Preference {
        return this;
    }

+    public ActionButtonPreference setButton2Icon(@DrawableRes int iconResId) {
+        if (iconResId == 0) {
+            return this;
+        }
+
+        final Drawable icon;
+        try {
+            icon = getContext().getDrawable(iconResId);
+            mButton2Info.mIcon = icon;
+            notifyChanged();
+        } catch (Resources.NotFoundException exception) {
+            Log.e(TAG, "Resource does not exist: " + iconResId);
+        }
+        return this;
+    }
+
    public ActionButtonPreference setButton2Enabled(boolean isEnabled) {
        if (isEnabled != mButton2Info.mIsEnabled) {
            mButton2Info.mIsEnabled = isEnabled;
@@ -123,65 +159,51 @@ public class ActionButtonPreference extends Preference {
        return this;
    }

+    @Deprecated
    public ActionButtonPreference setButton1Positive(boolean isPositive) {
-        if (isPositive != mButton1Info.mIsPositive) {
-            mButton1Info.mIsPositive = isPositive;
-            notifyChanged();
-        }
        return this;
    }

+    @Deprecated
    public ActionButtonPreference setButton2Positive(boolean isPositive) {
-        if (isPositive != mButton2Info.mIsPositive) {
-            mButton2Info.mIsPositive = isPositive;
-            notifyChanged();
-        }
        return this;
    }
-    public ActionButtonPreference setButton1Visible(boolean isPositive) {
-        if (isPositive != mButton1Info.mIsVisible) {
-            mButton1Info.mIsVisible = isPositive;
+
+    public ActionButtonPreference setButton1Visible(boolean isVisible) {
+        if (isVisible != mButton1Info.mIsVisible) {
+            mButton1Info.mIsVisible = isVisible;
            notifyChanged();
        }
        return this;
    }

-    public ActionButtonPreference setButton2Visible(boolean isPositive) {
-        if (isPositive != mButton2Info.mIsVisible) {
-            mButton2Info.mIsVisible = isPositive;
+    public ActionButtonPreference setButton2Visible(boolean isVisible) {
+        if (isVisible != mButton2Info.mIsVisible) {
+            mButton2Info.mIsVisible = isVisible;
            notifyChanged();
        }
        return this;
    }

    static class ButtonInfo {
-        private Button mPositiveButton;
-        private Button mNegativeButton;
+        private Button mButton;
        private CharSequence mText;
+        private Drawable mIcon;
        private View.OnClickListener mListener;
-        private boolean mIsPositive = true;
        private boolean mIsEnabled = true;
        private boolean mIsVisible = true;

        void setUpButton() {
-            setUpButton(mPositiveButton);
-            setUpButton(mNegativeButton);
-            if (!mIsVisible) {
-                mPositiveButton.setVisibility(View.INVISIBLE);
-                mNegativeButton.setVisibility(View.INVISIBLE);
-            } else if (mIsPositive) {
-                mPositiveButton.setVisibility(View.VISIBLE);
-                mNegativeButton.setVisibility(View.INVISIBLE);
+            mButton.setText(mText);
+            mButton.setOnClickListener(mListener);
+            mButton.setEnabled(mIsEnabled);
+            mButton.setCompoundDrawablesWithIntrinsicBounds(
+                    null /* left */, mIcon /* top */, null /* right */, null /* bottom */);
+            if (mIsVisible) {
+                mButton.setVisibility(View.VISIBLE);
            } else {
-                mPositiveButton.setVisibility(View.INVISIBLE);
-                mNegativeButton.setVisibility(View.VISIBLE);
+                mButton.setVisibility(View.GONE);
            }
        }
-
-        private void setUpButton(Button button) {
-            button.setText(mText);
-            button.setOnClickListener(mListener);
-            button.setEnabled(mIsEnabled);
-        }
    }
 }
--- a/tests/robotests/src/com/android/settings/applications/LayoutPreferenceTest.java
+++ b/tests/robotests/src/com/android/settings/applications/LayoutPreferenceTest.java
@@ -75,12 +75,12 @@ public class LayoutPreferenceTest {

    @Test
    public void disableSomeView_shouldMaintainStateAfterBind() {
-        mPreference.findViewById(R.id.button1_positive).setEnabled(false);
-        mPreference.findViewById(R.id.button2_positive).setEnabled(true);
+        mPreference.findViewById(R.id.button1).setEnabled(false);
+        mPreference.findViewById(R.id.button2).setEnabled(true);

        mPreference.onBindViewHolder(mHolder);

-        assertThat(mPreference.findViewById(R.id.button1_positive).isEnabled()).isFalse();
-        assertThat(mPreference.findViewById(R.id.button2_positive).isEnabled()).isTrue();
+        assertThat(mPreference.findViewById(R.id.button1).isEnabled()).isFalse();
+        assertThat(mPreference.findViewById(R.id.button2).isEnabled()).isTrue();
    }
 }
--- a/tests/robotests/src/com/android/settings/bluetooth/BluetoothDetailsButtonsControllerTest.java
+++ b/tests/robotests/src/com/android/settings/bluetooth/BluetoothDetailsButtonsControllerTest.java
@@ -56,8 +56,8 @@ public class BluetoothDetailsButtonsControllerTest extends BluetoothDetailsContr
        super.setUp();
        final View buttons = View.inflate(
                RuntimeEnvironment.application, R.layout.two_action_buttons, null /* parent */);
-        mConnectButton = buttons.findViewById(R.id.button2_positive);
-        mForgetButton = buttons.findViewById(R.id.button1_positive);
+        mConnectButton = buttons.findViewById(R.id.button2);
+        mForgetButton = buttons.findViewById(R.id.button1);
        mController =
            new BluetoothDetailsButtonsController(mContext, mFragment, mCachedDevice, mLifecycle);
        mButtonsPref = ActionButtonPreferenceTest.createMock();
--- a/tests/robotests/src/com/android/settings/widget/ActionButtonPreferenceTest.java
+++ b/tests/robotests/src/com/android/settings/widget/ActionButtonPreferenceTest.java
@@ -25,6 +25,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;

 import android.content.Context;
+import android.graphics.drawable.Drawable;
 import android.view.View;
 import android.widget.Button;

@@ -59,41 +60,17 @@ public class ActionButtonPreferenceTest {
        mPref.setButton1Visible(false).setButton2Visible(false);
        mPref.onBindViewHolder(mHolder);

-        assertThat(mRootView.findViewById(R.id.button1_positive).getVisibility())
-                .isEqualTo(View.INVISIBLE);
-        assertThat(mRootView.findViewById(R.id.button1_negative).getVisibility())
-                .isEqualTo(View.INVISIBLE);
-
-        assertThat(mRootView.findViewById(R.id.button2_positive).getVisibility())
-                .isEqualTo(View.INVISIBLE);
-        assertThat(mRootView.findViewById(R.id.button2_negative).getVisibility())
-                .isEqualTo(View.INVISIBLE);
+        assertThat(mRootView.findViewById(R.id.button1).getVisibility())
+                .isEqualTo(View.GONE);
+        assertThat(mRootView.findViewById(R.id.button2).getVisibility())
+                .isEqualTo(View.GONE);

        mPref.setButton1Visible(true).setButton2Visible(true);
        mPref.onBindViewHolder(mHolder);

-        assertThat(mRootView.findViewById(R.id.button1_positive).getVisibility())
+        assertThat(mRootView.findViewById(R.id.button1).getVisibility())
                .isEqualTo(View.VISIBLE);
-        assertThat(mRootView.findViewById(R.id.button1_negative).getVisibility())
-                .isEqualTo(View.INVISIBLE);
-        assertThat(mRootView.findViewById(R.id.button2_positive).getVisibility())
-                .isEqualTo(View.VISIBLE);
-        assertThat(mRootView.findViewById(R.id.button2_negative).getVisibility())
-                .isEqualTo(View.INVISIBLE);
-    }
-
-    @Test
-    public void setPositiveNegative_shouldHideOppositeButton() {
-        mPref.setButton1Positive(true).setButton2Positive(false);
-        mPref.onBindViewHolder(mHolder);
-
-        assertThat(mRootView.findViewById(R.id.button1_positive).getVisibility())
-                .isEqualTo(View.VISIBLE);
-        assertThat(mRootView.findViewById(R.id.button1_negative).getVisibility())
-                .isEqualTo(View.INVISIBLE);
-        assertThat(mRootView.findViewById(R.id.button2_positive).getVisibility())
-                .isEqualTo(View.INVISIBLE);
-        assertThat(mRootView.findViewById(R.id.button2_negative).getVisibility())
+        assertThat(mRootView.findViewById(R.id.button2).getVisibility())
                .isEqualTo(View.VISIBLE);
    }

@@ -102,36 +79,71 @@ public class ActionButtonPreferenceTest {
        mPref.setButton1Enabled(true).setButton2Enabled(false);
        mPref.onBindViewHolder(mHolder);

-        assertThat(mRootView.findViewById(R.id.button1_positive).isEnabled()).isTrue();
-        assertThat(mRootView.findViewById(R.id.button1_negative).isEnabled()).isTrue();
-        assertThat(mRootView.findViewById(R.id.button2_positive).isEnabled()).isFalse();
-        assertThat(mRootView.findViewById(R.id.button2_negative).isEnabled()).isFalse();
+        assertThat(mRootView.findViewById(R.id.button1).isEnabled()).isTrue();
+        assertThat(mRootView.findViewById(R.id.button2).isEnabled()).isFalse();
    }

    @Test
-    public void setText() {
+    public void setText_shouldShowSameText() {
        mPref.setButton1Text(R.string.settings_label);
        mPref.onBindViewHolder(mHolder);

-        assertThat(((Button) mRootView.findViewById(R.id.button1_positive)).getText())
-                .isEqualTo(mContext.getText(R.string.settings_label));
-        assertThat(((Button) mRootView.findViewById(R.id.button1_negative)).getText())
+        assertThat(((Button) mRootView.findViewById(R.id.button1)).getText())
                .isEqualTo(mContext.getText(R.string.settings_label));
    }

+    @Test
+    public void setButtonIcon_iconMustDisplayAboveText() {
+        mPref.setButton1Text(R.string.settings_label);
+        mPref.setButton1Icon(R.drawable.ic_settings);
+        mPref.onBindViewHolder(mHolder);
+        final Drawable[] drawablesAroundText =
+                ((Button) mRootView.findViewById(R.id.button1))
+                        .getCompoundDrawables();
+
+        assertThat(drawablesAroundText[1 /* top */]).isNotNull();
+    }
+
+    @Test
+    public void setButtonIcon_iconResourceIdIsZero_shouldNotDisplayIcon() {
+        mPref.setButton1Text(R.string.settings_label);
+        mPref.setButton1Icon(0);
+        mPref.onBindViewHolder(mHolder);
+        final Drawable[] drawablesAroundText =
+                ((Button) mRootView.findViewById(R.id.button1))
+                        .getCompoundDrawables();
+
+        assertThat(drawablesAroundText[1 /* top */]).isNull();
+    }
+
+    @Test
+    public void setButtonIcon_iconResourceIdNotExisting_shouldNotDisplayIconAndCrash() {
+        mPref.setButton1Text(R.string.settings_label);
+        mPref.setButton1Icon(999999999 /* not existing id */);
+        // Should not crash here
+        mPref.onBindViewHolder(mHolder);
+        final Drawable[] drawablesAroundText =
+                ((Button) mRootView.findViewById(R.id.button1))
+                        .getCompoundDrawables();
+
+        assertThat(drawablesAroundText[1 /* top */]).isNull();
+    }
+
    public static ActionButtonPreference createMock() {
        final ActionButtonPreference pref = mock(ActionButtonPreference.class);
        when(pref.setButton1Text(anyInt())).thenReturn(pref);
+        when(pref.setButton1Icon(anyInt())).thenReturn(pref);
        when(pref.setButton1Positive(anyBoolean())).thenReturn(pref);
        when(pref.setButton1Enabled(anyBoolean())).thenReturn(pref);
        when(pref.setButton1Visible(anyBoolean())).thenReturn(pref);
        when(pref.setButton1OnClickListener(any(View.OnClickListener.class))).thenReturn(pref);

        when(pref.setButton2Text(anyInt())).thenReturn(pref);
+        when(pref.setButton2Icon(anyInt())).thenReturn(pref);
        when(pref.setButton2Positive(anyBoolean())).thenReturn(pref);
        when(pref.setButton2Enabled(anyBoolean())).thenReturn(pref);
        when(pref.setButton2Visible(anyBoolean())).thenReturn(pref);
        when(pref.setButton2OnClickListener(any(View.OnClickListener.class))).thenReturn(pref);
        return pref;
    }
-}
+}