Bug 796187 - Send tab: usability tweaks. r=nalexander
authorRichard Newman <rnewman@mozilla.com>
Fri, 18 Jan 2013 16:10:32 -0800
changeset 119320 c30b94cd9c76099cd7327080040e390cbb0717ad
parent 119319 46726c3ab4e19add734d113e443ab6f73d059b12
child 119321 9120cc3988d85ef4706c51ad78558ac6a43d2ad2
push id24195
push userMs2ger@gmail.com
push dateSat, 19 Jan 2013 16:10:11 +0000
treeherdermozilla-central@02e12a80aef9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander
bugs796187
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 796187 - Send tab: usability tweaks. r=nalexander
mobile/android/base/resources/layout/sync_list_item.xml
mobile/android/base/resources/layout/sync_send_tab.xml
mobile/android/base/sync/setup/activities/ClientRecordArrayAdapter.java
mobile/android/base/sync/setup/activities/SendTabActivity.java
--- a/mobile/android/base/resources/layout/sync_list_item.xml
+++ b/mobile/android/base/resources/layout/sync_list_item.xml
@@ -1,18 +1,19 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - License, v. 2.0. If a copy of the MPL was not distributed with this
    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
 
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
-    android:orientation="horizontal"
+    android:gravity="center_vertical"
     android:layout_width="fill_parent"
     android:layout_height="wrap_content"
-    android:gravity="center_vertical"
+    android:minHeight="?android:attr/listPreferredItemHeight"
+    android:orientation="horizontal"
     android:padding="5dp" >
 
     <ImageView
         android:id="@+id/img"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:padding="10dp"
         android:layout_marginRight="10dip" />
--- a/mobile/android/base/resources/layout/sync_send_tab.xml
+++ b/mobile/android/base/resources/layout/sync_send_tab.xml
@@ -5,16 +5,36 @@
 
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
   style="@style/SyncContainer" >
 
   <TextView
       style="@style/SyncTop"
       android:text="@string/sync_title_send_tab" />
 
+  <LinearLayout
+    android:layout_width="fill_parent"
+    android:layout_height="wrap_content"
+    android:orientation="vertical"
+    android:padding="@dimen/SyncSpace" >
+
+    <TextView
+      android:id="@+id/title"
+      android:layout_width="fill_parent"
+      android:layout_height="wrap_content"
+      android:textAppearance="?android:attr/textAppearanceMedium"
+      android:text="@string/sync_title_send_tab" />
+    <TextView
+      android:id="@+id/uri"
+      android:layout_width="fill_parent"
+      android:layout_height="wrap_content"
+      android:textAppearance="?android:attr/textAppearanceSmall"
+      android:text="@string/sync_title_send_tab" />
+  </LinearLayout>
+
   <ListView
     android:id="@+id/device_list"
     style="@style/SyncMiddle"
     android:padding="0dp" >
   </ListView>
 
   <LinearLayout
     style="@style/SyncBottom" >
--- a/mobile/android/base/sync/setup/activities/ClientRecordArrayAdapter.java
+++ b/mobile/android/base/sync/setup/activities/ClientRecordArrayAdapter.java
@@ -1,97 +1,139 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
 package org.mozilla.gecko.sync.setup.activities;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.sync.repositories.domain.ClientRecord;
 
 import android.content.Context;
 import android.view.View;
 import android.view.View.OnClickListener;
 import android.view.ViewGroup;
 import android.widget.ArrayAdapter;
 import android.widget.CheckBox;
 import android.widget.ImageView;
 import android.widget.TextView;
 
