MD Settings: CrOS: Focus elements on dialog close

chrome/browser/resources/settings/internet_page/network_siminfo.js

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview Polymer element for displaying and modifying cellular sim info.
  */
 (function() {
 
 /** @enum {string} */
@@ skipping 81 matching lines @@
       this.$.changePinDialog.close();
       showUnlockPuk = true;
     }
     if (this.$.unlockPinDialog.open) {
       this.$.unlockPinDialog.close();
       showUnlockPuk = true;
     }
     if (!showUnlockPuk)
       return;
 
-    this.error_ = ErrorType.NONE;
-    this.$.unlockPukDialog.showModal();
+    this.showUnlockPukDialog_();
   },
 
   /**
    * Opens the pin dialog when the sim lock enabled state changes.
    * @param {Event} event
    * @private
    */
   onSimLockEnabledChange_: function(event) {
     if (!this.networkProperties || !this.networkProperties.Cellular)
       return;
@@ skipping 99 matching lines @@
       if (chrome.runtime.lastError) {
         this.error_ = ErrorType.INCORRECT_PIN;
         this.$.unlockPin.inputElement.select();
       } else {
         this.error_ = ErrorType.NONE;
         this.$.unlockPinDialog.close();
       }
     }.bind(this));
   },
 
-  /**
-   * Opens the Unlock PUK dialog.
-   * @param {Event} event
-   * @private
-   */
-  unlockPuk_: function(event) {
+  /** @private */
+  showUnlockPukDialog_: function() {
     this.error_ = ErrorType.NONE;
     this.$.unlockPukDialog.showModal();
     this.$.unlockPuk.value = '';

[dpapad, 2017/04/13 00:16:40]

How about initializing those three values before calling showModal()? Setting
them after the dialog is shown could cause flickering.

[stevenjb, 2017/04/13 00:39:09]

Done.

     this.$.unlockPin1.value = '';
     this.$.unlockPin2.value = '';
   },
 
   /**
    * Sends the PUK value and new PIN value from the Unblock PUK dialog.
    * @param {Event} event
    * @private
    */
   sendUnlockPuk_: function(event) {
@@ skipping 93 matching lines @@
    * @param {string} puk
    * @return {boolean} True if the puk is of minimum length.
    * @private
    */
   validatePuk_: function(puk) {
     if (puk.length < PUK_MIN_LENGTH) {
       this.error_ = ErrorType.INVALID_PUK;
       return false;
     }
     return true;
-  }
+  },
+
+  /** @private */
+  onEnterPinDialogClosed_: function() {
+    this.$$('#simLockButton').focus();
+  },
+
+  /** @private */
+  onChangePinDialogClose_: function() {
+    this.$$('#changePinButton').focus();
+  },
+
+  /** @private */
+  onUnlockPinDialogClose__: function() {

[dpapad, 2017/04/13 00:16:40]

Typo: There are two underscores here, which makes this mismatch the HTML code
binding.

Could you verify that all the focus() calls target the correct element manually
(given that we don't add tests for those)?

[stevenjb, 2017/04/13 00:39:09]

Thanks. Sadly these are extremely difficult to test correctly. I tested the
others, but included these optimistically. I'll be testing this along with other
Cellular related UI post branch.

Fixed.

+    this.$$('#unlockPinButton').focus();
+  },
 });
 })();