cros: Allow pin keyboard to keep focus without popping up the pin keyboard.

chrome/browser/resources/chromeos/quick_unlock/pin_keyboard.js

Index: chrome/browser/resources/chromeos/quick_unlock/pin_keyboard.js
diff --git a/chrome/browser/resources/chromeos/quick_unlock/pin_keyboard.js b/chrome/browser/resources/chromeos/quick_unlock/pin_keyboard.js
index d8b2762fb104855a57eb10cc72b4413c231f46ea..d18c20f22c2f4cdd63d34d58f71592ff931124be 100644
--- a/chrome/browser/resources/chromeos/quick_unlock/pin_keyboard.js
+++ b/chrome/browser/resources/chromeos/quick_unlock/pin_keyboard.js
@@ -12,14 +12,13 @@
  *           the PIN keyboard's value.
  *
  * Events:
- *    pin-change: Fired when the PIN value has changed. The pin is available at
+ *    pin-change: Fired when the PIN value has changed. The PIN is available at
  *                event.detail.pin.
- *    submit: Fired when the PIN is submitted. The pin is available at
+ *    submit: Fired when the PIN is submitted. The PIN is available at
  *            event.detail.pin.
  *
  * Example:
- *    <pin-keyboard on-pin-change="onPinChange" on-submit="onPinSubmit"
- *                  value="{{pinValue}}">
+ *    <pin-keyboard on-pin-change="onPinChange" on-submit="onPinSubmit">
  *    </pin-keyboard>
  */
 
@@ -40,6 +39,14 @@ var REPEAT_BACKSPACE_DELAY_MS = 150;
  */
 var INITIAL_BACKSPACE_DELAY_MS = 500;
 
