UX fixes for the language draggable cell

This should bring things closer to the recommended UX guidelines.

It changes the way the checkbox / label play together,
and changes the "localization missing" warning icon to a string.
Also some cleanup of hard-coded styles and sizes.

Bug: 26758865
Bug: 26707846
Change-Id: I07a78a1e4a41122b91e895e363a217d3e9cacd40

res/drawable/drag_handle.xml

@@ -15,15 +15,13 @@
 -->
 
 <vector xmlns:android="http://schemas.android.com/apk/res/android"
-        android:height="50dp"
+        android:height="24dp"
-        android:width="50dp"
+        android:width="24dp"
-        android:viewportHeight="100"
+        android:viewportHeight="48"
-        android:viewportWidth="100"
+        android:viewportWidth="48"
-        android:tint="#757575" >
+        android:tint="@color/material_grey_600">
 
     <path android:fillColor="@android:color/white"
-          android:pathData="M29,48 l 0,-4 42,0 0,4 z" />
+          android:pathData="M8,18v4h32v-4hm-32,8v4h32v-4z" />
-    <path android:fillColor="@android:color/white"
-          android:pathData="M29,56 l 0,-4 42,0 0,4 z" />
 
 </vector>

res/layout/locale_drag_cell.xml

@@ -19,61 +19,64 @@
               xmlns:tools="http://schemas.android.com/tools"
               android:layout_width="match_parent"
               android:layout_height="wrap_content"
-              android:gravity="center_vertical"
+              android:minHeight="?android:listPreferredItemHeight"
-              android:orientation="horizontal"
+              android:background="?android:colorBackground"
-              android:baselineAligned="false"
               android:layoutDirection="locale"
               android:textDirection="locale">
 
+    <CheckBox
+        android:id="@+id/checkbox"
+        style="@style/LanguageCheckboxAndLabel"
+        tools:text="French"
+        android:paddingStart="16dp"
+        android:layout_toStartOf="@+id/dragHandle"/>
+
+    <TextView
+        android:id="@+id/label"
+        style="@style/LanguageCheckboxAndLabel"
+        tools:text="French"
+        android:paddingStart="48dp"
+        android:layout_toStartOf="@+id/dragHandle"/>
+
     <TextView
         android:id="@+id/miniLabel"
         android:layout_width="wrap_content"
         android:layout_height="match_parent"
-        android:layout_marginLeft="12dp"
+        android:layout_marginStart="?android:attr/listPreferredItemPaddingStart"
-        android:layout_marginRight="12dp"
         tools:text="22"
         android:textColor="?android:attr/colorAccent"
-        android:minWidth="20sp"
+        android:minWidth="32sp"
-        android:textAlignment="textEnd"
+        android:gravity="center_vertical|center_horizontal"
-        android:gravity="center_vertical|end"
+        android:layout_alignTop="@id/checkbox"
-        android:background="?android:colorBackground"/>
+        android:layout_alignBottom="@id/checkbox"/>
-
-    <CheckBox
-        android:id="@+id/checkbox"
-        android:layout_width="wrap_content"
-        android:layout_height="wrap_content"
-        android:minWidth="20sp"
-        android:layout_marginLeft="6dp"
-        android:layout_marginRight="6dp"
-        android:visibility="gone"
-        android:background="?android:colorBackground"/>
 
     <TextView
-        android:id="@+id/label"
-        android:layout_width="0dp"
-        android:layout_height="wrap_content"
-        android:layout_weight="1"
-        android:padding="12dp"
-        tools:text="France"
-        android:textAppearance="?android:attr/textAppearanceListItem"
-        android:background="?android:colorBackground"/>
-
-    <ImageView
         android:id="@+id/l10nWarn"
-        android:layout_width="wrap_content"
+        style="@style/LanguageCheckboxAndLabel"
-        android:layout_height="match_parent"
+        android:layout_marginTop="-28dp"
-        android:src="@android:drawable/stat_sys_warning"
+        android:paddingStart="48dp"
-        android:focusableInTouchMode="false"
+        android:textAppearance="?android:attr/textAppearanceListItemSecondary"
-        android:focusable="false"
+        android:textColor="?android:textColorSecondary"
-        android:paddingStart="12dp"
+        android:text="@string/locale_not_translated"
-        android:paddingEnd="12dp"
+        android:layout_toStartOf="@+id/dragHandle"
-        android:tint="?android:attr/colorAccent"/>
+        android:layout_below="@id/label"/>
 
     <ImageView
         android:id="@+id/dragHandle"
         android:layout_width="wrap_content"
         android:layout_height="match_parent"
