Blimp: Added spinner to text input dialog

blimp/client/core/contents/android/java/src/org/chromium/blimp/core/contents/input/ImeHelperDialog.java

Index: blimp/client/core/contents/android/java/src/org/chromium/blimp/core/contents/input/ImeHelperDialog.java
diff --git a/blimp/client/core/contents/android/java/src/org/chromium/blimp/core/contents/input/ImeHelperDialog.java b/blimp/client/core/contents/android/java/src/org/chromium/blimp/core/contents/input/ImeHelperDialog.java
index b27e24f953445256c3ca644e772dfcf351f6d68c..461e46be917d1e2cfbaa4935e974065ee1f077c9 100644
--- a/blimp/client/core/contents/android/java/src/org/chromium/blimp/core/contents/input/ImeHelperDialog.java
+++ b/blimp/client/core/contents/android/java/src/org/chromium/blimp/core/contents/input/ImeHelperDialog.java
@@ -13,7 +13,9 @@ import android.view.KeyEvent;
 import android.view.View;
 import android.view.WindowManager;
 import android.view.inputmethod.EditorInfo;
+import android.view.inputmethod.InputMethodManager;
 import android.widget.EditText;
+import android.widget.ProgressBar;
 import android.widget.TextView;
 
 import org.chromium.base.annotations.CalledByNative;
@@ -24,8 +26,9 @@ import org.chromium.ui.base.ime.TextInputType;
 
 /**
  * Helper class showing the UI that allows users to enter text into a web page.
- * A pop up is created when user taps on a text area prompting user to start typing and closes as
- * soon as user hits enter or presses back button.
+ * A pop up is created when user taps on a text area prompting user to start typing.
+ * As soon as user submits the text, a spinner is shown which is dismissed only after
+ * receiving a hide IME request from the engine.
  */
 @JNINamespace("blimp::client")
 public class ImeHelperDialog {

[David Trainor- moved to gerrit, 2016/10/19 16:40:31]

We should probably rename this or look at restructuring the code in a bit.  This
class isn't actually a dialog :).

[shaktisahu, 2016/10/27 02:42:56]

Yea.. Probably rename to something else, but this class is still all about
driving a dialog. :)

[David Trainor- moved to gerrit, 2016/10/28 05:08:08]

Yeah good point.  Although I'd be fine with naming it something that describes
what it does to someone who doesn't know the internals.  But I'm okay if you
want to call it WebFormDialogDriver as well ;).

