code review comments

MyTracks/res/layout/send_to_google_result.xml

@@ -20,46 +20,46 @@ It is used to summarize the results of uploading track data to one or more
 services.
 -->
 <LinearLayout
-  xmlns:android="http://schemas.android.com/apk/res/android"
-  android:layout_width="fill_parent"
-  android:layout_height="wrap_content"
-  android:paddingTop="5dip"
-  android:paddingBottom="5dip"
-  android:paddingLeft="14dip"
-  android:paddingRight="14dip"
-  android:orientation="vertical">
-  <TextView
+    xmlns:android="http://schemas.android.com/apk/res/android"
     android:layout_width="fill_parent"
     android:layout_height="wrap_content"
-    android:text="@string/status_track_upload_status"
-    style="?android:attr/textAppearanceLarge"/>
+    android:paddingTop="5dip"
+    android:paddingBottom="5dip"
+    android:paddingLeft="14dip"
+    android:paddingRight="14dip"
+    android:orientation="vertical">
+  <TextView
+      android:layout_width="fill_parent"
+      android:layout_height="wrap_content"
+      android:text="@string/status_track_upload_status"
+      style="?android:attr/textAppearanceLarge"/>
   <TableLayout
-    android:layout_width="wrap_content"
-    android:layout_height="wrap_content"
-    android:gravity="center_horizontal">
-    <!-- We don't want a divider, so we set its color to transparent. -->
-    <!-- We do this explicitly, rather than with a color resource, because -->
-    <!-- an apparent bug causes the divider to be wildly misrendered on -->
-    <!-- a Mini X10 running 2.1. -->
-  	<ListView
-  	  android:id="@+id/send_to_google_result_list"
-      android:clickable="false"
-      android:paddingTop="10dip"
-      android:paddingBottom="10dip"
-      android:divider="#00000000"/>
+      android:layout_width="wrap_content"
+      android:layout_height="wrap_content"
+      android:gravity="center_horizontal">
+      <!-- We don't want a divider, so we set its color to transparent. -->
+      <!-- We do this explicitly, rather than with a color resource, because -->
+      <!-- an apparent bug causes the divider to be wildly misrendered on -->
+      <!-- a Mini X10 running 2.1. -->
+   	<ListView
+        android:id="@+id/send_to_google_result_list"
+        android:clickable="false"
+        android:paddingTop="10dip"
+        android:paddingBottom="10dip"
+        android:divider="#00000000"/>
   </TableLayout>
   <TextView
-    android:id="@+id/send_to_google_result_comment"
-    android:layout_width="fill_parent"
-    android:layout_height="wrap_content"
-    android:text="@string/status_mymap_info"
-    android:visibility="gone"
-    style="?android:attr/textAppearanceSmall"/>
+      android:id="@+id/send_to_google_result_comment"
+      android:layout_width="fill_parent"
+      android:layout_height="wrap_content"
+      android:text="@string/status_mymap_info"
+      android:visibility="gone"
+      style="?android:attr/textAppearanceSmall"/>
   <TextView
-    android:id="@+id/send_to_google_result_error"
-    android:layout_width="fill_parent"
-    android:layout_height="wrap_content"
-    android:text="@string/error_please_try_again_later"
-    android:visibility="gone"
-    style="?android:attr/textAppearanceLarge"/>
+      android:id="@+id/send_to_google_result_error"
+      android:layout_width="fill_parent"
+      android:layout_height="wrap_content"
+      android:text="@string/error_please_try_again_later"
+      android:visibility="gone"
+      style="?android:attr/textAppearanceLarge"/>
 </LinearLayout>

MyTracks/res/layout/send_to_google_result_list_item.xml

@@ -20,30 +20,30 @@ It will be instantiated by a custom ListAdapter - once for each service to
 which track data was uploaded.
 -->
 <TableRow
-  xmlns:android="http://schemas.android.com/apk/res/android"
-  android:layout_width="wrap_content"
-  android:layout_height="wrap_content">
-  <ImageView
-  	android:id="@+id/send_to_google_result_icon"
-  	android:src="@drawable/success"
-  	android:layout_width="wrap_content"
-  	android:layout_height="fill_parent"
-  	android:layout_gravity="top"/>
-  <LinearLayout
+    xmlns:android="http://schemas.android.com/apk/res/android"
     android:layout_width="wrap_content"
