Accessibility shortcut secondary action - improve shortcutPreference

1. Use setWidgetLayoutResource() for the controllable widget at the
right side.
2. Improve listener structure.
3. Extend checkbox clickable area.

Bug: 142530063
Test: make -j52 RunSettingsRoboTests ROBOTEST_FILTER=ShortcutPreferenceTest
Change-Id: Iebb9f62653914a0d7200c627f01cf7953d576960

res/layout/accessibility_shortcut_secondary_action.xml

@@ -24,12 +24,14 @@
     android:clipToPadding="false">
 
     <LinearLayout
+        android:id="@+id/main_frame"
         android:layout_width="0dp"
-        android:layout_height="match_parent"
+        android:layout_height="wrap_content"
         android:layout_weight="1"
-        android:background="?android:attr/selectableItemBackground"
         android:gravity="start|center_vertical"
-        android:clipToPadding="false"
+        android:focusable="true"
+        android:clickable="true"
+        android:background="?android:attr/selectableItemBackground"
         android:paddingStart="?android:attr/listPreferredItemPaddingStart"
         android:paddingEnd="?android:attr/listPreferredItemPaddingEnd">
 
@@ -39,23 +41,26 @@
             android:gravity="start|center_vertical"
             android:minWidth="56dp"
             android:orientation="horizontal"
-            android:clipToPadding="false"
             android:paddingRight="16dp"
             android:paddingTop="4dp"
-            android:paddingBottom="4dp">
+            android:paddingBottom="4dp"
+            android:focusable="false"
+            android:clickable="false">
 
             <CheckBox
                 android:id="@+id/checkbox"
                 android:layout_width="wrap_content"
                 android:layout_height="wrap_content"
-                android:layout_gravity="center"/>
+                android:layout_gravity="center"
+                android:focusable="false"
+                android:clickable="false"
+                android:background="@null"/>
 
         </FrameLayout>
 
         <RelativeLayout
             android:layout_width="wrap_content"
             android:layout_height="wrap_content"
-            android:layout_weight="1"
             android:paddingTop="16dp"
             android:paddingBottom="16dp">
 
@@ -71,8 +76,8 @@
                 android:id="@android:id/summary"
                 android:layout_width="wrap_content"
                 android:layout_height="wrap_content"
-                android:layout_below="@+android:id/title"
+                android:layout_below="@android:id/title"
-                android:layout_alignStart="@+android:id/title"
+                android:layout_alignStart="@android:id/title"
                 android:textAppearance="?android:attr/textAppearanceListItemSecondary"
                 android:textColor="?android:attr/textColorSecondary"
                 android:maxLines="10" />
@@ -81,41 +86,20 @@
 
     </LinearLayout>
 
-    <FrameLayout
+    <View
-        android:layout_width="wrap_content"
+        android:layout_width="1dp"
         android:layout_height="match_parent"
-        android:gravity="start|center_vertical"
+        android:layout_marginTop="16dp"
-        android:orientation="horizontal"
+        android:layout_marginBottom="16dp"
-        android:paddingTop="16dp"
+        android:background="?android:attr/listDivider" />
-        android:paddingBottom="16dp">
-
-        <View
-            android:layout_width="1dp"
-            android:layout_height="match_parent"
-            android:background="?android:attr/listDivider" />
-
-    </FrameLayout>
 
     <!-- Preference should place its actual preference widget here. -->
-    <FrameLayout
+    <LinearLayout
+        android:id="@android:id/widget_frame"
         android:layout_width="wrap_content"
         android:layout_height="match_parent"
-        android:minWidth="64dp"
+        android:minWidth="58dp"
-        android:gravity="center"
+        android:gravity="end|center_vertical"
-        android:orientation="vertical" >
+        android:orientation="vertical" />
-
-        <ImageView
-            android:id="@+id/settings_button"
-            android:layout_width="match_parent"
-            android:layout_height="match_parent"
-            android:layout_gravity="center"
-            android:paddingStart="?android:attr/listPreferredItemPaddingEnd"
-            android:paddingEnd="?android:attr/listPreferredItemPaddingEnd"
-            android:background="?android:attr/selectableItemBackground"
-            android:scaleType="center"
-            android:src="@drawable/ic_mode_edit"
-            android:contentDescription="@string/settings_button" />
-
-    </FrameLayout>
 
 </LinearLayout>

