OSDN Git Service

Fix Wi-Fi Easy Connect buttons UI defects
authorArc Wang <arcwang@google.com>
Fri, 15 Mar 2019 08:58:11 +0000 (16:58 +0800)
committerArc Wang <arcwang@google.com>
Tue, 19 Mar 2019 08:27:04 +0000 (16:27 +0800)
1. Apply attr/colorAccent to button icons for theme UI control
2. Apply attr/selectableItemBackground for button tapping ripple effect
3. Use ConstraintLayout to separate ssid EditText and scan button
4. Remove ButtonPreference and add AddNetworkPreference.
   ButtonPreference's naming and design look like a general purpose UI
   component but it's not. This change refactors the code.
5. In AddNetworkPreference, use settingslib layout file
   'preference_access_point' to fix UI alignment problems.

Bug: 126964552
Bug: 125434239
Bug: 126762937

Test: manual test
      atest com.android.settings.wifi.WifiSettingsTest
Change-Id: Ib899a1e10f96bb8427ff00d6b5dfca37a0642c44

res/drawable/ic_qrcode_24dp.xml
res/drawable/ic_scan_24dp.xml
res/layout/wifi_button_preference_widget.xml
res/layout/wifi_dialog.xml
src/com/android/settings/wifi/AddWifiNetworkPreference.java [new file with mode: 0644]
src/com/android/settings/wifi/ButtonPreference.java [deleted file]
src/com/android/settings/wifi/WifiSettings.java
tests/robotests/src/com/android/settings/wifi/ButtonPreferenceTest.java [deleted file]
tests/robotests/src/com/android/settings/wifi/WifiSettingsTest.java

index ff7806f..6928cb9 100644 (file)
@@ -2,7 +2,8 @@
     android:width="24dp"
     android:height="24dp"
     android:viewportWidth="24"
-    android:viewportHeight="24">
+    android:viewportHeight="24"
+    android:tint="?android:attr/colorAccent">
   <path
       android:fillColor="#FF000000"
       android:pathData="M7,4v3H4V4H7M9,2H2v7h7V2L9,2z"/>
index bcef8e3..c7b82d1 100644 (file)
@@ -2,7 +2,8 @@
     android:width="24dp"
     android:height="24dp"
     android:viewportWidth="24"
-    android:viewportHeight="24">
+    android:viewportHeight="24"
+    android:tint="?android:attr/colorAccent">
   <path
       android:fillColor="#FF000000"
       android:pathData="M9,2l-7,0l0,2l7,0l0,-2z"/>
index 0999d20..4b004e3 100644 (file)
@@ -22,5 +22,4 @@
            android:minWidth="@dimen/min_tap_target_size"
            android:minHeight="@dimen/min_tap_target_size"
            android:layout_gravity="center"
-           android:background="@null"
-           android:visibility="gone"/>
+           android:background="?android:attr/selectableItemBackground"/>
index 333296c..d5dbb17 100644 (file)
                         android:text="@string/wifi_ssid"
                         android:textDirection="locale" />
 
-                <RelativeLayout
+                <androidx.constraintlayout.widget.ConstraintLayout
+                        xmlns:app="http://schemas.android.com/apk/res-auto"
                         android:layout_width="match_parent"
-                        android:layout_height="wrap_content" >
+                        android:layout_height="wrap_content">
                     <EditText android:id="@+id/ssid"
-                            android:layout_width="match_parent"
+                            android:layout_width="0dp"
                             android:layout_height="wrap_content"
-                            android:layout_alignParentStart="true"
+                            app:layout_constraintStart_toStartOf="parent"
+                            app:layout_constraintEnd_toStartOf="@+id/ssid_scanner_button"
                             style="@style/wifi_item_edit_content"
                             android:hint="@string/wifi_ssid_hint"
                             android:singleLine="true"