+/**
+ * The key codes of the keys allowed to be used on the pin input, in addition to
+ * number keys. Currently we allow backspace(8), tab(9), left(37) and right(39).
+ * @type {Array<number>}
+ * @const
+ */
+var PIN_INPUT_ALLOWED_NON_NUMBER_KEY_CODES = [8, 9, 37, 39];
+
 Polymer({
   is: 'pin-keyboard',
 
@@ -51,6 +58,7 @@ Polymer({
     /**
      * Whether or not the keyboard's input element should be numerical
      * or password.
+     * @private
      */
     enablePassword: {
       type: Boolean,
@@ -58,17 +66,23 @@ Polymer({
     },
 
     /**
-     * Whether or not the keyboard's input should be shown.
+     * The password element the pin keyboard is associated with. If this is not
+     * set, then a default input element is shown and used.
+     * @type {HTMLInputElement}
+     * @private
      */
-    hideInput: {
-      type: Boolean,
-      value: false
+    passwordElement: {
+      type: Object,
+      value: function() { return this.$$('#pin-input'); },
+      observer: 'onPasswordElementAttached_'
     },
 
-    /** The value stored in the keyboard's input element. */
+    /**
+     * The value stored in the keyboard's input element.
+     * @private
+     */
     value: {
       type: String,
-      notify: true,
       value: '',
       observer: 'onPinValueChange_'
     },
@@ -93,37 +107,90 @@ Polymer({
   },
 
   /**
-   * @override
+   * Called when a password element is attached to the pin keyboard.
+   * @param {HTMLInputElement} inputElement The PIN keyboard's input element.
+   * @private
+   */
+  onPasswordElementAttached_: function(inputElement) {
+    inputElement.addEventListener('input',
+        this.handleInputChanged_.bind(this));
+  },
+
+  /**
+   * Called when the user uses the keyboard to enter a value into the input
+   * element.
+   * @param {Event} event The event object.
+   * @private
    */
-  attached: function() {
-    // Remove the space/enter key binds from the polymer
-    // iron-a11y-keys-behavior.
-    var digitButtons = Polymer.dom(this.root).querySelectorAll('.digit-button');
-    for (var i = 0; i < digitButtons.length; ++i)
-      digitButtons[i].keyEventTarget = null;
+  handleInputChanged_: function(event) {
+    this.value = event.target.value;
   },
 
   /**
-   * Gets the container holding the password field.
-   * @type {!HTMLInputElement}
+   * Gets the selection start of the input field.
+   * @type {number}
+   * @private
    */
-  get inputElement() {
-    return this.$$('#pin-input');
+  get selectionStart() {

[jdufault, 2016/11/03 17:42:41]

Please append _ to the name of these four methods.

[sammiequon, 2016/11/03 22:26:55]

Done.

+    return this.passwordElement.selectionStart;
+  },
+
+  /**
+   * Gets the selection end of the input field.
+   * @type {number}
+   * @private
+   */
+  get selectionEnd() {
+    return this.passwordElement.selectionEnd;
+  },
+
+  /**
+   * Sets the selection start of the input field.
+   * @param {number} start The new selection start of the input element.
+   * @private
+   */
+  set selectionStart(start) {
+    this.passwordElement.selectionStart = start;
+  },
+
+  /**
+   * Sets the selection end of the input field.
+   * @param {number} end The new selection end of the input element.
+   * @private
+   */
+  set selectionEnd(end) {
+    this.passwordElement.selectionEnd = end;
   },
 
   /** Transfers focus to the input element. */
   focus: function() {
-    this.$$('#pin-input').focus();
+    this.passwordElement.focus();
   },
 
   /**
    * Called when a keypad number has been tapped.
-   * @param {!{target: !PaperButtonElement}} event
+   * @param {Event} event The event object.
    * @private
    */
-  onNumberTap_: function(event, detail) {
+  onNumberTap_: function(event) {
     var numberValue = event.target.getAttribute('value');
-    this.value += numberValue;
+
+    // Add the number where the caret is, then update the selection range of the
+    // input element.
+    var selectionStart = this.selectionStart;
+    var selectionEnd = this.selectionEnd;

[jdufault, 2016/11/03 17:42:41]

Are selectionStart/selectionEnd used for caching the values, or does something
like the below work? If the variables are used just for caching you should
explicitly call that out in a comment.

  this.value = this.value.substring(0, this.selectionStart) + numberValue +
               this.value.substring(this.selectionEnd)
  this.selectionStart += 1
  this.selectionEnd = this.selectionStart;

[sammiequon, 2016/11/03 22:26:55]

Done.

+    this.value = this.value.substring(0, selectionStart) + numberValue +
+        this.value.substring(selectionEnd);
+    this.selectionStart = selectionStart + 1;
+    this.selectionEnd = selectionStart + 1;
+
+    // If a number button is clicked, we do not want to switch focus to the
+    // button, therefore we transfer focus back to the input, but if a number
+    // button is tabbed into, it should keep focus, so users can use tab and
+    // spacebar/return to enter their PIN.
+    if (!event.target.receivedFocusFromKeyboard)
+      this.focus();
+    event.preventDefault();
   },
 
   /** Fires a submit event with the current PIN value. */
@@ -138,8 +205,17 @@ Polymer({
    * @param {string} previous
    */
   onPinValueChange_: function(value, previous) {
-    if (value != previous)
+    if (value != previous) {
+      // The selection caret gets placed at the end after altering the
+      // password element, so we store the previous location(s) and reapply
+      // them after the the new value is set.
+      var selectionStart = this.selectionStart;
+      var selectionEnd = this.selectionEnd;
+      this.passwordElement.value = this.value;
+      this.selectionStart = selectionStart;
+      this.selectionEnd = selectionEnd;
       this.fire('pin-change', { pin: value });
+    }
   },
 
   /**
@@ -148,7 +224,20 @@ Polymer({
    * @private
    */
   onPinClear_: function() {
-    this.value = this.value.substring(0, this.value.length - 1);
+    // If the input is shown, clear the text based on the caret location or
+    // selected region of the input element.
+    var selectionStart = this.selectionStart;
+    var selectionEnd = this.selectionEnd;
+
+    // If it is just a caret, remove the character in front of the caret.
+    if (selectionStart == selectionEnd)
+      selectionStart--;
+    this.value = this.value.substring(0, selectionStart) +
+        this.value.substring(selectionEnd);
+
+    // Move the caret or selected region to the correct new place.
+    this.selectionStart = selectionStart;
+    this.selectionEnd = selectionStart;
   },
 
   /**
@@ -163,6 +252,10 @@ Polymer({
         this.repeatBackspaceIntervalId_ = setInterval(
             this.onPinClear_.bind(this), REPEAT_BACKSPACE_DELAY_MS);
     }.bind(this), INITIAL_BACKSPACE_DELAY_MS);
+
+    if (!event.target.receivedFocusFromKeyboard)
+      this.focus();
+    event.preventDefault();
   },
 
   /**
@@ -184,6 +277,10 @@ Polymer({
    */
   onBackspacePointerOut_: function(event) {
     this.clearAndReset_();
+
+    if (!event.target.receivedFocusFromKeyboard)
+      this.focus();
+    event.preventDefault();
   },
 
   /**
@@ -198,82 +295,69 @@ Polymer({
     if (!this.repeatBackspaceIntervalId_)
       this.onPinClear_();
     this.clearAndReset_();
-  },
-
-  /** Called when a key event is pressed while the input element has focus. */
-  onInputKeyDown_: function(event) {
-    // Up/down pressed, swallow the event to prevent the input value from
-    // being incremented or decremented.
-    if (event.keyCode == 38 || event.keyCode == 40) {
-      event.preventDefault();
-      return;
-    }
 
-    // Enter pressed.
-    if (event.keyCode == 13) {
-      this.firePinSubmitEvent_();
-      event.preventDefault();
-      return;
-    }
+    if (!event.target.receivedFocusFromKeyboard)
+      this.focus();
+    event.preventDefault();
   },
 
   /**
-   * Keypress does not handle backspace but handles the char codes nicely, so we
-   * have a seperate event to process the backspaces.
-   * @param {Event} event Keydown Event object.
+   * Helper function to check whether a given |event| should be processed by
+   * the numeric only input.
+   * @param {Event} event The event object.
    * @private
    */
-  onKeyDown_: function(event) {
-    // Backspace pressed.
-    if (event.keyCode == 8) {
-      this.onPinClear_();
-      event.preventDefault();
-      return;
-    }
+  isValidEventForInput_: function(event) {
+    // Valid if the key is a number, and shift is not pressed.
+    if ((event.keyCode >= 48 && event.keyCode <= 57) && !event.shiftKey)
+      return true;
+
+    // Valid if the key is one of the selected special keys defined in
+    // |PIN_INPUT_ALLOWED_NON_NUMBER_KEY_CODES|.
+    if (PIN_INPUT_ALLOWED_NON_NUMBER_KEY_CODES.indexOf(event.keyCode) > -1)
+      return true;
+
+    // Valid if a is pressed while the control key is pressed to allow users to
+    // quickly select the entire PIN.
+    if (event.keyCode == 65 && event.ctrlKey)
+      return true;
+
+    // The rest of the keys are invalid.
+    return false;
   },
 
   /**
-   * Called when a key press event is fired while the number button is focused.
-   * Ideally we would want to pass focus back to the input element, but the we
-   * cannot or the virtual keyboard will keep poping up.
-   * @param {Event} event Keypress Event object.
+   * Called when a key event is pressed while the input element has focus.
+   * @param {Event} event The event object.
    * @private
    */
-  onKeyPress_: function(event) {
-    // If the active element is the input element, the input element itself will
-    // handle the keypresses, so we do not handle them here.
-    if (this.shadowRoot.activeElement == this.inputElement)
+  onInputKeyDown_: function(event) {
+    // Up/down pressed, swallow the event to prevent the input value from
+    // being incremented or decremented.
+    if (event.keyCode == 38 || event.keyCode == 40) {
+      event.preventDefault();
       return;
-
-    var code = event.keyCode;
+    }
 
     // Enter pressed.
-    if (code == 13) {
+    if (event.keyCode == 13) {
       this.firePinSubmitEvent_();
       event.preventDefault();
       return;
     }
 
-    // Space pressed. We want the old polymer function of space activating the
-    // button with focus.
-    if (code == 32) {
-      // Check if target was a number button.
-      if (event.target.hasAttribute('value')) {
-        this.value += event.target.getAttribute('value');
-        return;
-      }
-      // Check if target was backspace button.
-      if (event.target.classList.contains('backspace-button')) {
-        this.onPinClear_();
-        return;
-      }
+    // Do not pass events that are not numbers or special keys we care about. We
+    // use this instead of input type number because there are several issues
+    // with input type number, such as no selectionStart/selectionEnd and
+    // entered non numbers causes the caret to jump to the left.
+    if (!this.isValidEventForInput_(event)) {
+      event.preventDefault();
+      return;
     }
-
-    this.value += String.fromCharCode(code);
   },
 
   /**
-   * Disables the submit and backspace button if nothing is entered.
+   * Disables the backspace button if nothing is entered.
    * @param {string} value
    * @private
    */
@@ -282,16 +366,6 @@ Polymer({
   },
 
   /**
-   * Computes whether the input type for the pin input should be password or
-   * numerical.
-   * @param {boolean} enablePassword
-   * @private
-   */
-  getInputType_: function(enablePassword) {
-    return enablePassword ? 'password' : 'number';
-  },
-
-  /**
    * Computes the value of the pin input placeholder.
    * @param {boolean} enablePassword
    * @private