From 00540bcfcc5db81abb97014d3218b8617b54d3d8 Mon Sep 17 00:00:00 2001
From: Rodrigo Damazio <Rodrigo Damazio@localhost>
Date: Mon, 6 Feb 2012 10:37:26 -0200
Subject: [PATCH] Addressing review comments for the search functionality.

---
 MyTracks/res/layout/mytracks_list_item.xml    | 49 +++++++---------
 MyTracks/res/layout/search_list.xml           | 10 ++--
 MyTracks/res/values/strings.xml               | 16 +----
 .../android/apps/mytracks/SearchActivity.java | 58 +++++++++++--------
 .../apps/mytracks/util/StringUtils.java       |  4 +-
 5 files changed, 66 insertions(+), 71 deletions(-)

diff --git a/MyTracks/res/layout/mytracks_list_item.xml b/MyTracks/res/layout/mytracks_list_item.xml
index 0bb50950b..4980291db 100644
--- a/MyTracks/res/layout/mytracks_list_item.xml
+++ b/MyTracks/res/layout/mytracks_list_item.xml
@@ -15,44 +15,39 @@ See the License for the specific language governing permissions and
 limitations under the License.
 -->
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
-  android:paddingTop="2dip"
-  android:paddingBottom="2dip"
-  android:layout_width="fill_parent"
   android:layout_height="wrap_content"
+  android:layout_width="fill_parent"
   android:minHeight="?android:attr/listPreferredItemHeight"
-  android:orientation="vertical">
+  android:orientation="vertical"
+  android:paddingBottom="2dip"
+  android:paddingTop="2dip">
 
   <LinearLayout
-    android:orientation="horizontal"
+    android:layout_height="wrap_content"
     android:layout_width="fill_parent"
-    android:layout_height="wrap_content">
+    android:orientation="horizontal">
     <ImageView android:id="@+id/track_list_item_icon"
-      android:layout_width="wrap_content"
+      android:contentDescription="@string/track_list_icon_description"
       android:layout_height="wrap_content"
+      android:layout_width="wrap_content"
       android:paddingLeft="4dip"
       android:paddingRight="2dip"
       android:paddingTop="5dip"
-      android:src="@drawable/track"
-      android:contentDescription="@string/track_list_icon_description"/>
-    <LinearLayout
-      android:orientation="horizontal"
+      android:src="@drawable/track" />
+    <TextView android:id="@+id/track_list_item_name"
+      android:layout_height="wrap_content"
+      android:layout_marginLeft="6dip"
+      android:layout_marginTop="6dip"
+      android:layout_width="wrap_content"
+      android:singleLine="true"
+      android:textAppearance="?android:attr/textAppearanceLarge"/>
+    <TextView android:id="@+id/track_list_item_category"
+      android:layout_height="wrap_content"
+      android:layout_marginLeft="6dip"
+      android:layout_marginTop="6dip"
       android:layout_width="fill_parent"
-      android:layout_height="wrap_content">
-      <TextView android:id="@+id/track_list_item_name"
-        android:layout_width="wrap_content"
-        android:layout_height="wrap_content"
-        android:layout_marginLeft="6dip"
-        android:layout_marginTop="6dip"
-        android:singleLine="true"
-        android:textAppearance="?android:attr/textAppearanceLarge"/>
-      <TextView android:id="@+id/track_list_item_category"
-        android:layout_width="fill_parent"
-        android:layout_height="wrap_content"
-        android:layout_marginLeft="6dip"
-        android:layout_marginTop="6dip"
-        android:singleLine = "true"
-        android:textAppearance="?android:attr/textAppearanceSmall"/>
-    </LinearLayout>
+      android:singleLine = "true"
+      android:textAppearance="?android:attr/textAppearanceSmall"/>
   </LinearLayout>
 
   <TextView android:id="@+id/track_list_item_time"
diff --git a/MyTracks/res/layout/search_list.xml b/MyTracks/res/layout/search_list.xml
index 706f7107b..b9322466b 100644
--- a/MyTracks/res/layout/search_list.xml
+++ b/MyTracks/res/layout/search_list.xml
@@ -20,15 +20,15 @@ limitations under the License.
   android:layout_width="fill_parent"
   android:layout_height="fill_parent">
   <ListView android:id="@android:id/list"
-    android:layout_width="fill_parent"
-    android:layout_height="fill_parent" />
-  <TextView android:id="@android:id/empty"
-    android:layout_width="fill_parent"
     android:layout_height="fill_parent"