res/layout/preference_widget_settings.xml

@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+  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
+  -->
+
+<!-- Layout used by ShortcutPreference. This is inflated inside android.R.layout.preference. -->
+<ImageView xmlns:android="http://schemas.android.com/apk/res/android"
+    android:id="@+id/settings_button"
+    android:layout_width="match_parent"
+    android:layout_height="match_parent"
+    android:layout_gravity="center"
+    android:background="?android:attr/selectableItemBackground"
+    android:scaleType="center"
+    android:src="@drawable/ic_mode_edit"
+    android:contentDescription="@string/settings_button" />

src/com/android/settings/accessibility/ShortcutPreference.java

@@ -20,8 +20,8 @@ import android.content.Context;
 import android.util.AttributeSet;
 import android.view.View;
 import android.widget.CheckBox;
+import android.widget.LinearLayout;
 
-import androidx.annotation.Nullable;
 import androidx.preference.Preference;
 import androidx.preference.PreferenceViewHolder;
 
@@ -33,76 +33,112 @@ import com.android.settings.R;
  */
 public class ShortcutPreference extends Preference {
 
-    private View.OnClickListener mCheckBoxListener;
+    /**
-    private View.OnClickListener mSettingButtonListener;
+     * Interface definition for a callback to be invoked when the checkbox or settings has been
+     * clicked.
+     */
+    public interface OnClickListener {
+        /**
+         * Called when the checkbox in ShortcutPreference has been clicked.
+         *
+         * @param preference The clicked preference
+         */
+        void onCheckboxClicked(ShortcutPreference preference);
+        /**
+         * Called when the settings view has been clicked.
+         *
+         * @param preference The clicked preference
+         */
+        void onSettingsClicked(ShortcutPreference preference);
+    }
+    private OnClickListener mListener = null;
 
+    private int mSettingsVisibility = View.VISIBLE;
     private boolean mChecked = false;
 
     ShortcutPreference(Context context, AttributeSet attrs) {
         super(context, attrs);
-
+        init();
-        setLayoutResource(R.layout.accessibility_shortcut_secondary_action);
     }
 
     @Override
     public void onBindViewHolder(PreferenceViewHolder holder) {
         super.onBindViewHolder(holder);
-        holder.itemView.setClickable(false);
 
         final CheckBox checkBox = holder.itemView.findViewById(R.id.checkbox);
-        checkBox.setOnClickListener(mCheckBoxListener);
+        final LinearLayout mainFrame = holder.itemView.findViewById(R.id.main_frame);
-        checkBox.setChecked(mChecked);
+        if (mainFrame != null && checkBox != null) {
-        final View settingButton = holder.itemView.findViewById(R.id.settings_button);
+            mainFrame.setOnClickListener(view -> callOnCheckboxClicked());
-        settingButton.setOnClickListener(mSettingButtonListener);
+            checkBox.setChecked(mChecked);
+        }
+
+        final View settings = holder.itemView.findViewById(android.R.id.widget_frame);
+        if (settings != null) {
+            settings.setOnClickListener(view -> callOnSettingsClicked());
+            settings.setVisibility(mSettingsVisibility);
+        }
     }
 
     /**
      * Set the shortcut checkbox according to checked value.
      *
-     * @param checked the state value of shortcut checkbox.
+     * @param checked the state value of shortcut checkbox
      */
     public void setChecked(boolean checked) {
-        if (checked != mChecked) {
+        if (mChecked != checked) {
             mChecked = checked;
             notifyChanged();
         }
     }
 
     /**
-     * Set the given onClickListener to the SettingButtonListener.
+     * Get the checked value of shortcut checkbox.
      *
-     * @param listener the given onClickListener.
+     * @return the checked value of shortcut checkbox
      */
-    public void setSettingButtonListener(@Nullable View.OnClickListener listener) {
+    public boolean getChecked() {
-        mSettingButtonListener = listener;
+        return mChecked;
-        notifyChanged();
+    }
+
+
+
+    /**
+     * Sets the visibility state of Settings view.
+     *
+     * @param visibility one of {@link View#VISIBLE}, {@link View#INVISIBLE}, or {@link View#GONE}.
+     */
+    public void setSettingsVisibility(@View.Visibility int visibility) {
+        if (mSettingsVisibility != visibility) {
+            mSettingsVisibility = visibility;
+            notifyChanged();
+        }
     }
 
     /**
-     * Returns the callback to be invoked when the setting button is clicked.
+     * Sets the callback to be invoked when this preference is clicked by the user.
      *
-     * @return The callback to be invoked
+     * @param listener the callback to be invoked
      */
-    public View.OnClickListener getSettingButtonListener() {
+    public void setOnClickListener(OnClickListener listener) {
-        return mSettingButtonListener;
+        mListener = listener;
     }
 
-    /**
+    private void init() {
-     * Set the given onClickListener to the CheckBoxListener.
+        setLayoutResource(R.layout.accessibility_shortcut_secondary_action);
-     *
+        setWidgetLayoutResource(R.layout.preference_widget_settings);
-     * @param listener the given onClickListener.
+        setIconSpaceReserved(false);
-     */
-    public void setCheckBoxListener(@Nullable View.OnClickListener listener) {
-        mCheckBoxListener = listener;
-        notifyChanged();
     }
 
-    /**
+    private void callOnSettingsClicked() {
-     * Returns the callback to be invoked when the checkbox is clicked.
+        if (mListener != null) {
-     *
+            mListener.onSettingsClicked(this);
-     * @return The callback to be invoked
+        }
-     */
+    }
-    public View.OnClickListener getCheckBoxListener() {
+
-        return mCheckBoxListener;
+    private void callOnCheckboxClicked() {
+        setChecked(!mChecked);
+        if (mListener != null) {
+            mListener.onCheckboxClicked(this);
+        }
     }
 }

src/com/android/settings/accessibility/ToggleAccessibilityServicePreferenceFragment.java

@@ -43,7 +43,6 @@ import android.view.MenuInflater;
 import android.view.View;
 import android.view.ViewGroup;
 import android.view.accessibility.AccessibilityManager;
-import android.widget.CheckBox;
 
 import androidx.preference.PreferenceScreen;
 
@@ -57,8 +56,11 @@ import com.android.settingslib.accessibility.AccessibilityUtils;
 
 import java.util.List;
 
-public class ToggleAccessibilityServicePreferenceFragment extends ToggleFeaturePreferenceFragment {
+/** Fragment for providing toggle bar and basic accessibility service setup. */
+public class ToggleAccessibilityServicePreferenceFragment extends
+        ToggleFeaturePreferenceFragment implements ShortcutPreference.OnClickListener {
 
+    private static final String KEY_SHORTCUT_PREFERENCE = "shortcut_preference";
     private static final int DIALOG_ID_ENABLE_WARNING = 1;
     private static final int DIALOG_ID_DISABLE_WARNING = 2;
     private static final int DIALOG_ID_LAUNCH_ACCESSIBILITY_TUTORIAL = 3;
@@ -77,18 +79,6 @@ public class ToggleAccessibilityServicePreferenceFragment extends ToggleFeatureP
                 }
             };
 
-    private final View.OnClickListener mSettingButtonListener = (View view) -> showDialog(
-            DIALOG_ID_EDIT_SHORTCUT);
-
-    private final View.OnClickListener mCheckBoxListener = (View view) -> {
-        CheckBox checkBox = (CheckBox) view;
-        if (checkBox.isChecked()) {
-            // TODO(b/142530063): Enable shortcut when checkbox is checked.
-        } else {
-            // TODO(b/142530063): Disable shortcut when checkbox is unchecked.
-        }
-    };
-
     private final SettingsContentObserver mSettingsContentObserver =
             new SettingsContentObserver(new Handler()) {
                 @Override
@@ -275,16 +265,22 @@ public class ToggleAccessibilityServicePreferenceFragment extends ToggleFeatureP
         final ShortcutPreference shortcutPreference = new ShortcutPreference(
                 preferenceScreen.getContext(), null);
         // Put the shortcutPreference before settingsPreference.
+        shortcutPreference.setPersistent(false);
+        shortcutPreference.setKey(getShortcutPreferenceKey());
         shortcutPreference.setOrder(-1);
         shortcutPreference.setTitle(R.string.accessibility_shortcut_title);
+        shortcutPreference.setOnClickListener(this);
+
         // TODO(b/142530063): Check the new setting key to decide which summary should be shown.
         // TODO(b/142530063): Check if gesture mode is on to decide which summary should be shown.
         // TODO(b/142530063): Check the new key to decide whether checkbox should be checked.
-        shortcutPreference.setSettingButtonListener(mSettingButtonListener);
-        shortcutPreference.setCheckBoxListener(mCheckBoxListener);
         preferenceScreen.addPreference(shortcutPreference);
     }
 
+    public String getShortcutPreferenceKey() {
+        return KEY_SHORTCUT_PREFERENCE;
+    }
+
     private void updateSwitchBarToggleSwitch() {
         final boolean checked = AccessibilityUtils.getEnabledServicesFromSettings(getActivity())
                 .contains(mComponentName);
@@ -387,6 +383,20 @@ public class ToggleAccessibilityServicePreferenceFragment extends ToggleFeatureP
         });
     }
 
+    @Override
+    public void onCheckboxClicked(ShortcutPreference preference) {
+        if (preference.getChecked()) {
+            // TODO(b/142530063): Enable shortcut when checkbox is checked.
+        } else {
+            // TODO(b/142530063): Disable shortcut when checkbox is unchecked.
+        }
+    }
+
+    @Override
+    public void onSettingsClicked(ShortcutPreference preference) {
+        showDialog(DIALOG_ID_EDIT_SHORTCUT);
+    }
+
     @Override
     protected void onProcessArguments(Bundle arguments) {
         super.onProcessArguments(arguments);

src/com/android/settings/accessibility/ToggleDaltonizerPreferenceFragment.java

@@ -26,7 +26,6 @@ import android.provider.Settings;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
-import android.widget.CheckBox;
 import android.widget.Switch;
 
 import androidx.preference.Preference;
@@ -46,7 +45,7 @@ import java.util.List;
 @SearchIndexable
 public final class ToggleDaltonizerPreferenceFragment extends ToggleFeaturePreferenceFragment
         implements DaltonizerRadioButtonPreferenceController.OnChangeListener,
-        SwitchBar.OnSwitchChangeListener {
+        SwitchBar.OnSwitchChangeListener, ShortcutPreference.OnClickListener {
 
     public static final BaseSearchIndexProvider SEARCH_INDEX_DATA_PROVIDER =
             new BaseSearchIndexProvider(R.xml.accessibility_daltonizer_settings);
@@ -77,18 +76,6 @@ public final class ToggleDaltonizerPreferenceFragment extends ToggleFeaturePrefe
                 }
             };
 
-    private final View.OnClickListener mSettingButtonListener =
-            (View view) -> showDialog(DIALOG_ID_EDIT_SHORTCUT);
-
-    private final View.OnClickListener mCheckBoxListener = (View view) -> {
-        CheckBox checkBox = (CheckBox) view;
-        if (checkBox.isChecked()) {
-            // TODO(b/142530063): Enable shortcut when checkbox is checked.
-        } else {
-            // TODO(b/142530063): Disable shortcut when checkbox is unchecked.
-        }
-    };
-
     private Dialog mDialog;
 
     @Override
@@ -191,6 +178,20 @@ public final class ToggleDaltonizerPreferenceFragment extends ToggleFeaturePrefe
         mSwitchBar.addOnSwitchChangeListener(this);
     }
 
+    @Override
+    public void onCheckboxClicked(ShortcutPreference preference) {
+        if (preference.getChecked()) {
+            // TODO(b/142530063): Enable shortcut when checkbox is checked.
+        } else {
+            // TODO(b/142530063): Disable shortcut when checkbox is unchecked.
+        }
+    }
+
+    @Override
+    public void onSettingsClicked(ShortcutPreference preference) {
+        showDialog(DIALOG_ID_EDIT_SHORTCUT);
+    }
+
     private void initShortcutPreference() {
         final PreferenceScreen preferenceScreen = getPreferenceScreen();
         final ShortcutPreference shortcutPreference = new ShortcutPreference(
@@ -202,8 +203,7 @@ public final class ToggleDaltonizerPreferenceFragment extends ToggleFeaturePrefe
         // TODO(b/142530063): Check the new setting key to decide which summary should be shown.
         // TODO(b/142530063): Check if gesture mode is on to decide which summary should be shown.
         // TODO(b/142530063): Check the new key to decide whether checkbox should be checked.
-        shortcutPreference.setSettingButtonListener(mSettingButtonListener);
+        shortcutPreference.setOnClickListener(this);
-        shortcutPreference.setCheckBoxListener(mCheckBoxListener);
         preferenceScreen.addPreference(shortcutPreference);
     }
 }

src/com/android/settings/accessibility/ToggleScreenMagnificationPreferenceFragment.java

@@ -38,7 +38,6 @@ import android.view.View;
 import android.view.ViewGroup;
 import android.view.ViewTreeObserver.OnGlobalLayoutListener;
 import android.view.WindowManager;
-import android.widget.CheckBox;
 import android.widget.ImageView;
 import android.widget.RelativeLayout.LayoutParams;
 import android.widget.Switch;
@@ -52,7 +51,8 @@ import com.android.settings.R;
 import com.android.settings.widget.SwitchBar;
 
 public class ToggleScreenMagnificationPreferenceFragment extends
-        ToggleFeaturePreferenceFragment implements SwitchBar.OnSwitchChangeListener {
+        ToggleFeaturePreferenceFragment implements SwitchBar.OnSwitchChangeListener,
+        ShortcutPreference.OnClickListener {
 
     private static final String SETTINGS_KEY = "screen_magnification_settings";
     private static final int DIALOG_ID_GESTURE_NAVIGATION_TUTORIAL = 1;
@@ -66,18 +66,6 @@ public class ToggleScreenMagnificationPreferenceFragment extends
                 }
             };
 
-    private final View.OnClickListener mSettingButtonListener =
-            (View view) -> showDialog(DIALOG_ID_EDIT_SHORTCUT);
-
-    private final View.OnClickListener mCheckBoxListener = (View view) -> {
-        CheckBox checkBox = (CheckBox) view;
-        if (checkBox.isChecked()) {
-            // TODO(b/142530063): Enable shortcut when checkbox is checked.
-        } else {
-            // TODO(b/142530063): Disable shortcut when checkbox is unchecked.
-        }
-    };
-
     private Dialog mDialog;
 
     protected class VideoPreference extends Preference {
@@ -327,6 +315,20 @@ public class ToggleScreenMagnificationPreferenceFragment extends
         }
     }
 