+        android:paddingStart="?android:attr/listPreferredItemPaddingStart"
+        android:paddingEnd="?android:attr/listPreferredItemPaddingEnd"
         android:layout_gravity="center_vertical"
-        android:src="@drawable/drag_handle"/>
+        android:src="@drawable/drag_handle"
+        android:layout_alignParentEnd="true"
+        android:layout_alignTop="@id/checkbox"
+        android:layout_alignBottom="@id/checkbox"/>
+
+    <View
+        android:layout_width="match_parent"
+        android:layout_height="1dp"
+        android:background="?android:attr/listDivider"
+        android:layout_alignParentBottom="true"/>
 
 </com.android.settings.localepicker.LocaleDragCell>

res/layout/locale_order_list.xml

@@ -17,8 +17,10 @@
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
               android:layout_width="match_parent"
               android:layout_height="match_parent"
-              android:layoutDirection="locale"
+              android:paddingTop="8dp"
+              android:paddingBottom="8dp"
               android:orientation="vertical"
+              android:layoutDirection="locale"
               android:textDirection="locale">
 
     <com.android.settings.localepicker.LocaleRecyclerView
@@ -31,10 +33,11 @@
     <Button
         android:id="@+id/add_language"
         android:layout_width="match_parent"
-        android:layout_height="wrap_content"
+        android:layout_height="?android:listPreferredItemHeight"
+        android:paddingStart="?android:attr/listPreferredItemPaddingStart"
+        android:paddingEnd="?android:attr/listPreferredItemPaddingEnd"
         android:drawableStart="@drawable/ic_add_24dp"
-        android:drawablePadding="12dp"
+        android:drawablePadding="24dp"
-        android:minHeight="64dp"
         android:textAlignment="textStart"
         android:text="@string/add_a_language"
         style="@style/Base.Widget.AppCompat.Button.Borderless"

res/values/strings.xml

@@ -536,6 +536,9 @@
     <!-- The text of the error dialog shown when the user selected all the languages and tries to remove them [CHAR LIMIT=NONE] -->
     <string name="dlg_remove_locales_error_message">Can’t remove all the languages, you should leave at least one.</string>
 
+    <!-- TODO: Marked non-translatable for now, as it is not final. Waiting on b/27070104 -->
+    <!-- This text shows in the language picker when the system is not translated into that languages [CHAR LIMIT=130] -->
+    <string name="locale_not_translated" translatable="false">Not all applications are translated into this language; you might experience a mixture.</string>
 
     <!-- The title of the dialog to pick an activity.  This is shown when there are multiple activities that can do a particular action.  For example, suppose you click on the "Share" menu item in the Browser.  Since you can share the webpage URL via many communication methods, this dialog would come up with choices like "Email", "IM", etc.  This is a generic message, and the previous example is a single possible scenario (so please don't assume it's for the browser or anything :) ). -->
     <string name="activity_picker_label">Choose activity</string>

res/values/styles.xml

@@ -372,4 +372,14 @@
         <item name="pageIndicatorColor">@android:color/tertiary_text_light</item>
         <item name="currentPageIndicatorColor">@android:color/primary_text_light</item>
     </style>
+
+    <style name="LanguageCheckboxAndLabel">
+        <item name="android:layout_width">match_parent</item>
+        <item name="android:layout_height">wrap_content</item>
+        <item name="android:layout_marginStart">?android:attr/listPreferredItemPaddingStart</item>
+        <item name="android:minHeight">?android:attr/listPreferredItemHeight</item>
+        <item name="android:textAppearance">?android:attr/textAppearanceListItem</item>
+        <item name="android:gravity">center_vertical</item>
+    </style>
+
 </resources>

src/com/android/settings/localepicker/LocaleDragAndDropAdapter.java

@@ -21,6 +21,7 @@ import android.support.v4.view.MotionEventCompat;
 import android.support.v7.widget.RecyclerView;
 import android.support.v7.widget.helper.ItemTouchHelper;
 import android.util.LocaleList;
+import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.MotionEvent;
 import android.view.View;