-    android:layout_height="wrap_content"
-    android:orientation="vertical">
-    <TextView
-      android:id="@+id/send_to_google_result_name"
+    android:layout_height="wrap_content">
+  <ImageView
+      android:id="@+id/send_to_google_result_icon"
+      android:src="@drawable/success"
+      android:layout_width="wrap_content"
+      android:layout_height="fill_parent"
+      android:layout_gravity="top"/>
+  <LinearLayout
       android:layout_width="wrap_content"
       android:layout_height="wrap_content"
-      android:text="Google Docs"
-      style="?android:attr/textAppearanceMedium"/>
+      android:orientation="vertical">
     <TextView
-      android:id="@+id/send_to_google_result_url"
-      android:layout_width="wrap_content"
-      android:layout_height="wrap_content"
-      android:text="http://google.com"
-      style="?android:attr/textAppearanceSmall"/>
+        android:id="@+id/send_to_google_result_name"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:text="Google Docs"
+        style="?android:attr/textAppearanceMedium"/>
+    <TextView
+        android:id="@+id/send_to_google_result_url"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:text="http://google.com"
+        style="?android:attr/textAppearanceSmall"/>
   </LinearLayout>
 </TableRow>

MyTracks/res/values/do_not_translate.xml

@@ -34,4 +34,9 @@ limitations under the License.
   <string name="ant_sensor_type">ANT</string>
   <string name="srm_ant_bridge_sensor_type">SRM_ANT_BRIDGE</string>
   <string name="zephyr_sensor_type">ZEPHYR</string>
+
+  <!-- Send to Google URLs -->
+  <string name="sendtogoogle_google_fusiontables_url">google.com/fusiontables</string>
+  <string name="sendtogoogle_google_mymaps_url">maps.google.com</string>
+  <string name="sendtogoogle_google_docs_url">docs.google.com</string>
 </resources>

MyTracks/res/values/strings.xml

@@ -278,7 +278,7 @@ limitations under the License.
   <string name="maps_list_failed">Your maps could not be loaded at this time!</string>
   <string name="open_map">Select this Map</string>
   <string name="edit_map_details">Edit Map Details</string>
-  <string name="share_track">Share Track with Friends</string>
+  <string name="share_track">Share Track</string>
   <string name="delete_map">Delete Map from your Account</string>
   <string name="map_title">Map Title:</string>
   <string name="map_description">Map Description:</string>
@@ -297,7 +297,6 @@ limitations under the License.
   <string name="send_track_body_format">I think you might be interested in this track.
 
     To view this track use Google Earth or other mapping software.</string>
-  <string name="share_track">Share track with friends</string>
 
   <!-- Visibility -->
   <string name="public_map">Public</string>
@@ -410,11 +409,8 @@ limitations under the License.
   <!-- Send to Google -->
   <string name="sendtogoogle_title">Send to...</string>
   <string name="sendtogoogle_google_fusiontables">Google Fusion Tables</string>
-  <string name="sendtogoogle_google_fusiontables_url">google.com/fusiontables</string>
   <string name="sendtogoogle_google_mymaps">Google My Maps</string>
-  <string name="sendtogoogle_google_mymaps_url">maps.google.com</string>
   <string name="sendtogoogle_google_docs">Google Docs</string>
-  <string name="sendtogoogle_google_docs_url">docs.google.com</string>
   <string name="sendtogoogle_send_now">Send now</string>
   <string name="sendtogoogle_create_new_map">Create a new map</string>
   <string name="sendtogoogle_pick_existing_map">Pick an existing map</string>

MyTracks/src/com/google/android/apps/mytracks/MyTracks.java

@@ -1180,6 +1180,24 @@ public class MyTracks extends TabActivity implements OnTouchListener,
     final boolean sentToFusionTables = sendToGoogleDialog.getSendToFusionTables();
     dialogManager.dismissDialogSafely(DIALOG_PROGRESS);
 
+    // We've finished sending the track to the user-selected services.  Now
+    // we tell them the results of the upload, and optionally share the track.
+    // There are a few different paths through this code:
+    //
+    // 1. The user pre-requested a share (shareRequested == true).  We're going
+    //    to display the result dialog *without* the share button (the share
+    //    listener will be null).  The OK button listener will initiate the
+    //    share.
+    //
+    // 2. The user did not pre-request a share, but the set of services to
+    //    which we succeeded in uploading the track are compatible with
+    //    sharing.  We'll display a share button (the share listener will be
+    //    non-null), and will share the link if the user clicks it.
+    //
+    // 3. The user did not pre-request a share, and the set of services to
+    //    which we succeeded in uploading the track are incompatible with
+    //    sharing.  We won't display a share button.
+
     final boolean canShare = getSendToFusionTablesTableId() != null
         || getSendToMyMapsMapId() != null;
 