-                            android:inputType="textNoSuggestions" />
+                            android:inputType="textNoSuggestions"/>
+
                     <ImageButton
                         android:id="@+id/ssid_scanner_button"
                         android:layout_width="wrap_content"
                         android:layout_height="wrap_content"
                         android:minWidth="@dimen/min_tap_target_size"
                         android:minHeight="@dimen/min_tap_target_size"
-                        android:layout_alignParentEnd="true"
-                        android:layout_centerVertical="true"
-                        android:background="@null"
+                        app:layout_constraintEnd_toEndOf="parent"
+                        android:background="?android:attr/selectableItemBackground"
                         android:src="@drawable/ic_scan_24dp"
                         android:contentDescription="@string/wifi_dpp_scan_qr_code"/>
-                </RelativeLayout>
+                </androidx.constraintlayout.widget.ConstraintLayout>
 
                 <LinearLayout android:id="@+id/ssid_too_long_warning"
                               android:layout_width="match_parent"
                         style="@style/wifi_item_label"
                         android:text="@string/wifi_password" />
 
-                <RelativeLayout
+                <androidx.constraintlayout.widget.ConstraintLayout
+                    xmlns:app="http://schemas.android.com/apk/res-auto"
                     android:layout_width="match_parent"
-                    android:layout_height="wrap_content" >
+                    android:layout_height="wrap_content">
                     <EditText android:id="@+id/password"
-                              android:layout_width="match_parent"
+                              android:layout_width="0dp"
                               android:layout_height="wrap_content"
-                              android:layout_alignParentStart="true"
+                              app:layout_constraintStart_toStartOf="parent"
+                              app:layout_constraintEnd_toStartOf="@+id/password_scanner_button"
                               style="@style/wifi_item_edit_content"
                               android:singleLine="true"
-                              android:password="true" />
+                              android:password="true"/>
 
                     <ImageButton
                         android:id="@+id/password_scanner_button"
                         android:layout_height="wrap_content"
                         android:minWidth="@dimen/min_tap_target_size"
                         android:minHeight="@dimen/min_tap_target_size"
-                        android:layout_alignParentEnd="true"
-                        android:layout_centerVertical="true"
-                        android:background="@null"
+                        app:layout_constraintEnd_toEndOf="parent"
+                        android:background="?android:attr/selectableItemBackground"
                         android:src="@drawable/ic_scan_24dp"
                         android:contentDescription="@string/wifi_dpp_scan_qr_code"/>
-                </RelativeLayout>
+                </androidx.constraintlayout.widget.ConstraintLayout>
             </LinearLayout>
 
             <LinearLayout android:id="@+id/show_password_layout"