@@ -41,6 +42,7 @@ import java.util.Locale;
 class LocaleDragAndDropAdapter
         extends RecyclerView.Adapter<LocaleDragAndDropAdapter.CustomViewHolder> {
 
+    private static final String TAG = "LocaleDragAndDropAdapter";
     private final Context mContext;
     private final List<LocaleStore.LocaleInfo> mFeedItemList;
     private final ItemTouchHelper mItemTouchHelper;
@@ -55,8 +57,6 @@ class LocaleDragAndDropAdapter
             super(view);
             mLocaleDragCell = view;
             mLocaleDragCell.getDragHandle().setOnTouchListener(this);
-            mLocaleDragCell.getTextLabel().setOnTouchListener(this);
-            mLocaleDragCell.getTranslateableLabel().setOnTouchListener(this);
         }
 
         public LocaleDragCell getLocaleDragCell() {
@@ -144,7 +144,14 @@ class LocaleDragAndDropAdapter
     }
 
     private void onItemMove(int fromPosition, int toPosition) {
-        Collections.swap(mFeedItemList, fromPosition, toPosition);
+        if (fromPosition >= 0 && toPosition >= 0) {
+            Collections.swap(mFeedItemList, fromPosition, toPosition);
+        } else {
+            // TODO: It looks like sometimes the RecycleView tries to swap item -1
+            // Investigate and file a bug.
+            Log.e(TAG, String.format(Locale.US,
+                    "Negative position in onItemMove %d -> %d", fromPosition, toPosition));
+        }
         notifyItemChanged(fromPosition); // to update the numbers
         notifyItemChanged(toPosition);
         notifyItemMoved(fromPosition, toPosition);

src/com/android/settings/localepicker/LocaleDragCell.java

@@ -20,16 +20,19 @@ import android.content.Context;
 import android.util.AttributeSet;
 import android.widget.CheckBox;
 import android.widget.ImageView;
-import android.widget.LinearLayout;
+import android.widget.RelativeLayout;
 import android.widget.TextView;
 
 import com.android.settings.R;
 
-class LocaleDragCell extends LinearLayout {
+class LocaleDragCell extends RelativeLayout {
+    // We need to keep the label and the checkbox "in sync"
+    // The checkbox shows in remove mode, and the label shows in normal mode, in the same position.
+    // So we need to set the same text to both of them, and coordinate the show / hide.
     private TextView mLabel;
-    private TextView mMiniLabel;
-    private ImageView mLocalized;
     private CheckBox mCheckbox;
+    private TextView mMiniLabel;
+    private TextView mLocalized;
     private ImageView mDragHandle;
 
     public LocaleDragCell(Context context, AttributeSet attrs) {
@@ -40,20 +43,31 @@ class LocaleDragCell extends LinearLayout {
     protected void onFinishInflate() {
         super.onFinishInflate();
         mLabel = (TextView) findViewById(R.id.label);
-        mLocalized = (ImageView) findViewById(R.id.l10nWarn);
+        mLocalized = (TextView) findViewById(R.id.l10nWarn);
         mMiniLabel = (TextView) findViewById(R.id.miniLabel);
         mCheckbox = (CheckBox) findViewById(R.id.checkbox);
         mDragHandle = (ImageView) findViewById(R.id.dragHandle);
     }
 
     public void setShowHandle(boolean showHandle) {
-        mDragHandle.setVisibility(showHandle ? VISIBLE : GONE);
+        // We want invisible, not gone, so that everything else stays the same.
+        // With GONE there is more space for the labels and the text wrapping change.
+        // So the transition between "normal" mode (with numbers) and
+        // "remove mode" (with checkboxes) is not that "smooth"
+        mDragHandle.setVisibility(showHandle ? VISIBLE : INVISIBLE);
         invalidate();
         requestLayout();
     }
 
     public void setShowCheckbox(boolean showCheckbox) {
-        mCheckbox.setVisibility(showCheckbox ? VISIBLE : GONE);
+        // "Opposite" visibility for label / checkbox
+        if (showCheckbox) {
+            mCheckbox.setVisibility(VISIBLE);
+            mLabel.setVisibility(INVISIBLE);
+        } else {
+            mCheckbox.setVisibility(INVISIBLE);
+            mLabel.setVisibility(VISIBLE);
+        }
         invalidate();
         requestLayout();
     }
@@ -75,11 +89,12 @@ class LocaleDragCell extends LinearLayout {
 
     public void setLabel(String labelText) {
         mLabel.setText(labelText);
+        mCheckbox.setText(labelText);
         invalidate();
     }
 
     public void setLocalized(boolean localized) {
-        mLocalized.setVisibility(localized ? INVISIBLE : VISIBLE);
+        mLocalized.setVisibility(localized ? GONE : VISIBLE);
         invalidate();
     }
 
@@ -87,14 +102,6 @@ class LocaleDragCell extends LinearLayout {
         return mDragHandle;
     }
 
-    public ImageView getTranslateableLabel() {
-        return mLocalized;
-    }
-
-    public TextView getTextLabel() {
-        return mLabel;
-    }
-
     public CheckBox getCheckbox() {
         return mCheckbox;
     }