-public class ClientRecordArrayAdapter extends ArrayAdapter<Object> {
+public class ClientRecordArrayAdapter extends ArrayAdapter<ClientRecord> {
   public static final String LOG_TAG = "ClientRecArrayAdapter";
-  private ClientRecord[] clientRecordList;
+
   private boolean[] checkedItems;
   private int numCheckedGUIDs;
   private SendTabActivity sendTabActivity;
 
-  public ClientRecordArrayAdapter(Context context, int textViewResourceId,
-      ClientRecord[] clientRecordList) {
-    super(context, textViewResourceId, clientRecordList);
+  public ClientRecordArrayAdapter(Context context,
+                                  int textViewResourceId) {
+    super(context, textViewResourceId, new ArrayList<ClientRecord>());
+    this.checkedItems = new boolean[0];
     this.sendTabActivity = (SendTabActivity) context;
-    this.clientRecordList = clientRecordList;
-    this.checkedItems = new boolean[clientRecordList.length];
+  }
+
+  public synchronized void setClientRecordList(final Collection<ClientRecord> clientRecordList) {
+    this.clear();
+    this.checkedItems = new boolean[clientRecordList.size()];
+    this.numCheckedGUIDs = 0;
+    for (ClientRecord clientRecord : clientRecordList) {
+      this.add(clientRecord);
+    }
+    this.notifyDataSetChanged();
+  }
+
+  /**
+   * If we have only a single client record in the list, mark it as checked.
+   */
+  public synchronized void checkItem(final int position, boolean checked) throws ArrayIndexOutOfBoundsException {
+    if (position < 0 ||
+        position >= checkedItems.length) {
+      throw new ArrayIndexOutOfBoundsException(position);
+    }
+
+    if (setRowChecked(position, true)) {
+      this.notifyDataSetChanged();
+    }
+  }
+
+  /**
+   * Set the specified row to the specified checked state.
+   * @param position an index.
+   * @param checked whether the checkbox should be checked.
+   * @return <code>true</code> if the state changed, <code>false</code> if the
+   *         box was already in the requested state.
+   */
+  protected synchronized boolean setRowChecked(int position, boolean checked) {
+    boolean current = checkedItems[position];
+    if (current == checked) {
+      return false;
+    }
+
+    checkedItems[position] = checked;
+    numCheckedGUIDs += checked ? 1 : -1;
+    if (numCheckedGUIDs <= 0) {
+      sendTabActivity.enableSend(numCheckedGUIDs > 0);
+    }
+    return true;
   }
 
   @Override
   public View getView(final int position, View convertView, ViewGroup parent) {
     final Context context = this.getContext();
 
     // Reuse View objects if they exist.
     View row = convertView;
     if (row == null) {
       row = View.inflate(context, R.layout.sync_list_item, null);
       setSelectable(row, true);
       row.setBackgroundResource(android.R.drawable.menuitem_background);
     }
 
-    final ClientRecord clientRecord = clientRecordList[position];
+    final ClientRecord clientRecord = this.getItem(position);
     ImageView clientType = (ImageView) row.findViewById(R.id.img);
     TextView clientName = (TextView) row.findViewById(R.id.client_name);
+
     // Set up checkbox and restore stored state.
     CheckBox checkbox = (CheckBox) row.findViewById(R.id.check);
     checkbox.setChecked(checkedItems[position]);
     setSelectable(checkbox, false);
 
     clientName.setText(clientRecord.name);
     clientType.setImageResource(getImage(clientRecord));
 
     row.setOnClickListener(new OnClickListener() {
       @Override
       public void onClick(View view) {
-        CheckBox item = (CheckBox) view.findViewById(R.id.check);
-
-        // Update the checked item, both in the UI and in the checkedItems array.
-        boolean newCheckedValue = !item.isChecked();
-        item.setChecked(newCheckedValue);
-        checkedItems[position] = newCheckedValue;
+        final CheckBox item = (CheckBox) view.findViewById(R.id.check);
 
-        numCheckedGUIDs += newCheckedValue ? 1 : -1;
-        if (numCheckedGUIDs <= 0) {
-          sendTabActivity.enableSend(false);
-          return;
-        }
-        sendTabActivity.enableSend(true);
+        // Update the checked item, both in the UI and in our internal state.
+        final boolean checked = !item.isChecked();    // Because it hasn't happened yet.
+        item.setChecked(checked);
+        setRowChecked(position, checked);
       }
-
     });
 
     return row;
   }
 
-  public List<String> getCheckedGUIDs() {
+  public synchronized List<String> getCheckedGUIDs() {
     final List<String> guids = new ArrayList<String>();
     for (int i = 0; i < checkedItems.length; i++) {
       if (checkedItems[i]) {
-        guids.add(clientRecordList[i].guid);
+        guids.add(this.getItem(i).guid);
       }
     }
     return guids;
   }
 
-  public int getNumCheckedGUIDs() {
+  public synchronized int getNumCheckedGUIDs() {
     return numCheckedGUIDs;
   }
 
   private int getImage(ClientRecord record) {
     if ("mobile".equals(record.type)) {
       return R.drawable.mobile;
     }
     return R.drawable.desktop;
--- a/mobile/android/base/sync/setup/activities/SendTabActivity.java
+++ b/mobile/android/base/sync/setup/activities/SendTabActivity.java
@@ -1,23 +1,27 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.sync.setup.activities;
 
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
 
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.sync.CommandProcessor;
 import org.mozilla.gecko.sync.CommandRunner;
-import org.mozilla.gecko.sync.SyncConstants;
 import org.mozilla.gecko.sync.GlobalSession;
 import org.mozilla.gecko.sync.Logger;
 import org.mozilla.gecko.sync.SyncConfiguration;
+import org.mozilla.gecko.sync.SyncConstants;
 import org.mozilla.gecko.sync.repositories.NullCursorException;
 import org.mozilla.gecko.sync.repositories.android.ClientsDatabaseAccessor;
 import org.mozilla.gecko.sync.repositories.domain.ClientRecord;
 import org.mozilla.gecko.sync.setup.Constants;
 import org.mozilla.gecko.sync.setup.SyncAccounts;
 import org.mozilla.gecko.sync.stage.SyncClientsEngineStage;
 import org.mozilla.gecko.sync.syncadapter.SyncAdapter;
 
@@ -26,27 +30,98 @@ import android.accounts.AccountManager;
 import android.app.Activity;
 import android.content.Context;
 import android.content.Intent;
 import android.content.SharedPreferences;
 import android.os.AsyncTask;
 import android.os.Bundle;
 import android.view.View;
 import android.widget.ListView;
+import android.widget.TextView;
 import android.widget.Toast;
 
 public class SendTabActivity extends Activity {
   public static final String LOG_TAG = "SendTabActivity";
   private ClientRecordArrayAdapter arrayAdapter;
   private AccountManager accountManager;
   private Account localAccount;
+  private SendTabData sendTabData;
 
   @Override
   public void onCreate(Bundle savedInstanceState) {
     super.onCreate(savedInstanceState);
+
+    Intent intent = getIntent();
+    if (intent == null) {
+      Logger.warn(LOG_TAG, "intent was null; aborting without sending tab.");
+      notifyAndFinish(false);
+      return;
+    }
+
+    Bundle extras = intent.getExtras();
+    if (extras == null) {
+      Logger.warn(LOG_TAG, "extras was null; aborting without sending tab.");
+      notifyAndFinish(false);
+      return;
+    }
+
+    sendTabData = SendTabData.fromBundle(extras);
+    if (sendTabData == null) {
+      Logger.warn(LOG_TAG, "send tab data was null; aborting without sending tab.");
+      notifyAndFinish(false);
+      return;
+    }
+
+    if (sendTabData.uri == null) {
+      Logger.warn(LOG_TAG, "uri was null; aborting without sending tab.");
+      notifyAndFinish(false);
+      return;
+    }
+
+    if (sendTabData.title == null) {
+      Logger.warn(LOG_TAG, "title was null; ignoring and sending tab anyway.");
+    }
+  }
+
+  /**
+   * Ensure that the view's list of clients is backed by a recently populated
+   * array adapter. But only once, so we don't end up blowing away your selections
+   * just because you got a text message.
+   */
+  protected synchronized void ensureClientList(final Context context,
+                                               final ListView listview) {
+    if (arrayAdapter != null) {
+      Logger.debug(LOG_TAG, "Already have an array adapter for client lists.");
+      listview.setAdapter(arrayAdapter);
+      return;
+    }
+
+    arrayAdapter = new ClientRecordArrayAdapter(context, R.layout.sync_list_item);
+    listview.setAdapter(arrayAdapter);
+
+    // Fetching the client list hits the clients database, so we spin this onto
+    // a background task.
+    new AsyncTask<Void, Void, Collection<ClientRecord>>() {
+
+      @Override
+      protected Collection<ClientRecord> doInBackground(Void... params) {
+        return getOtherClients();
+      }
+
+      @Override
+      protected void onPostExecute(final Collection<ClientRecord> clientArray) {
+        // We're allowed to update the UI from here.
+
+        Logger.debug(LOG_TAG, "Got " + clientArray.size() + " clients.");
+        arrayAdapter.setClientRecordList(clientArray);
+        if (clientArray.size() == 1) {
+          arrayAdapter.checkItem(0, true);
+        }
+      }
+    }.execute();
   }
 
   @Override
   public void onResume() {
     ActivityUtils.prepareLogging();
     Logger.info(LOG_TAG, "Called SendTabActivity.onResume.");
     super.onResume();
 
@@ -55,33 +130,23 @@ public class SendTabActivity extends Act
 
     setContentView(R.layout.sync_send_tab);
     final ListView listview = (ListView) findViewById(R.id.device_list);
     listview.setItemsCanFocus(true);
     listview.setTextFilterEnabled(true);
     listview.setChoiceMode(ListView.CHOICE_MODE_MULTIPLE);
     enableSend(false);
 
-    // Fetching the client list hits the clients database, so we spin this onto
-    // a background task.
-    final Context context = this;
-    new AsyncTask<Void, Void, ClientRecord[]>() {
+    ensureClientList(this, listview);
 
-      @Override
-      protected ClientRecord[] doInBackground(Void... params) {
-        return getClientArray();
-      }
+    TextView textView = (TextView) findViewById(R.id.title);
+    textView.setText(sendTabData.title);
 
-      @Override
-      protected void onPostExecute(final ClientRecord[] clientArray) {
-        // We're allowed to update the UI from here.
-        arrayAdapter = new ClientRecordArrayAdapter(context, R.layout.sync_list_item, clientArray);
-        listview.setAdapter(arrayAdapter);
-      }
-    }.execute();
+    textView = (TextView) findViewById(R.id.uri);
+    textView.setText(sendTabData.uri);
   }
 
   private static void registerDisplayURICommand() {
     final CommandProcessor processor = CommandProcessor.getProcessor();
     processor.registerCommand("displayURI", new CommandRunner(3) {
       @Override
       public void executeCommand(final GlobalSession session, List<String> args) {
         CommandProcessor.displayURI(args, session.getContext());
@@ -121,35 +186,16 @@ public class SendTabActivity extends Act
     } catch (Exception e) {
       Logger.warn(LOG_TAG, "Could not get Sync account parameters or preferences; aborting.");
       return null;
     }
   }
 
   public void sendClickHandler(View view) {
     Logger.info(LOG_TAG, "Send was clicked.");
-    Bundle extras = this.getIntent().getExtras();
-    if (extras == null) {
-      Logger.warn(LOG_TAG, "extras was null; aborting without sending tab.");
-      notifyAndFinish(false);
-      return;
-    }
-
-    final SendTabData sendTabData = SendTabData.fromBundle(extras);
-
-    if (sendTabData.title == null) {
-      Logger.warn(LOG_TAG, "title was null; ignoring and sending tab anyway.");
-    }
-
-    if (sendTabData.uri == null) {
-      Logger.warn(LOG_TAG, "uri was null; aborting without sending tab.");
-      notifyAndFinish(false);
-      return;
-    }
-
     final List<String> remoteClientGuids = arrayAdapter.getCheckedGUIDs();
 
     if (remoteClientGuids == null) {
       // Should never happen.
       Logger.warn(LOG_TAG, "guids was null; aborting without sending tab.");
       notifyAndFinish(false);
       return;
     }
@@ -209,21 +255,36 @@ public class SendTabActivity extends Act
   }
 
   public void enableSend(boolean shouldEnable) {
     View sendButton = findViewById(R.id.send_button);
     sendButton.setEnabled(shouldEnable);
     sendButton.setClickable(shouldEnable);
   }
 
-  protected ClientRecord[] getClientArray() {
+  protected Map<String, ClientRecord> getClients() {
     ClientsDatabaseAccessor db = new ClientsDatabaseAccessor(this.getApplicationContext());
-
     try {
-      return db.fetchAllClients().values().toArray(new ClientRecord[0]);
+      return db.fetchAllClients();
     } catch (NullCursorException e) {
       Logger.warn(LOG_TAG, "NullCursorException while populating device list.", e);
       return null;
     } finally {
       db.close();
     }
   }
+
+  /**
+   * @return a collection of client records, excluding our own.
+   */
+  protected Collection<ClientRecord> getOtherClients() {
+    final Map<String, ClientRecord> all = getClients();
+    final ArrayList<ClientRecord> out = new ArrayList<ClientRecord>(all.size());
+    final String ourGUID = getAccountGUID();
+    for (Entry<String, ClientRecord> entry : all.entrySet()) {
+      if (ourGUID.equals(entry.getKey())) {
+        continue;
+      }
+      out.add(entry.getValue());
+    }
+    return out;
+  }
 }