Always show WAIT/PAUSE in dialer overflow menu.
Used to hide menu items when not applicable. Now we show them always
and only perform the action in those cases where menu items were
previously visible.
Updated code to do zero-position checks when there is no
selection...previously missing check.
Changed code to use chars instead of Strings when dealing with single
digits.
Consolidated duplicate code with updateDialString() function.
bug: 7478840
Change-Id: I2aa5d3badd40079e9aa75abf7e4051f9dba5e667
diff --git a/Android.mk b/Android.mk
index 97dfd82..ea13c65 100644
--- a/Android.mk
+++ b/Android.mk
@@ -34,5 +34,5 @@
include $(BUILD_PACKAGE)
-# Use the folloing include to make our test apk.
+# Use the following include to make our test apk.
include $(call all-makefiles-under,$(LOCAL_PATH))
diff --git a/src/com/android/dialer/dialpad/DialpadFragment.java b/src/com/android/dialer/dialpad/DialpadFragment.java
index f70a279..123d700 100644
--- a/src/com/android/dialer/dialpad/DialpadFragment.java
+++ b/src/com/android/dialer/dialpad/DialpadFragment.java
@@ -85,6 +85,7 @@
import com.android.internal.telephony.ITelephony;
import com.android.phone.common.CallLogAsync;
import com.android.phone.common.HapticFeedback;
+import com.google.common.annotations.VisibleForTesting;
import java.util.List;
@@ -103,6 +104,8 @@
private static final boolean DEBUG = DialtactsActivity.DEBUG;
private static final String EMPTY_NUMBER = "";
+ private static final char PAUSE = ',';
+ private static final char WAIT = ';';
/** The length of DTMF tones in milliseconds */
private static final int TONE_LENGTH_MS = 150;
@@ -692,8 +695,6 @@
private void setupMenuItems(Menu menu) {
final MenuItem callSettingsMenuItem = menu.findItem(R.id.menu_call_settings_dialpad);
final MenuItem addToContactMenuItem = menu.findItem(R.id.menu_add_contacts);
- final MenuItem twoSecPauseMenuItem = menu.findItem(R.id.menu_2s_pause);
- final MenuItem waitMenuItem = menu.findItem(R.id.menu_add_wait);
// Check if all the menu items are inflated correctly. As a shortcut, we assume all menu
// items are ready if the first item is non-null.
@@ -710,54 +711,17 @@
callSettingsMenuItem.setIntent(DialtactsActivity.getCallSettingsIntent());
}
- // We show "add to contacts", "2sec pause", and "add wait" menus only when the user is
- // seeing usual dialpads and has typed at least one digit.
+ // We show "add to contacts" menu only when the user is
+ // seeing usual dialpad and has typed at least one digit.
// We never show a menu if the "choose dialpad" UI is up.
if (dialpadChooserVisible() || isDigitsEmpty()) {
addToContactMenuItem.setVisible(false);
- twoSecPauseMenuItem.setVisible(false);
- waitMenuItem.setVisible(false);
} else {
final CharSequence digits = mDigits.getText();
// Put the current digits string into an intent
addToContactMenuItem.setIntent(getAddToContactIntent(digits));
addToContactMenuItem.setVisible(true);
-
- // Check out whether to show Pause & Wait option menu items
- int selectionStart;
- int selectionEnd;
- String strDigits = digits.toString();
-
- selectionStart = mDigits.getSelectionStart();
- selectionEnd = mDigits.getSelectionEnd();
-
- if (selectionStart != -1) {
- if (selectionStart > selectionEnd) {
- // swap it as we want start to be less then end
- int tmp = selectionStart;
- selectionStart = selectionEnd;
- selectionEnd = tmp;
- }
-
- if (selectionStart != 0) {
- // Pause can be visible if cursor is not in the begining
- twoSecPauseMenuItem.setVisible(true);
-
- // For Wait to be visible set of condition to meet
- waitMenuItem.setVisible(showWait(selectionStart, selectionEnd, strDigits));
- } else {
- // cursor in the beginning both pause and wait to be invisible
- twoSecPauseMenuItem.setVisible(false);
- waitMenuItem.setVisible(false);
- }
- } else {
- twoSecPauseMenuItem.setVisible(true);
-
- // cursor is not selected so assume new digit is added to the end
- int strLength = strDigits.length();
- waitMenuItem.setVisible(showWait(strLength, strLength, strDigits));
- }
}
}
@@ -1528,10 +1492,10 @@
public boolean onOptionsItemSelected(MenuItem item) {
switch (item.getItemId()) {
case R.id.menu_2s_pause:
- updateDialString(",");
+ updateDialString(PAUSE);
return true;
case R.id.menu_add_wait:
- updateDialString(";");
+ updateDialString(WAIT);
return true;
default:
return false;
@@ -1547,7 +1511,12 @@
* Updates the dial string (mDigits) after inserting a Pause character (,)
* or Wait character (;).
*/
- private void updateDialString(String newDigits) {
+ private void updateDialString(char newDigit) {
+ if(newDigit != WAIT && newDigit != PAUSE) {
+ Log.wtf(TAG, "Not expected for anything other than PAUSE & WAIT");
+ return;
+ }
+
int selectionStart;
int selectionEnd;
@@ -1558,20 +1527,19 @@
selectionStart = Math.min(anchor, point);
selectionEnd = Math.max(anchor, point);
+ if (selectionStart == -1) {
+ selectionStart = selectionEnd = mDigits.length();
+ }
+
Editable digits = mDigits.getText();
- if (selectionStart != -1) {
- if (selectionStart == selectionEnd) {
- // then there is no selection. So insert the pause at this
- // position and update the mDigits.
- digits.replace(selectionStart, selectionStart, newDigits);
- } else {
- digits.replace(selectionStart, selectionEnd, newDigits);
- // Unselect: back to a regular cursor, just pass the character inserted.
- mDigits.setSelection(selectionStart + 1);
+
+ if (canAddDigit(digits, selectionStart, selectionEnd, newDigit)) {
+ digits.replace(selectionStart, selectionEnd, Character.toString(newDigit));
+
+ if (selectionStart != selectionEnd) {
+ // Unselect: back to a regular cursor, just pass the character inserted.
+ mDigits.setSelection(selectionStart + 1);
}
- } else {
- int len = mDigits.length();
- digits.replace(len, len, newDigits);
}
}
@@ -1617,28 +1585,38 @@
}
/**
- * This function return true if Wait menu item can be shown
- * otherwise returns false. Assumes the passed string is non-empty
- * and the 0th index check is not required.
+ * Returns true of the newDigit parameter can be added at the current selection
+ * point, otherwise returns false.
+ * Only prevents input of WAIT and PAUSE digits at an unsupported position.
+ * Fails early if start == -1 or start is larger than end.
*/
- private static boolean showWait(int start, int end, String digits) {
- if (start == end) {
- // visible false in this case
- if (start > digits.length()) return false;
-
- // preceding char is ';', so visible should be false
- if (digits.charAt(start - 1) == ';') return false;
-
- // next char is ';', so visible should be false
- if ((digits.length() > start) && (digits.charAt(start) == ';')) return false;
- } else {
- // visible false in this case
- if (start > digits.length() || end > digits.length()) return false;
-
- // In this case we need to just check for ';' preceding to start
- // or next to end
- if (digits.charAt(start - 1) == ';') return false;
+ @VisibleForTesting
+ /* package */ static boolean canAddDigit(CharSequence digits, int start, int end,
+ char newDigit) {
+ if(newDigit != WAIT && newDigit != PAUSE) {
+ Log.wtf(TAG, "Should not be called for anything other than PAUSE & WAIT");
+ return false;
}
+
+ // False if no selection, or selection is reversed (end < start)
+ if (start == -1 || end < start) {
+ return false;
+ }
+
+ // unsupported selection-out-of-bounds state
+ if (start > digits.length() || end > digits.length()) return false;
+
+ // Special digit cannot be the first digit
+ if (start == 0) return false;
+
+ if (newDigit == WAIT) {
+ // preceding char is ';' (WAIT)
+ if (digits.charAt(start - 1) == WAIT) return false;
+
+ // next char is ';' (WAIT)
+ if ((digits.length() > end) && (digits.charAt(end) == WAIT)) return false;
+ }
+
return true;
}
diff --git a/tests/src/com/android/dialer/dialpad/DialpadFragmentTest.java b/tests/src/com/android/dialer/dialpad/DialpadFragmentTest.java
new file mode 100644
index 0000000..a123e74
--- /dev/null
+++ b/tests/src/com/android/dialer/dialpad/DialpadFragmentTest.java
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * 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.android.dialer.dialpad;
+
+import android.test.suitebuilder.annotation.SmallTest;
+
+import junit.framework.TestCase;
+
+/** Unit tests for {@link DialpadFragment}. */
+@SmallTest
+public class DialpadFragmentTest extends TestCase {
+
+ public void testCanAddDigit_Valid() {
+ // end, middle, selection to end, middle selection
+ assertTrue(DialpadFragment.canAddDigit("123", 3, 3, ';'));
+ assertTrue(DialpadFragment.canAddDigit("123", 1, 1, ','));
+ assertTrue(DialpadFragment.canAddDigit("123", 1, 3, ';'));
+ assertTrue(DialpadFragment.canAddDigit("123", 1, 2, ','));
+ }
+
+ public void testCanAddDigit_InvalidCharacter() {
+ // only handles wait/pause
+ assertFalse(DialpadFragment.canAddDigit("123", 1, 1, '5'));
+ }
+
+ public void testCanAddDigit_BadOrNoSelection() {
+ // no selection
+ assertFalse(DialpadFragment.canAddDigit("123", -1, -1, ';'));
+ assertFalse(DialpadFragment.canAddDigit("123", -1, 1, ','));
+
+ // start > end
+ assertFalse(DialpadFragment.canAddDigit("123", 2, 1, ','));
+ }
+
+ public void testCanAddDigit_OutOfBounds() {
+ // start or end is > digits.length()
+ assertFalse(DialpadFragment.canAddDigit("123", 1, 4, ';'));
+ assertFalse(DialpadFragment.canAddDigit("123", 4, 4, ','));
+ }
+
+ public void testCanAddDigit_AsFirstCharacter() {
+ assertFalse(DialpadFragment.canAddDigit("", 0, 0, ','));
+ assertFalse(DialpadFragment.canAddDigit("123", 0, 0, ';'));
+ assertFalse(DialpadFragment.canAddDigit("123", 0, 2, ','));
+ assertFalse(DialpadFragment.canAddDigit("123", 0, 3, ','));
+ }
+
+ public void testCanAddDigit_AdjacentCharacters_Before() {
+ // before
+ assertFalse(DialpadFragment.canAddDigit("55;55", 2, 2, ';')); // WAIT
+ assertFalse(DialpadFragment.canAddDigit("55;55", 1, 2, ';'));
+ assertTrue(DialpadFragment.canAddDigit("55,55", 2, 2, ',')); // PAUSE
+ assertTrue(DialpadFragment.canAddDigit("55,55", 1, 2, ','));
+ assertTrue(DialpadFragment.canAddDigit("55;55", 2, 2, ',')); // WAIT & PAUSE
+ assertTrue(DialpadFragment.canAddDigit("55,55", 1, 2, ';'));
+ }
+
+ public void testCanAddDigit_AdjacentCharacters_After() {
+ // after
+ assertFalse(DialpadFragment.canAddDigit("55;55", 3, 3, ';')); // WAIT
+ assertFalse(DialpadFragment.canAddDigit("55;55", 3, 4, ';'));
+ assertTrue(DialpadFragment.canAddDigit("55,55", 3, 3, ',')); // PAUSE
+ assertTrue(DialpadFragment.canAddDigit("55,55", 3, 4, ','));
+ assertTrue(DialpadFragment.canAddDigit("55;55", 3, 3, ',')); // WAIT & PAUSE
+ assertTrue(DialpadFragment.canAddDigit("55,55", 3, 4, ';'));
+ }
+}