diff --git a/src/com/android/settings/wifi/AddWifiNetworkPreference.java b/src/com/android/settings/wifi/AddWifiNetworkPreference.java
new file mode 100644 (file)
index 0000000..cd2e4a8
--- /dev/null
@@ -0,0 +1,83 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.settings.wifi;
+
+import android.content.Context;
+import android.content.res.Resources;
+import android.graphics.drawable.Drawable;
+import android.util.Log;
+import android.view.View;
+import android.widget.ImageButton;
+
+import androidx.annotation.DrawableRes;
+import androidx.preference.Preference;
+import androidx.preference.PreferenceViewHolder;
+
+import com.android.settings.R;
+import com.android.settings.wifi.dpp.WifiDppUtils;
+
+/**
+ * The Preference for users to add Wi-Fi networks in WifiSettings
+ */
+public class AddWifiNetworkPreference extends Preference {
+
+    private static final String TAG = "AddWifiNetworkPreference";
+
+    private boolean mInitialized;
+
+    public AddWifiNetworkPreference(Context context) {
+        super(context);
+
+        setLayoutResource(com.android.settingslib.R.layout.preference_access_point);
+        setWidgetLayoutResource(R.layout.wifi_button_preference_widget);
+        setIcon(R.drawable.ic_menu_add);
+        setTitle(R.string.wifi_add_network);
+    }
+
+    @Override
+    public void onBindViewHolder(PreferenceViewHolder holder) {
+        super.onBindViewHolder(holder);
+
+        if (!mInitialized) {
+            mInitialized = true;
+
+            final ImageButton imageButton = (ImageButton) holder.findViewById(R.id.button_icon);
+            imageButton.setImageDrawable(getDrawable(R.drawable.ic_scan_24dp));
+            imageButton.setContentDescription(
+                    getContext().getString(R.string.wifi_dpp_scan_qr_code));
+            imageButton.setOnClickListener(view -> {
+                getContext().startActivity(
+                    WifiDppUtils.getEnrolleeQrCodeScannerIntent(/* ssid */ null));
+            });
+
+            final View divider = (View) holder.findViewById(
+                    com.android.settingslib.R.id.two_target_divider);
+            divider.setVisibility(View.INVISIBLE);
+        }
+    }
+
+    private Drawable getDrawable(@DrawableRes int iconResId) {
+        Drawable buttonIcon = null;
+
+        try {
+            buttonIcon = getContext().getDrawable(iconResId);
+        } catch (Resources.NotFoundException exception) {
+            Log.e(TAG, "Resource does not exist: " + iconResId);
+        }
+        return buttonIcon;
+    }
+}
diff --git a/src/com/android/settings/wifi/ButtonPreference.java b/src/com/android/settings/wifi/ButtonPreference.java
deleted file mode 100644 (file)
index 5169d7a..0000000
+++ /dev/null
@@ -1,135 +0,0 @@
-/*
- * Copyright (C) 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * 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.
- * See the License for the specific language governing permissions and
- * limitations under the License
- */
-
-package com.android.settings.wifi;
-
-import android.content.Context;
-import android.content.res.Resources;
-import android.graphics.drawable.Drawable;
-import android.util.AttributeSet;
-import android.util.Log;
-import android.view.View;
-import android.widget.ImageButton;
-
-import androidx.annotation.DrawableRes;
-import androidx.annotation.VisibleForTesting;
-import androidx.preference.Preference;
-import androidx.preference.PreferenceViewHolder;
-
-import com.android.settings.R;
-
-/**
- * This preference provides one button layout with Settings style.
- * It looks like below
- *
- * --------------------------------------------------------------
- * | icon | title                                    |  button  |
- * --------------------------------------------------------------
- *
- * User can set icon / click listener for button.
- * By default, the button is invisible.
- */
-public class ButtonPreference extends Preference {
-
-    private static final String TAG = "ButtonPreference";
-
-    private ImageButton mImageButton;
-    private Drawable mButtonIcon;
-    private View.OnClickListener mClickListener;
-    private String mContentDescription;
-
-    // Used for dummy pref.
-    public ButtonPreference(Context context, AttributeSet attrs) {
-        super(context, attrs);
-        setWidgetLayoutResource(R.layout.wifi_button_preference_widget);
-        mImageButton = null;
-        mButtonIcon = null;
-        mClickListener = null;
-        mContentDescription = null;
-    }
-
-    public ButtonPreference(Context context) {
-        this(context, /* attrs */ null);
-    }
-
-    @Override
-    public void onBindViewHolder(final PreferenceViewHolder view) {
-        super.onBindViewHolder(view);
-        initButton(view);
-    }
-
-    @Override
-    public void setOrder(int order) {
-        super.setOrder(order);
-        setButtonVisibility();
-    }
-
-    @VisibleForTesting
-    protected void initButton(final PreferenceViewHolder view) {
-        if (mImageButton == null) {
-            mImageButton = (ImageButton) view.findViewById(R.id.button_icon);
-        }
-        if (mImageButton != null) {
-            mImageButton.setImageDrawable(mButtonIcon);
-            mImageButton.setOnClickListener(mClickListener);
-            mImageButton.setContentDescription(mContentDescription);
-        }
-        setButtonVisibility();
-    }
-
-    private void setButtonVisibility() {
-        if(mImageButton != null) {
-            mImageButton.setVisibility(mButtonIcon == null ? View.GONE : View.VISIBLE);
-        }
-    }
-
-    /**
-     * Sets the drawable to be displayed in button.
-     */
-    public void setButtonIcon(@DrawableRes int iconResId) {
-        if (iconResId == 0) {
-            return;
-        }
-
-        try {
-            mButtonIcon = getContext().getDrawable(iconResId);
-            notifyChanged();
-        } catch (Resources.NotFoundException exception) {
-            Log.e(TAG, "Resource does not exist: " + iconResId);
-        }
-    }
-
-    /**
-     * Register a callback to be invoked when button is clicked.
-     */
-    public void setButtonOnClickListener(View.OnClickListener listener) {
-        if (listener != mClickListener) {
-            mClickListener = listener;
-            notifyChanged();
-        }
-    }
-
-    /**
-     * A content description briefly describes the button and is primarily used for accessibility
-     * support to determine how a button should be presented to the user.
-     */
-    public void setButtonContentDescription(String contentDescription) {
-        if (contentDescription != mContentDescription) {
-            mContentDescription = contentDescription;
-            notifyChanged();
-        }
-    }
-}
index 9e5bfc8..7dd9163 100644 (file)
@@ -178,7 +178,8 @@ public class WifiSettings extends RestrictedSettingsFragment
 
     private PreferenceCategory mConnectedAccessPointPreferenceCategory;
     private PreferenceCategory mAccessPointsPreferenceCategory;
