Distinguish devices with same name in chooser on Android
For devices with different names, the chooser just shows their names;
for devices with the same name, the chooser shows their name with
its unique device identifier.
So the chooser will look like:
device_different_name
device_same_name (device_1)
device_same_name (device_2)
device_same_name (device_3)
BUG=544121
Review-Url: https://codereview.chromium.org/2098983002
Cr-Commit-Position: refs/heads/master@{#403295}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java b/chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java
index 3858ec8..f1659d1 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ItemChooserDialog.java
@@ -33,7 +33,9 @@
import org.chromium.ui.base.DeviceFormFactor;
import org.chromium.ui.widget.TextViewWithClickableSpans;
+import java.util.HashMap;
import java.util.HashSet;
+import java.util.Map;
import java.util.Set;
/**
@@ -119,7 +121,7 @@
/**
* An adapter for keeping track of which items to show in the dialog.
*/
- private class ItemAdapter extends ArrayAdapter<ItemChooserRow>
+ public class ItemAdapter extends ArrayAdapter<ItemChooserRow>
implements AdapterView.OnItemClickListener {
private final LayoutInflater mInflater;
@@ -136,6 +138,9 @@
// A set of keys that are marked as disabled in the dialog.
private Set<String> mDisabledEntries = new HashSet<String>();
+ // Item descriptions are counted in a map.
+ private Map<String, Integer> mItemDescriptionMap = new HashMap<>();
+
public ItemAdapter(Context context, int resource) {
super(context, resource);
@@ -148,6 +153,29 @@
}
@Override
+ public void add(ItemChooserRow item) {
+ String description = item.mDescription;
+ int count = mItemDescriptionMap.containsKey(description)
+ ? mItemDescriptionMap.get(description) : 0;
+ mItemDescriptionMap.put(description, count + 1);
+ super.add(item);
+ }
+
+ @Override
+ public void remove(ItemChooserRow item) {
+ String description = item.mDescription;
+ if (mItemDescriptionMap.containsKey(description)) {
+ int count = mItemDescriptionMap.get(description);
+ if (count == 1) {
+ mItemDescriptionMap.remove(description);
+ } else {
+ mItemDescriptionMap.put(description, count - 1);
+ }
+ }
+ super.remove(item);
+ }
+
+ @Override
public void clear() {
mSelectedItem = ListView.INVALID_POSITION;
mConfirmButton.setEnabled(false);
@@ -165,6 +193,20 @@
}
/**
+ * Returns the text to be displayed on the chooser for an item. For items with the same
+ * description, their unique keys are appended to distinguish them.
+ * @param position The index of the item.
+ */
+ public String getDisplayText(int position) {
+ ItemChooserRow item = getItem(position);
+ String description = item.mDescription;
+ int counter = mItemDescriptionMap.get(description);
+ return counter == 1 ? description
+ : mActivity.getString(R.string.item_chooser_item_name_with_id, description,
+ item.mKey);
+ }
+
+ /**
* Sets whether the itam is enabled. Disabled items are grayed out.
* @param id The id of the item to affect.
* @param enabled Whether the item should be enabled or not.
@@ -218,8 +260,7 @@
}
}
- ItemChooserRow item = getItem(position);
- view.setText(item.mDescription);
+ view.setText(getDisplayText(position));
return view;
}
diff --git a/chrome/android/java/strings/android_chrome_strings.grd b/chrome/android/java/strings/android_chrome_strings.grd
index 0130446..381de34 100644
--- a/chrome/android/java/strings/android_chrome_strings.grd
+++ b/chrome/android/java/strings/android_chrome_strings.grd
@@ -2694,6 +2694,11 @@
<message name="IDS_TAB_SWITCHER_CALLOUT_BODY" desc="Indicates that clicking the tab switcher button gives you quick access to your tabs.">
Tap this button for quick access to your tabs.
</message>
+
+ <!-- Item Chooser UI strings -->
+ <message name="IDS_ITEM_CHOOSER_ITEM_NAME_WITH_ID" desc="To distinguish items with the same name, the item chooser shows the item name with id.">
+ <ph name="ITEM_NAME">%1$s<ex>item_name</ex></ph> (<ph name="ITEM_ID">%2$s<ex>item id</ex></ph>)
+ </message>
</messages>
</release>
</grit>
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/ItemChooserDialogTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/ItemChooserDialogTest.java
index ef2cd24..da4b28b 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/ItemChooserDialogTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/ItemChooserDialogTest.java
@@ -262,6 +262,64 @@
}
@SmallTest
+ public void testAddItemWithSameNameToListAndRemoveItemFromList() throws InterruptedException {
+ Dialog dialog = mChooserDialog.getDialogForTesting();
+ assertTrue(dialog.isShowing());
+
+ ItemChooserDialog.ItemAdapter itemAdapter = mChooserDialog.getItemAdapterForTesting();
+
+ // Add item 1.
+ ItemChooserDialog.ItemChooserRow item1 =
+ new ItemChooserDialog.ItemChooserRow("device_id_1", "same_device_name");
+ mChooserDialog.addItemToList(item1);
+ assertEquals(1, itemAdapter.getCount());
+ assertEquals(itemAdapter.getItem(0), item1);
+
+ // Add item 2.
+ ItemChooserDialog.ItemChooserRow item2 =
+ new ItemChooserDialog.ItemChooserRow("device_id_2", "different_device_name");
+ mChooserDialog.addItemToList(item2);
+ assertEquals(2, itemAdapter.getCount());
+ assertEquals(itemAdapter.getItem(0), item1);
+ assertEquals(itemAdapter.getItem(1), item2);
+
+ // Add item 3.
+ ItemChooserDialog.ItemChooserRow item3 =
+ new ItemChooserDialog.ItemChooserRow("device_id_3", "same_device_name");
+ mChooserDialog.addItemToList(item3);
+ assertEquals(3, itemAdapter.getCount());
+ assertEquals(itemAdapter.getItem(0), item1);
+ assertEquals(itemAdapter.getItem(1), item2);
+ assertEquals(itemAdapter.getItem(2), item3);
+
+ // Since two items have the same name, their display text should have their unique
+ // keys appended.
+ assertEquals("same_device_name (device_id_1)", itemAdapter.getDisplayText(0));
+ assertEquals("different_device_name", itemAdapter.getDisplayText(1));
+ assertEquals("same_device_name (device_id_3)", itemAdapter.getDisplayText(2));
+
+ // Remove item 2.
+ mChooserDialog.removeItemFromList(item2);
+ assertEquals(2, itemAdapter.getCount());
+ // Make sure the remaining items are item 1 and item 3.
+ assertEquals(itemAdapter.getItem(0), item1);
+ assertEquals(itemAdapter.getItem(1), item3);
+ assertEquals("same_device_name (device_id_1)", itemAdapter.getDisplayText(0));
+ assertEquals("same_device_name (device_id_3)", itemAdapter.getDisplayText(1));
+
+ // Remove item 1.
+ mChooserDialog.removeItemFromList(item1);
+ assertEquals(1, itemAdapter.getCount());
+ // Make sure the remaining item is item 3.
+ assertEquals(itemAdapter.getItem(0), item3);
+ // After removing item 1, item 3 is the only remaining item, so its display text
+ // also changed to its original description.
+ assertEquals("same_device_name", itemAdapter.getDisplayText(0));
+
+ mChooserDialog.dismiss();
+ }
+
+ @SmallTest
public void testListHeight() throws InterruptedException {
// 500 * .3 is 150, which is 48 * 3.125. 48 * 3.5 is 168.
assertEquals(168, ItemChooserDialog.getListHeight(500, 1.0f));