+    android:layout_width="fill_parent" />
+  <TextView android:id="@android:id/empty"
+    android:gravity="center"
+    android:layout_height="fill_parent"
+    android:layout_width="fill_parent"
     android:text="@string/search_no_results"
     android:textColor="@android:color/white"
     android:textSize="20dip"
     android:textStyle="bold"
-    android:gravity="center"
     android:visibility="gone" />
 </LinearLayout>
\ No newline at end of file
diff --git a/MyTracks/res/values/strings.xml b/MyTracks/res/values/strings.xml
index 21cb7ae83..0f7606407 100644
--- a/MyTracks/res/values/strings.xml
+++ b/MyTracks/res/values/strings.xml
@@ -816,9 +816,7 @@ limitations under the License.
     Edit
   </string>
   <string name="marker_list_icon_description"
-    translation_description="In the Markers page, the accessibility description for the marker icon.
-
-      Screenshot: marker_list.png">
+    translation_description="In the Markers page, the accessibility description for the marker icon.">
     Marker
   </string>
   <string name="marker_list_insert_statistics"
@@ -842,12 +840,6 @@ limitations under the License.
       Screenshot: marker_list.png">
     Pick a marker. Press &amp; hold for more options.
   </string>
-  <string name="marker_list_name"
-    translation_description="In the Markers page, the label for the marker's name.
-
-      Screenshot: marker_list.png">
-    Name
-  </string>
   <string name="marker_list_show_on_map"
     translation_description="In the context menu for a marker, the menu option to show the marker
       on the map.
@@ -2344,16 +2336,14 @@ limitations under the License.
     Show on map
   </string>
   <string name="track_list_icon_description"
-    translation_description="In the 'Tracks' page, the accessibility description for the track icon.
-
-      Screenshot: track_list.png">
+    translation_description="In the 'Tracks' page, the accessibility description for the track icon.">
     Track
   </string>
   <string name="track_list_track_name"
     translation_description="In the 'Search Result' page, the text field indicating the track that a
       waypoint belongs to.
 
-      %1$d: track name. E.g., 'Grand Canyon trail'">
+      %1$s: track name. E.g., 'Grand Canyon trail'">
     In track \'%1$s\'.
   </string>
   <string name="track_name_format"