-    private ButtonPreference mAddPreference;
+    @VisibleForTesting
+    AddWifiNetworkPreference mAddWifiNetworkPreference;
     @VisibleForTesting
     Preference mConfigureWifiSettingsPreference;
     @VisibleForTesting
@@ -236,20 +237,8 @@ public class WifiSettings extends RestrictedSettingsFragment
                 (PreferenceCategory) findPreference(PREF_KEY_ACCESS_POINTS);
         mConfigureWifiSettingsPreference = findPreference(PREF_KEY_CONFIGURE_WIFI_SETTINGS);
         mSavedNetworksPreference = findPreference(PREF_KEY_SAVED_NETWORKS);
-
-        Context prefContext = getPrefContext();
-        mAddPreference = new ButtonPreference(prefContext);
-        mAddPreference.setIcon(R.drawable.ic_menu_add);
-        mAddPreference.setTitle(R.string.wifi_add_network);
-        mAddPreference.setButtonIcon(R.drawable.ic_scan_24dp);
-        mAddPreference.setButtonOnClickListener((View v) -> {
-            // Launch QR code scanner to join a network.
-            getContext().startActivity(
-                    WifiDppUtils.getEnrolleeQrCodeScannerIntent(/* ssid */ null));
-        });
-        mAddPreference.setButtonContentDescription(getString(R.string.wifi_dpp_scan_qr_code));
+        mAddWifiNetworkPreference = new AddWifiNetworkPreference(getPrefContext());
         mStatusMessagePreference = (LinkablePreference) findPreference(PREF_KEY_STATUS_MESSAGE);
-
         mUserBadgeCache = new AccessPointPreference.UserBadgeCache(getPackageManager());
     }
 
@@ -589,7 +578,7 @@ public class WifiSettings extends RestrictedSettingsFragment
                     showDialog(mSelectedAccessPoint, WifiConfigUiBase.MODE_CONNECT);
                     break;
             }
-        } else if (preference == mAddPreference) {
+        } else if (preference == mAddWifiNetworkPreference) {
             onAddNetworkPressed();
         } else {
             return super.onPreferenceTreeClick(preference);
@@ -808,8 +797,8 @@ public class WifiSettings extends RestrictedSettingsFragment
             }
         }
         removeCachedPrefs(mAccessPointsPreferenceCategory);