@@ -1195,8 +1213,6 @@ public class MyTracks extends TabActivity implements OnTouchListener,
 
     DialogInterface.OnClickListener onOkListener = (canShare && shareRequested)
         ? doShareListener : null;
-
-    // If the share listener is null, the share button will not be displayed.
     DialogInterface.OnClickListener onShareListener = (canShare && !shareRequested)
         ? doShareListener : null;
 

MyTracks/src/com/google/android/apps/mytracks/io/sendtogoogle/ResultDialogFactory.java

@@ -19,6 +19,7 @@ import com.google.android.maps.mytracks.R;
 
 import android.app.Activity;
 import android.app.AlertDialog;
+import android.app.Dialog;
 import android.content.DialogInterface;
 import android.view.View;
 import android.widget.ListView;
@@ -89,7 +90,7 @@ public class ResultDialogFactory {
   private static View makeDialogContent(Activity activity, List<SendResult> results,
       boolean success) {
     ResultListAdapter resultListAdapter = new ResultListAdapter(activity,
-        R.layout.send_to_google_result_fragment, results);
+        R.layout.send_to_google_result_list_item, results);
 
     View content = activity.getLayoutInflater().inflate(R.layout.send_to_google_result, null);
     ListView resultList = (ListView) content.findViewById(R.id.send_to_google_result_list);

MyTracks/src/com/google/android/apps/mytracks/io/sendtogoogle/ResultListAdapter.java

@@ -23,6 +23,7 @@ import android.view.View;
 import android.view.ViewGroup;
 import android.widget.ArrayAdapter;
 import android.widget.ImageView;
+import android.widget.ListAdapter;
 import android.widget.TextView;
 
 import java.util.List;
@@ -44,7 +45,7 @@ class ResultListAdapter extends ArrayAdapter<SendResult> {
     if (view == null) {
       LayoutInflater inflater
           = (LayoutInflater) getContext().getSystemService(Context.LAYOUT_INFLATER_SERVICE);
-      view = inflater.inflate(R.layout.send_to_google_result_fragment, null);
+      view = inflater.inflate(R.layout.send_to_google_result_list_item, null);
     }
 
     SendResult result = getItem(position);

MyTracksTest/src/com/google/android/apps/mytracks/io/sendtogoogle/ResultDialogFactoryTest.java

@@ -1,3 +1,18 @@
+/*
+ * Copyright 2011 Google Inc.
+ *
+ * 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.google.android.apps.mytracks.io.sendtogoogle;
 
 import com.google.android.apps.mytracks.MyTracks;
@@ -13,6 +28,11 @@ import android.widget.ListView;
 import java.util.ArrayList;
 import java.util.List;
 
+/**
+ * Tests {@link ResultDialogFactory}.
+ *
+ * @author Matthew Simmons
+ */
 public class ResultDialogFactoryTest extends ActivityInstrumentationTestCase2<MyTracks> {
   public ResultDialogFactoryTest() {
     super(MyTracks.class);

MyTracksTest/src/com/google/android/apps/mytracks/io/sendtogoogle/ResultListAdapterTest.java

@@ -1,3 +1,18 @@
+/*
+ * Copyright 2011 Google Inc.
+ *
+ * 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.google.android.apps.mytracks.io.sendtogoogle;
 
 import com.google.android.maps.mytracks.R;
@@ -10,6 +25,11 @@ import android.view.View;
 import java.util.ArrayList;
 import java.util.List;
 
+/**
+ * Tests {@link ResultListAdapter}.
+ *
+ * @author Matthew Simmons
+ */
 public class ResultListAdapterTest extends AndroidTestCase {
   private static class TestResultListAdapter extends ResultListAdapter {
     public View contentView;
@@ -73,7 +93,7 @@ public class ResultListAdapterTest extends AndroidTestCase {
   }
 
   public void testSuccess_reusesView() {
-    View contentView = inflateContentView(R.layout.send_to_google_result_fragment);
+    View contentView = inflateContentView(R.layout.send_to_google_result_list_item);
     TestResultListAdapter adapter = new TestResultListAdapter(getContext(),
         makeResult(sendType, true));
     adapter.getView(0, contentView, null);