diff --git a/MyTracks/src/com/google/android/apps/mytracks/SearchActivity.java b/MyTracks/src/com/google/android/apps/mytracks/SearchActivity.java
index db8fae1f0..04b1a6403 100644
--- a/MyTracks/src/com/google/android/apps/mytracks/SearchActivity.java
+++ b/MyTracks/src/com/google/android/apps/mytracks/SearchActivity.java
@@ -60,6 +60,15 @@ import java.util.SortedSet;
  */
 public class SearchActivity extends ListActivity {
 
+  private static final String ICON_FIELD = "icon";
+  private static final String NAME_FIELD = "name";
+  private static final String DESCRIPTION_FIELD = "description";
+  private static final String CATEGORY_FIELD = "category";
+  private static final String TIME_FIELD = "time";
+  private static final String STATS_FIELD = "stats";
+  private static final String TRACK_ID_FIELD = "trackId";
+  private static final String WAYPOINT_ID_FIELD = "waypointId";
+
   private static final String EXTRA_CURRENT_TRACK_ID = "trackId";
 
   private static final boolean LOG_SCORES = true;
@@ -179,28 +188,29 @@ public class SearchActivity extends ListActivity {
     }
 
     // TODO: Yellow pushpin for statistics marker.
-    resultMap.put("icon", R.drawable.blue_pushpin);
-    resultMap.put("name", waypoint.getName());
-    resultMap.put("description", waypoint.getDescription());
-    resultMap.put("category", waypoint.getCategory());
-    resultMap.put("stats", StringUtils.formatDateTime(this, waypoint.getLocation().getTime()));
-    resultMap.put("trackId", waypoint.getTrackId());
-    resultMap.put("waypointId", waypoint.getId());
-    resultMap.put("time", getString(R.string.track_list_track_name, trackName));
+    resultMap.put(ICON_FIELD, R.drawable.blue_pushpin);
+    resultMap.put(NAME_FIELD, waypoint.getName());
+    resultMap.put(DESCRIPTION_FIELD, waypoint.getDescription());
+    resultMap.put(CATEGORY_FIELD, waypoint.getCategory());
+    // In the same place as we show time for tracks, show the track name for waypoints.
+    resultMap.put(TIME_FIELD, getString(R.string.track_list_track_name, trackName));
+    resultMap.put(STATS_FIELD, StringUtils.formatDateTime(this, waypoint.getLocation().getTime()));
+    resultMap.put(TRACK_ID_FIELD, waypoint.getTrackId());
+    resultMap.put(WAYPOINT_ID_FIELD, waypoint.getId());
   }
 
   private void prepareTrackForDisplay(Track track, Map<String, Object> resultMap) {
     TripStatistics stats = track.getStatistics();
 
-    resultMap.put("icon", R.drawable.track);
-    resultMap.put("name", track.getName());
-    resultMap.put("description", track.getDescription());
-    resultMap.put("category", track.getCategory());
-    resultMap.put("time", StringUtils.formatDateTime(this, stats.getStartTime()));
-    resultMap.put("trackId", track.getId());
-    resultMap.put("stats",
+    resultMap.put(ICON_FIELD, R.drawable.track);
+    resultMap.put(NAME_FIELD, track.getName());
+    resultMap.put(DESCRIPTION_FIELD, track.getDescription());
+    resultMap.put(CATEGORY_FIELD, track.getCategory());
+    resultMap.put(TIME_FIELD, StringUtils.formatDateTime(this, stats.getStartTime()));
+    resultMap.put(STATS_FIELD,
         StringUtils.formatTimeDistance(this, stats.getTotalDistance(), stats.getTotalTime(),
             metricUnits));
+    resultMap.put(TRACK_ID_FIELD, track.getId());
   }
 
   /**
@@ -214,12 +224,12 @@ public class SearchActivity extends ListActivity {
         // TODO: Custom view for search results.
         R.layout.mytracks_list_item,
         new String[] {
-          "icon",
-          "name",
-          "description",
-          "category",
-          "time",
-          "stats",
+          ICON_FIELD,
+          NAME_FIELD,
+          DESCRIPTION_FIELD,
+          CATEGORY_FIELD,
+          TIME_FIELD,
+          STATS_FIELD,
         },
         new int[] {
           R.id.track_list_item_icon,
@@ -243,12 +253,12 @@ public class SearchActivity extends ListActivity {
 
   private Intent createViewDataIntent(Map<String, Object> clickedData) {
     Intent intent = new Intent(Intent.ACTION_VIEW);
-    if (clickedData.containsKey("waypointId")) {
-      long waypointId = (Long) clickedData.get("waypointId");
+    if (clickedData.containsKey(WAYPOINT_ID_FIELD)) {
+      long waypointId = (Long) clickedData.get(WAYPOINT_ID_FIELD);
       Uri uri = ContentUris.withAppendedId(WaypointsColumns.CONTENT_URI, waypointId);
       intent.setDataAndType(uri, WaypointsColumns.CONTENT_ITEMTYPE);
     } else {
-      long trackId = (Long) clickedData.get("trackId");
+      long trackId = (Long) clickedData.get(TRACK_ID_FIELD);
       Uri uri = ContentUris.withAppendedId(TracksColumns.CONTENT_URI, trackId);
       intent.setDataAndType(uri, TracksColumns.CONTENT_ITEMTYPE);
     }
diff --git a/MyTracks/src/com/google/android/apps/mytracks/util/StringUtils.java b/MyTracks/src/com/google/android/apps/mytracks/util/StringUtils.java
index a78bfc1ab..6aab3f21c 100644
--- a/MyTracks/src/com/google/android/apps/mytracks/util/StringUtils.java
+++ b/MyTracks/src/com/google/android/apps/mytracks/util/StringUtils.java
@@ -89,14 +89,14 @@ public class StringUtils implements DescriptionGenerator {
   public static String formatTimeDistance(Context context, double totalDistance, long totalTime, boolean metric) {
     String distanceUnit;
     if (metric) {
-      if (totalDistance > 1000.0) {
+      if (totalDistance > 2000.0) {
         totalDistance /= 1000.0;
         distanceUnit = context.getString(R.string.unit_kilometer);
       } else {
         distanceUnit = context.getString(R.string.unit_meter);
       }
     } else {
-      if (totalDistance > UnitConversions.MI_TO_M) {
+      if (totalDistance > 2 * UnitConversions.MI_TO_M) {
         totalDistance /= UnitConversions.MI_TO_M;
         distanceUnit = context.getString(R.string.unit_mile);
       } else {