+    @Override
+    public void onCheckboxClicked(ShortcutPreference preference) {
+        if (preference.getChecked()) {
+            // TODO(b/142530063): Enable shortcut when checkbox is checked.
+        } else {
+            // TODO(b/142530063): Disable shortcut when checkbox is unchecked.
+        }
+    }
+
+    @Override
+    public void onSettingsClicked(ShortcutPreference preference) {
+        showDialog(DIALOG_ID_EDIT_SHORTCUT);
+    }
+
     private void initShortcutPreference() {
         final PreferenceScreen preferenceScreen = getPreferenceScreen();
         final ShortcutPreference shortcutPreference = new ShortcutPreference(
@@ -334,11 +336,10 @@ public class ToggleScreenMagnificationPreferenceFragment extends
         // Put the shortcutPreference before videoPreference.
         shortcutPreference.setOrder(mVideoPreference.getOrder() - 1);
         shortcutPreference.setTitle(R.string.accessibility_magnification_shortcut_title);
+        shortcutPreference.setOnClickListener(this);
         // TODO(b/142530063): Check the new setting key to decide which summary should be shown.
         // TODO(b/142530063): Check if gesture mode is on to decide which summary should be shown.
         // TODO(b/142530063): Check the new key to decide whether checkbox should be checked.
-        shortcutPreference.setSettingButtonListener(mSettingButtonListener);
-        shortcutPreference.setCheckBoxListener(mCheckBoxListener);
         preferenceScreen.addPreference(shortcutPreference);
     }
 

tests/robotests/src/com/android/settings/accessibility/ShortcutPreferenceTest.java

@@ -19,7 +19,13 @@ package com.android.settings.accessibility;
 import static com.google.common.truth.Truth.assertThat;
 
 import android.content.Context;
+import android.view.LayoutInflater;
 import android.view.View;
+import android.widget.LinearLayout;
+
+import androidx.preference.PreferenceViewHolder;
+
+import com.android.settings.R;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -31,23 +37,64 @@ import org.robolectric.RuntimeEnvironment;
 @RunWith(RobolectricTestRunner.class)
 public class ShortcutPreferenceTest {
 
+    private static final String CHECKBOX_CLICKED = "checkbox_clicked";
+    private static final String SETTINGS_CLICKED = "settings_clicked";
+
     private ShortcutPreference mShortcutPreference;
-    private View.OnClickListener mSettingButtonListener;
+    private PreferenceViewHolder mPreferenceViewHolder;
-    private View.OnClickListener mCheckBoxListener;
+    private String mResult;
+
+    private ShortcutPreference.OnClickListener mListener =
+            new ShortcutPreference.OnClickListener() {
+                @Override
+                public void onCheckboxClicked(ShortcutPreference preference) {
+                    mResult = CHECKBOX_CLICKED;
+                }
+
+                @Override
+                public void onSettingsClicked(ShortcutPreference preference) {
+                    mResult = SETTINGS_CLICKED;
+                }
+            };
 
     @Before
     public void setUp() {
-        final Context mContext = RuntimeEnvironment.application;
+        final Context context = RuntimeEnvironment.application;
-        mShortcutPreference = new ShortcutPreference(mContext, null);
+        mShortcutPreference = new ShortcutPreference(context, null);
+
+        final LayoutInflater inflater = LayoutInflater.from(context);
+        final View view =
+                inflater.inflate(R.layout.accessibility_shortcut_secondary_action, null);
+        mPreferenceViewHolder = PreferenceViewHolder.createInstanceForTests(view);
     }
     
     @Test
-    public void setOnClickListeners_shouldSetListeners() {
+    public void testClickLinearLayout_checkboxClicked() {
-        mShortcutPreference.setSettingButtonListener(mSettingButtonListener);
+        mShortcutPreference.onBindViewHolder(mPreferenceViewHolder);
-        mShortcutPreference.setCheckBoxListener(mCheckBoxListener);
+        mShortcutPreference.setOnClickListener(mListener);
 
-        assertThat(mShortcutPreference.getCheckBoxListener()).isEqualTo(mCheckBoxListener);
+        LinearLayout mainFrame = mPreferenceViewHolder.itemView.findViewById(R.id.main_frame);
-        assertThat(mShortcutPreference.getSettingButtonListener()).isEqualTo(
+        mainFrame.performClick();
-                mSettingButtonListener);
+
+        assertThat(mResult).isEqualTo(CHECKBOX_CLICKED);
+        assertThat(mShortcutPreference.getChecked()).isTrue();
+    }
+
+    @Test
+    public void testClickSettings_settingsClicked() {
+        mShortcutPreference.onBindViewHolder(mPreferenceViewHolder);
+        mShortcutPreference.setOnClickListener(mListener);
+
+        View settings = mPreferenceViewHolder.itemView.findViewById(android.R.id.widget_frame);
+        settings.performClick();
+
+        assertThat(mResult).isEqualTo(SETTINGS_CLICKED);
+    }
+
+    @Test
+    public void testSetCheckedTrue_getCheckedIsTrue() {
+        mShortcutPreference.setChecked(true);
+
+        assertThat(mShortcutPreference.getChecked()).isEqualTo(true);
     }
 }