MD Settings: CrOS: Focus elements on dialog close

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

Index: chrome/browser/resources/settings/internet_page/network_siminfo.js
diff --git a/chrome/browser/resources/settings/internet_page/network_siminfo.js b/chrome/browser/resources/settings/internet_page/network_siminfo.js
index 8772be4c69a704c9089385d36108a00133f9b818..6b11b88e163dac441ecd09905bf109e3353443c5 100644
--- a/chrome/browser/resources/settings/internet_page/network_siminfo.js
+++ b/chrome/browser/resources/settings/internet_page/network_siminfo.js
@@ -99,8 +99,7 @@ Polymer({
     if (!showUnlockPuk)
       return;
 
-    this.error_ = ErrorType.NONE;
-    this.$.unlockPukDialog.showModal();
+    this.showUnlockPukDialog_();
   },
 
   /**
@@ -220,12 +219,8 @@ Polymer({
     }.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.

@@ -342,6 +337,21 @@ Polymer({
       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();
+  },
 });
 })();