-        mAddPreference.setOrder(index);
-        mAccessPointsPreferenceCategory.addPreference(mAddPreference);
+        mAddWifiNetworkPreference.setOrder(index);
+        mAccessPointsPreferenceCategory.addPreference(mAddWifiNetworkPreference);
         setAdditionalSettingsSummaries();
 
         if (!hasAvailableAccessPoints) {
diff --git a/tests/robotests/src/com/android/settings/wifi/ButtonPreferenceTest.java b/tests/robotests/src/com/android/settings/wifi/ButtonPreferenceTest.java
deleted file mode 100644 (file)
index 7f0598d..0000000
+++ /dev/null
@@ -1,82 +0,0 @@
-/*
- * Copyright (C) 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * 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.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.settings.wifi;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import android.content.Context;
-import android.view.View;
-import android.widget.ImageButton;
-
-import androidx.preference.PreferenceViewHolder;
-
-import com.android.settings.R;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.MockitoAnnotations;
-import org.robolectric.RobolectricTestRunner;
-import org.robolectric.RuntimeEnvironment;
-
-@RunWith(RobolectricTestRunner.class)
-public class ButtonPreferenceTest {
-
-    private Context mContext;
-    private View mRootView;
-    private ButtonPreference mPref;
-    private PreferenceViewHolder mHolder;
-    private boolean mClicked;
-
-    @Before
-    public void setUp() {
-        MockitoAnnotations.initMocks(this);
-
-        mContext = RuntimeEnvironment.application;
-        mPref = new ButtonPreference(mContext);
-        mRootView = View.inflate(mContext, R.layout.wifi_button_preference_widget, /* parent */
-                null);
-        mHolder = PreferenceViewHolder.createInstanceForTests(mRootView);
-    }
-
-    @Test
-    public void initButton_noIcon_shouldInvisible() {
-        mPref.initButton(mHolder);
-        assertThat(mRootView.findViewById(R.id.button_icon).getVisibility()).isEqualTo(View.GONE);
-    }
-
-    @Test
-    public void initButton_withIcon_shouldVisible() {
-        mPref.setButtonIcon(R.drawable.ic_qrcode_24dp);
-        mPref.initButton(mHolder);
-        assertThat(mRootView.findViewById(R.id.button_icon).getVisibility()).isEqualTo(
-                View.VISIBLE);
-    }
-
-    @Test
-    public void initButton_whenClick_shouldCallback() {
-        mClicked = false;
-        mPref.setButtonIcon(R.drawable.ic_qrcode_24dp);
-        mPref.setButtonOnClickListener((View v) -> {
-            mClicked = true;
-        });
-        mPref.initButton(mHolder);
-        ImageButton button = (ImageButton) mRootView.findViewById(R.id.button_icon);
-        button.performClick();
-        assertThat(mClicked).isTrue();
-    }
-}
index 22ccd3c..c811b0c 100644 (file)
@@ -69,6 +69,7 @@ public class WifiSettingsTest {
         mWifiSettings = spy(new WifiSettings());
         doReturn(mContext).when(mWifiSettings).getContext();
         doReturn(mPowerManager).when(mContext).getSystemService(PowerManager.class);
+        mWifiSettings.mAddWifiNetworkPreference = new AddWifiNetworkPreference(mContext);
         mWifiSettings.mSavedNetworksPreference = new Preference(mContext);
         mWifiSettings.mConfigureWifiSettingsPreference = new Preference(mContext);
         mWifiSettings.mWifiTracker = mWifiTracker;
@@ -151,4 +152,11 @@ public class WifiSettingsTest {
         assertThat(mWifiSettings.mConfigureWifiSettingsPreference.getSummary()).isEqualTo(
                 mContext.getString(R.string.wifi_configure_settings_preference_summary_wakeup_off));
     }
-}
\ No newline at end of file
+
+    @Test
+    public void checkAddWifiNetworkPrefernce_preferenceVisible() {
+        assertThat(mWifiSettings.mAddWifiNetworkPreference.isVisible()).isTrue();
+        assertThat(mWifiSettings.mAddWifiNetworkPreference.getTitle()).isEqualTo(
+                mContext.getString(R.string.wifi_add_network));
+    }
+}