fix(TokenCompleteTextView): let the tab key move focus out of the field

library/token-auto-complete/src/main/java/com/tokenautocomplete/TokenCompleteTextView.java

@@ -34,6 +34,7 @@
 import android.view.inputmethod.InputConnectionWrapper;
 import android.view.inputmethod.InputMethodManager;
 import android.widget.Filter;
+import android.widget.ListAdapter;
 import android.widget.ListView;
 import android.widget.TextView;
 
@@ -68,7 +69,7 @@ public abstract class TokenCompleteTextView<T> extends AppCompatAutoCompleteText
     private boolean initialized = false;
     private boolean performBestGuess = true;
     private boolean savingState = false;
-    private boolean shouldFocusNext = false;
+    private boolean completeOnKeyUp = false;
     private boolean allowCollapse = true;
     private boolean internalEditInProgress = false;
 
@@ -520,23 +521,29 @@ private void handleDone() {
 
     @Override
     public boolean onKeyUp(int keyCode, @NonNull KeyEvent event) {
-        boolean handled = super.onKeyUp(keyCode, event);
-        if (shouldFocusNext) {
-            shouldFocusNext = false;
+        // A completion armed by an enter or dpad centre key down is finished off here. It is drained before the
+        // tab key is looked at, so that a tab key up arriving while enter is still held cannot leave it armed to
+        // fire on some later, unrelated key up.
+        if (completeOnKeyUp) {
+            completeOnKeyUp = false;
             handleDone();
         }
-        return handled;
+
+        if (keyCode == KeyEvent.KEYCODE_TAB && event.hasNoModifiers()) {
+            return handleTabInCompletionList();
+        }
+
+        return super.onKeyUp(keyCode, event);
     }
 
     @Override
     public boolean onKeyDown(int keyCode, @NonNull KeyEvent event) {
         boolean handled = false;
         switch (keyCode) {
-            case KeyEvent.KEYCODE_TAB:
             case KeyEvent.KEYCODE_ENTER:
             case KeyEvent.KEYCODE_DPAD_CENTER:
                 if (event.hasNoModifiers()) {
-                    shouldFocusNext = true;
+                    completeOnKeyUp = true;
                     handled = true;
                 }
                 break;
@@ -545,9 +552,45 @@ public boolean onKeyDown(int keyCode, @NonNull KeyEvent event) {
                 break;
         }
 
+        // The tab key is deliberately not handled here. AutoCompleteTextView consumes it while the completion
+        // list is open and leaves it alone otherwise, which is what lets the framework move the focus on to the
+        // next view. Consuming it here would trap the focus in this field.
         return handled || super.onKeyDown(keyCode, event);
     }
 
+    /**
+     * Handles the tab key while the completion list is open.
+     *
+     * {@link android.widget.AutoCompleteTextView} completes straight away on tab, which takes the choice away
+     * from the user as soon as there is more than one suggestion. Instead a single suggestion is completed and
+     * several suggestions move the selection into the list, so it can be walked with tab or the arrow keys and
+     * confirmed with enter.
+     *
+     * @return true if the key was consumed.
+     */
+    private boolean handleTabInCompletionList() {
+        ListAdapter adapter = getAdapter();
+        if (!isPopupShowing() || adapter == null || adapter.getCount() == 0) {
+            return false;
+        }
+
+        if (adapter.getCount() == 1) {
+            // Nothing to choose between, complete the only suggestion and stay in this field.
+            setListSelection(0);
+            performCompletion();
+            return true;
+        }
+
+        if (getListSelection() == ListView.INVALID_POSITION) {
+            // The drop down keeps its selection hidden, and ignores key events, until something is selected.
+            // Selecting the first suggestion here is what makes it navigable in the first place.
+            setListSelection(0);
+        }
+        // With a selection in place the list moves it along itself, so there is nothing left to do but keep
+        // the key away from AutoCompleteTextView's complete-on-tab handling.
+        return true;
+    }
+
     @Override
     public boolean onEditorAction(TextView view, int action, KeyEvent keyEvent) {
         if (action == EditorInfo.IME_ACTION_DONE) {

library/token-auto-complete/src/test/java/com/tokenautocomplete/TokenCompleteTextViewBehaviorTest.java

@@ -11,9 +11,11 @@
 import android.os.SystemClock;
 import android.text.Editable;
 import android.text.SpannableStringBuilder;
+import android.view.KeyEvent;
 import android.view.MotionEvent;
 import android.view.View;
 import android.view.ViewGroup;
+import android.widget.ArrayAdapter;
 import android.widget.EditText;
 import android.widget.LinearLayout;
 import android.widget.TextView;
@@ -285,4 +287,160 @@ public void tokenTaps_neverRequestIme_emptyTap_requestsIme() {
         e.resetFromOther();
         e.tapAndAssert(xEmpty, yLast, null, 1);
     }
+
+    private static KeyEvent tab(int action) {
+        return new KeyEvent(action, KeyEvent.KEYCODE_TAB);
+    }
+
+    private static KeyEvent shiftTab(int action) {
+        return new KeyEvent(0, 0, action, KeyEvent.KEYCODE_TAB, 0, KeyEvent.META_SHIFT_ON);
+    }
+
+    private static void openCompletionList(Env e, String... suggestions) {
+        e.v.setAdapter(new ArrayAdapter<>(e.a, android.R.layout.simple_list_item_1, suggestions));
+        e.v.showDropDown();
+        e.idle();
+        assertTrue("Expected the completion list to be open", e.v.isPopupShowing());
+    }
+
+    @Test
+    public void tab_withCompletionListClosed_isNotConsumed() {
+        Env e = new Env();
+        e.focusTokens();
+        assertFalse(e.v.isPopupShowing());
+
+        // Not consuming the key is what lets the framework move the focus on to the next view.
+        assertFalse(e.v.onKeyDown(KeyEvent.KEYCODE_TAB, tab(KeyEvent.ACTION_DOWN)));
+        assertEquals(0, e.spans().length);
+    }
+
+    @Test
+    public void shiftTab_withCompletionListClosed_isNotConsumed() {
+        Env e = new Env();
+        e.focusTokens();
+
+        assertFalse(e.v.onKeyDown(KeyEvent.KEYCODE_TAB, shiftTab(KeyEvent.ACTION_DOWN)));
+    }
+
+    @Test
+    public void tab_withSingleSuggestion_completesIt() {
+        Env e = new Env();
+        e.focusTokens();
+        openCompletionList(e, "one@example.com");
+
+        assertTrue(e.v.onKeyUp(KeyEvent.KEYCODE_TAB, tab(KeyEvent.ACTION_UP)));
+        e.layout();
+
+        assertEquals(1, e.spans().length);
+        assertEquals("one@example.com", e.v.getObjects().get(0));
+    }
+
+    @Test
+    public void tab_withSeveralSuggestions_selectsTheFirstOneInsteadOfCompleting() {
+        Env e = new Env();
+        e.focusTokens();
+        openCompletionList(e, "one@example.com", "two@example.com");
+
+        assertTrue(e.v.onKeyUp(KeyEvent.KEYCODE_TAB, tab(KeyEvent.ACTION_UP)));
+        e.layout();
+
+        assertEquals(0, e.v.getListSelection());
+        assertEquals("Nothing should be completed yet", 0, e.spans().length);
+    }
+
+    @Test
+    public void tab_withSeveralSuggestions_neverCompletesOnItsOwn() {
+        Env e = new Env();
+        e.focusTokens();
+        openCompletionList(e, "one@example.com", "two@example.com");
+        e.v.setListSelection(1);
+        e.idle();
+
+        assertTrue(e.v.onKeyUp(KeyEvent.KEYCODE_TAB, tab(KeyEvent.ACTION_UP)));
+        e.layout();
+
+        assertEquals("The selection is left for the list to move", 1, e.v.getListSelection());
+        assertEquals("Picking a suggestion is enter's job", 0, e.spans().length);
+    }
+
+    @Test
+    public void enter_completesTheSelectedSuggestion() {
+        Env e = new Env();
+        e.focusTokens();
+        openCompletionList(e, "one@example.com", "two@example.com");
+        e.v.setListSelection(1);
+        e.idle();
+
+        e.v.onKeyDown(KeyEvent.KEYCODE_ENTER, new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_ENTER));
+        e.v.onKeyUp(KeyEvent.KEYCODE_ENTER, new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_ENTER));
+        e.layout();
+
+        assertEquals(1, e.spans().length);
+        assertEquals("two@example.com", e.v.getObjects().get(0));
+    }
+
+    @Test
+    public void enter_stillCompletesTheCurrentText() {
+        Env e = new Env();
+        e.focusTokens();
+        e.v.append("one@example.com");
+        e.layout();
+
+        assertTrue(e.v.onKeyDown(KeyEvent.KEYCODE_ENTER, new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_ENTER)));
+        e.v.onKeyUp(KeyEvent.KEYCODE_ENTER, new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_ENTER));
+        e.layout();
+
+        assertEquals(1, e.spans().length);
+    }
+
+    @Test
+    public void tab_downThenUp_withSeveralSuggestions_selectsWithoutCompleting() {
+        Env e = new Env();
+        e.focusTokens();
+        e.v.append("one");
+        e.layout();
+        openCompletionList(e, "one@example.com", "two@example.com");
+
+        // The real key sequence: the down leg goes through the drop down, which may already move the selection.
+        e.v.onKeyDown(KeyEvent.KEYCODE_TAB, tab(KeyEvent.ACTION_DOWN));
+        assertTrue(e.v.onKeyUp(KeyEvent.KEYCODE_TAB, tab(KeyEvent.ACTION_UP)));
+        e.layout();
+
+        assertTrue("A suggestion should be selected", e.v.getListSelection() >= 0);
+        assertEquals("Picking one is enter's job", 0, e.spans().length);
+    }
+
+    @Test
+    public void tab_downThenUp_withSingleSuggestion_completesIt() {
+        Env e = new Env();
+        e.focusTokens();
+        e.v.append("one");
+        e.layout();
+        openCompletionList(e, "one@example.com");
+
+        e.v.onKeyDown(KeyEvent.KEYCODE_TAB, tab(KeyEvent.ACTION_DOWN));
+        assertTrue(e.v.onKeyUp(KeyEvent.KEYCODE_TAB, tab(KeyEvent.ACTION_UP)));
+        e.layout();
+
+        assertEquals(1, e.spans().length);
+        assertEquals("one@example.com", e.v.getObjects().get(0));
+    }
+
+    @Test
+    public void tab_doesNotLeaveAnEnterCompletionArmedForALaterKey() {
+        Env e = new Env();
+        e.focusTokens();
+        openCompletionList(e, "one@example.com", "two@example.com");
+
+        // Enter arms a completion for its own key up. A tab key up arriving first must not leave it armed.
+        e.v.onKeyDown(KeyEvent.KEYCODE_ENTER, new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_ENTER));
+        e.v.onKeyUp(KeyEvent.KEYCODE_TAB, tab(KeyEvent.ACTION_UP));
+        e.layout();
+        int spansAfterTab = e.spans().length;
+
+        e.v.onKeyUp(KeyEvent.KEYCODE_A, new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_A));
+        e.layout();
+
+        assertEquals("An unrelated key up must not complete anything", spansAfterTab, e.spans().length);
+    }
 }