From bbd91e58444f092e1080f5a0a746fcd4b21ce113 Mon Sep 17 00:00:00 2001
From: Felipe Leme <felipeal@google.com>
Date: Fri, 26 Feb 2016 16:48:22 -0800
Subject: [PATCH] Make bugreport details dialog confirm to Material Guidelines
 for Dialogs.

Changes:
- Removed hints.
- Added TextViews for field labels.
- Added padding for inner dialog
- Adedd autoCorrect and capSentences to title and summary
- Changed strings.
- Set name to be selectAllOnFocus initially.

Also improved some logging statements.

BUG: 26324085
Change-Id: I32597a7c2839ca706dbbcf13660e976469ab8dd0
---
 .../res/layout/dialog_bugreport_info.xml      | 26 +++++++++++++++----
 packages/Shell/res/values/strings.xml         | 13 ++++++----
 .../shell/BugreportProgressService.java       | 11 +++++---
 .../android/shell/BugreportReceiverTest.java  |  8 +++---
 4 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/packages/Shell/res/layout/dialog_bugreport_info.xml b/packages/Shell/res/layout/dialog_bugreport_info.xml
index 5d1e9f9949469..b6b8d6bcf2b2b 100644
--- a/packages/Shell/res/layout/dialog_bugreport_info.xml
+++ b/packages/Shell/res/layout/dialog_bugreport_info.xml
@@ -16,28 +16,44 @@
 
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
     android:orientation="vertical"
+    android:paddingTop="15dp"
+    android:paddingStart="24dp"
+    android:paddingEnd="24dp"
     android:layout_width="wrap_content"
     android:layout_height="wrap_content">
+    <TextView
+        android:inputType="textNoSuggestions"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:text="@string/bugreport_info_name"/>
     <EditText
         android:id="@+id/name"
         android:maxLength="30"
         android:singleLine="true"
+        android:selectAllOnFocus="true"
         android:inputType="textNoSuggestions"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"/>
+    <TextView
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
-        android:hint="@string/bugreport_info_name"/>
+        android:text="@string/bugreport_info_title"/>
     <EditText
         android:id="@+id/title"
         android:maxLength="80"
         android:singleLine="true"
+        android:inputType="textAutoCorrect|textCapSentences"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"/>
+    <TextView
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
-        android:hint="@string/bugreport_info_title"/>
+        android:editable="false"
+        android:text="@string/bugreport_info_description"/>
     <EditText
         android:id="@+id/description"
         android:singleLine="false"
-        android:inputType="textMultiLine"
+        android:inputType="textMultiLine|textAutoCorrect|textCapSentences"
         android:layout_width="match_parent"
-        android:layout_height="wrap_content"
-        android:hint="@string/bugreport_info_description"/>
+        android:layout_height="wrap_content"/>
 </LinearLayout>
diff --git a/packages/Shell/res/values/strings.xml b/packages/Shell/res/values/strings.xml
index c26b54978e1d0..38ea8804a2d88 100644
--- a/packages/Shell/res/values/strings.xml
+++ b/packages/Shell/res/values/strings.xml
@@ -59,13 +59,16 @@
     <!--  Title of the dialog asking for user-defined bug report details like name, title, and description. -->
     <string name="bugreport_info_dialog_title">Bug report <xliff:g id="id">#%d</xliff:g> details</string>
 
-    <!-- Text of the hint asking for the bug report name, which when set will define a suffix in the
+    <!-- Text of the label identifying the bug report name, which when set will define a suffix in the
          bug report file names. [CHAR LIMIT=30] -->
     <string name="bugreport_info_name">Filename</string>
-    <!-- Text of hint asking for the bug report title, which when set will define the
+    <!-- Text of the label identifying the bug report title, which when set will define the
          Subject of the email message. [CHAR LIMIT=60] -->
-    <string name="bugreport_info_title">Title</string>
-    <!-- Text of hint asking for the bug report description, which when set will describe
+    <string name="bugreport_info_title">Bug title</string>
+    <!-- Text of the label identifying the bug report description, which when set will describe
          what the bug report is about. [CHAR LIMIT=NONE] -->