@@ -33,7 +36,10 @@ public class ImeHelperDialog {
     private final Context mContext;
 
     private AlertDialog mAlertDialog;
+    private ImeEditText mEditText;
+    private ProgressBar mProgressDialog;

[David Trainor- moved to gerrit, 2016/10/19 16:40:31]

Dialog seems like an incorrect name

[shaktisahu, 2016/10/27 02:42:56]

Done. Removed field.

     private long mNativeImeHelperDialog;

[David Trainor- moved to gerrit, 2016/10/19 16:40:30]

Can you pull the pointer out separately from the java-specific members?

[shaktisahu, 2016/10/27 02:42:56]

Done.

+    private String mExistingText;
 
     @CalledByNative
     private static ImeHelperDialog create(long nativeImeHelperDialog, WindowAndroid windowAndroid) {
@@ -60,9 +66,26 @@ public class ImeHelperDialog {
      * Sends the text entered from IME to blimp engine.
      * @param text The text the user entered.
      */
-    private void onImeTextEntered(String text) {
+    private void onImeTextEntered(final String text) {
         if (mNativeImeHelperDialog == 0) return;
 
+        if (text.equals(mExistingText)) {

[David Trainor- moved to gerrit, 2016/10/19 16:40:31]

We still need to tell the engine that we're dismissing the IME, we just don't
need to send text to it or trigger the spinner.

[shaktisahu, 2016/10/27 02:42:56]

Why? Is it for the state sync? Anyway, I think handling this condition is
unnecessary. I will remove this.

[David Trainor- moved to gerrit, 2016/10/28 05:08:08]

Yeah we'll have to do it for state sync, but that's a good point I don't know
what the implications are in the blink layer.  But for state sync we'll have to
mark the web form UI as hidden.

+            mAlertDialog.dismiss();
+            return;
+        }
+
+        // Hide the IME along with the OK/Cancel buttons. Start the progress spinner.
+        InputMethodManager imm =
+                (InputMethodManager) mContext.getSystemService(Activity.INPUT_METHOD_SERVICE);
+        imm.toggleSoftInput(InputMethodManager.HIDE_IMPLICIT_ONLY, 0);
+
+        mAlertDialog.getButton(DialogInterface.BUTTON_POSITIVE).setVisibility(View.GONE);

[David Trainor- moved to gerrit, 2016/10/19 16:40:30]

Let's talk about this today.  It might be clean to write a simple widget that
understands how to swap those buttons with a progress bar, that way we can reuse
it for all types of input/eventual UI if we need to?

[shaktisahu, 2016/10/27 02:42:56]

Done.

[David Trainor- moved to gerrit, 2016/10/28 05:08:08]

w00t!

+        mAlertDialog.getButton(DialogInterface.BUTTON_NEGATIVE).setVisibility(View.GONE);
+        mProgressDialog.setVisibility(View.VISIBLE);
+
+        mEditText.setTextColor(R.color.disabled_text_color);
+        mEditText.setBackgroundResource(R.drawable.dotted_line);

[David Trainor- moved to gerrit, 2016/10/19 16:40:31]

Do we have a blue line for normal entry?

[shaktisahu, 2016/10/27 02:42:56]

Yes, the blue line comes with the default theme.

[David Trainor- moved to gerrit, 2016/10/28 05:08:08]

Acknowledged.

+
         nativeOnImeTextEntered(mNativeImeHelperDialog, text);
     }
 
@@ -83,36 +106,27 @@ public class ImeHelperDialog {
     }
 
     private void createTextInputPopup(int inputType, String existingText) {
+        mExistingText = existingText;
         final View viewPopup =
                 ((Activity) mContext).getLayoutInflater().inflate(R.layout.text_input_popup, null);
-        final ImeEditText editText = (ImeEditText) viewPopup.findViewById(R.id.ime_edit_text);
+        final TextView tvLabel = (TextView) viewPopup.findViewById(R.id.label);

[David Trainor- moved to gerrit, 2016/10/19 16:40:30]

Can we have this and the set text lines close together?

[shaktisahu, 2016/10/27 02:42:56]

Done.

+        mEditText = (ImeEditText) viewPopup.findViewById(R.id.ime_edit_text);
+        mProgressDialog = (ProgressBar) viewPopup.findViewById(R.id.submit_spinner);
+
         mAlertDialog = new AlertDialog.Builder(mContext)
-                               .setTitle(R.string.blimp_ime_dialog_title)
-                               .setPositiveButton(R.string.blimp_form_input_ok,
-                                       new DialogInterface.OnClickListener() {
-                                           @Override
-                                           public void onClick(DialogInterface dialog, int which) {
-                                               onImeTextEntered(editText.getText().toString());
-                                               dialog.dismiss();
-                                           }
-                                       })
-                               .setNegativeButton(R.string.blimp_form_input_cancel,
-                                       new DialogInterface.OnClickListener() {
-                                           @Override
-                                           public void onClick(DialogInterface dialog, int id) {
-                                               dialog.dismiss();
-                                           }
-                                       })
+                               .setPositiveButton(R.string.blimp_form_input_ok, null)
+                               .setNegativeButton(R.string.blimp_form_input_cancel, null)
                                .create();
-        editText.initialize(mAlertDialog);
+        mEditText.initialize(mAlertDialog);
         mAlertDialog.setView(viewPopup);
         mAlertDialog.getWindow().setSoftInputMode(
                 WindowManager.LayoutParams.SOFT_INPUT_STATE_VISIBLE);
         mAlertDialog.setCanceledOnTouchOutside(true);
 
-        setEditorOptions(editText, inputType);
-        editText.setText(existingText);
-        editText.setOnEditorActionListener(new TextView.OnEditorActionListener() {
+        tvLabel.setText(R.string.blimp_ime_dialog_title);

[David Trainor- moved to gerrit, 2016/10/19 16:40:31]

Should this be called "web_input_default_label"?  This will be overridden by the
info coming from the site if available right?

[shaktisahu, 2016/10/27 02:42:56]

Done. Yes.

+        setEditorOptions(mEditText, inputType);
+        mEditText.setText(existingText);
+        mEditText.setOnEditorActionListener(new TextView.OnEditorActionListener() {
             @Override
             public boolean onEditorAction(TextView tv, int actionId, KeyEvent event) {
                 switch (actionId) {
@@ -121,7 +135,6 @@ public class ImeHelperDialog {
                     case EditorInfo.IME_ACTION_SEARCH:
                     case EditorInfo.IME_ACTION_GO:
                         onImeTextEntered(tv.getText().toString());
-                        mAlertDialog.dismiss();
                         return true;
                     default:
                         return false;
@@ -129,9 +142,18 @@ public class ImeHelperDialog {
             }
         });
 
-        editText.requestFocus();
-
         mAlertDialog.show();
+
+        // Override the OK button after it is created and shown.
+        mAlertDialog.getButton(DialogInterface.BUTTON_POSITIVE)
+                .setOnClickListener(new View.OnClickListener() {
+                    @Override
+                    public void onClick(View view) {
+                        onImeTextEntered(mEditText.getText().toString());
+                    }
+                });
+
+        mEditText.requestFocus();
     }
 
     /**
@@ -146,36 +168,36 @@ public class ImeHelperDialog {
         switch (inputTypeEngine) {
             case TextInputType.TEXT:
                 inputType = InputType.TYPE_CLASS_TEXT;
-                imeOptions = EditorInfo.IME_ACTION_GO;
+                imeOptions |= EditorInfo.IME_ACTION_GO;
                 break;
             case TextInputType.PASSWORD:
                 inputType = InputType.TYPE_CLASS_TEXT | InputType.TYPE_TEXT_VARIATION_PASSWORD;
-                imeOptions = EditorInfo.IME_ACTION_GO;
+                imeOptions |= EditorInfo.IME_ACTION_GO;
                 break;
             case TextInputType.SEARCH:
-                imeOptions = EditorInfo.IME_ACTION_SEARCH;
+                imeOptions |= EditorInfo.IME_ACTION_SEARCH;
                 break;
             case TextInputType.EMAIL:
                 inputType = InputType.TYPE_CLASS_TEXT | InputType.TYPE_TEXT_VARIATION_EMAIL_ADDRESS;
-                imeOptions = EditorInfo.IME_ACTION_GO;
+                imeOptions |= EditorInfo.IME_ACTION_GO;
                 break;
             case TextInputType.NUMBER:
                 inputType = InputType.TYPE_CLASS_NUMBER | InputType.TYPE_NUMBER_VARIATION_NORMAL
                         | InputType.TYPE_NUMBER_FLAG_DECIMAL;
-                imeOptions = EditorInfo.IME_ACTION_NEXT;
+                imeOptions |= EditorInfo.IME_ACTION_NEXT;
                 break;
             case TextInputType.TELEPHONE:
                 inputType = InputType.TYPE_CLASS_PHONE;
-                imeOptions = EditorInfo.IME_ACTION_NEXT;
+                imeOptions |= EditorInfo.IME_ACTION_NEXT;
                 break;
             case TextInputType.URL:
                 inputType = InputType.TYPE_CLASS_TEXT | InputType.TYPE_TEXT_VARIATION_URI;
-                imeOptions = EditorInfo.IME_ACTION_GO;
+                imeOptions |= EditorInfo.IME_ACTION_GO;
                 break;
             case TextInputType.TEXT_AREA:
             case TextInputType.CONTENT_EDITABLE:
-                inputType = InputType.TYPE_TEXT_FLAG_MULTI_LINE;
-                imeOptions = EditorInfo.IME_ACTION_NONE;
+                inputType |= InputType.TYPE_TEXT_FLAG_MULTI_LINE;
+                imeOptions |= EditorInfo.IME_ACTION_NONE;
                 break;
             default:
                 inputType = InputType.TYPE_CLASS_TEXT;