-    <string name="bugreport_info_description">Detailed description</string>
+    <string name="bugreport_info_description">Bug summary</string>
+
+    <!-- Label of button that save bugreport details.  -->
+    <string name="save">Save</string>
 </resources>
diff --git a/packages/Shell/src/com/android/shell/BugreportProgressService.java b/packages/Shell/src/com/android/shell/BugreportProgressService.java
index 36097a124ff98..0b52588957759 100644
--- a/packages/Shell/src/com/android/shell/BugreportProgressService.java
+++ b/packages/Shell/src/com/android/shell/BugreportProgressService.java
@@ -601,7 +601,8 @@ public class BugreportProgressService extends Service {
             // Most likely am killed Shell before user tapped the notification. Since system might
             // be too busy anwyays, it's better to ignore the notification and switch back to the
             // non-interactive mode (where the bugerport will be shared upon completion).
-            Log.d(TAG, "launchBugreportInfoDialog(" + id + "): cancel notification");
+            Log.w(TAG, "launchBugreportInfoDialog(): canceling notification because id " + id
+                    + " was not found");
             // TODO: add test case to make sure notification is canceled.
             NotificationManager.from(mContext).cancel(TAG, id);
             return;
@@ -627,7 +628,8 @@ public class BugreportProgressService extends Service {
             // Most likely am killed Shell before user tapped the notification. Since system might
             // be too busy anwyays, it's better to ignore the notification and switch back to the
             // non-interactive mode (where the bugerport will be shared upon completion).
-            Log.d(TAG, "takeScreenshot(" + id + ", " + delayed + "): cancel notification");
+            Log.w(TAG, "takeScreenshot(): canceling notification because id " + id
+                    + " was not found");
             // TODO: add test case to make sure notification is canceled.
             NotificationManager.from(mContext).cancel(TAG, id);
             return;
@@ -1268,6 +1270,9 @@ public class BugreportProgressService extends Service {
                         if (hasFocus) {
                             return;
                         }
+                        // Select-all is useful just initially, since the date-based filename is
+                        // full of hyphens.
+                        mInfoName.setSelectAllOnFocus(false);
                         sanitizeName();
                     }
                 });
@@ -1276,7 +1281,7 @@ public class BugreportProgressService extends Service {
                         .setView(view)
                         .setTitle(dialogTitle)
                         .setCancelable(false)
-                        .setPositiveButton(context.getString(com.android.internal.R.string.ok),
+                        .setPositiveButton(context.getString(R.string.save),
                                 null)
                         .setNegativeButton(context.getString(com.android.internal.R.string.cancel),
                                 new DialogInterface.OnClickListener()
diff --git a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
index d0499a5e39460..a629aac33d250 100644
--- a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
+++ b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
@@ -400,8 +400,8 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
 
         DetailsUi detailsUi = new DetailsUi(mUiBot, ID);
         detailsUi.assertName(NAME);
-        detailsUi.assertTitle(mContext.getString(R.string.bugreport_info_title));
-        detailsUi.assertDescription(mContext.getString(R.string.bugreport_info_description));
+        detailsUi.assertTitle("");
+        detailsUi.assertDescription("");
         detailsUi.nameField.setText(NEW_NAME);
         detailsUi.titleField.setText(TITLE);
         detailsUi.descField.setText(DESCRIPTION);
@@ -415,8 +415,8 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
 
         detailsUi = new DetailsUi(mUiBot, ID2);
         detailsUi.assertName(NAME2);
-        detailsUi.assertTitle(mContext.getString(R.string.bugreport_info_title));
-        detailsUi.assertDescription(mContext.getString(R.string.bugreport_info_description));
+        detailsUi.assertTitle("");
+        detailsUi.assertDescription("");
         detailsUi.nameField.setText(NEW_NAME2);
         detailsUi.titleField.setText(TITLE2);
         detailsUi.descField.setText(DESCRIPTION2);