MD Settings: Display: Move event listeners to dialog open/close

chrome/browser/resources/settings/device_page/display_overscan_dialog.js

Index: chrome/browser/resources/settings/device_page/display_overscan_dialog.js
diff --git a/chrome/browser/resources/settings/device_page/display_overscan_dialog.js b/chrome/browser/resources/settings/device_page/display_overscan_dialog.js
index 4cb019fcabdedb1d74e988030345aaf1032ed325..192c9dc313544592000c8b10dcafd314004ec681 100644
--- a/chrome/browser/resources/settings/device_page/display_overscan_dialog.js
+++ b/chrome/browser/resources/settings/device_page/display_overscan_dialog.js
@@ -30,23 +30,16 @@ Polymer({
    */
   keyHandler_: null,
 
-  /** @override */
-  attached: function() {
-    this.keyHandler_ = this.handleKeyEvent_.bind(this);
-    window.addEventListener('keydown', this.keyHandler_);
-  },
-
-  /** @override */
-  detached: function() {
-    window.removeEventListener('keydown', this.keyHandler_);
-  },
-
   open: function() {
+    this.keyHandler_ = this.handleKeyEvent_.bind(this);
+    this.addEventListener('keydown', this.keyHandler_);
     this.comitted_ = false;
     this.$.dialog.showModal();
   },
 
   close: function() {
+    this.removeEventListener('keydown', this.keyHandler_);
+
     this.displayId = '';  // Will trigger displayIdChanged_.
 
     if (this.$.dialog.open)
@@ -83,40 +76,49 @@ Polymer({
    * @private
    */
   handleKeyEvent_: function(event) {
-    switch (event.keyCode) {
-      case 37:  // left arrow
-        if (event.shiftKey)
-          this.move_(-1, 0);
-        else
-          this.resize_(1, 0);
-        break;
-      case 38:  // up arrow
-        if (event.shiftKey)
-          this.move_(0, -1);
-        else
-          this.resize_(0, -1);
-        break;
-      case 39:  // right arrow
-        if (event.shiftKey)
-          this.move_(1, 0);
-        else
-          this.resize_(-1, 0);
-        break;
-      case 40:  // down arrow
-        if (event.shiftKey)
-          this.move_(0, 1);
-        else
-          this.resize_(0, 1);
-        break;
-      default:

[Dan Beam, 2016/09/28 22:10:38]

// Only preventDefault() for keys that do something (i.e. arrow keys).

-        return;
+    if (event.altKey || event.ctrlKey || event.metaKey)
+      return;
+    if (this.handleKeyCode_(event.keyCode, event.shiftKey))
+      event.preventDefault();
+  },
+
+  /**
+   * @param {number} keyCode
+   * @param {boolean} shiftKey
+   * @return {boolean}
+   * @private
+   */
+  handleKeyCode_: function(keyCode, shiftKey) {
+    if (shiftKey) {
+      switch (keyCode) {
+        case 37:  // left arrow
+          return this.move_(-1, 0);
+        case 38:  // up arrow
+          return this.move_(0, -1);
+        case 39:  // right arrow
+          return this.move_(1, 0);
+        case 40:  // down arrow
+          return this.move_(0, 1);
+      }
+    } else {
+      switch (keyCode) {
+        case 37:  // left arrow
+          return this.resize_(1, 0);
+        case 38:  // up arrow
+          return this.resize_(0, -1);
+        case 39:  // right arrow
+          return this.resize_(-1, 0);
+        case 40:  // down arrow
+          return this.resize_(0, 1);
+      }
     }
-    event.preventDefault();
+    return false;
   },
 
   /**
    * @param {number} x
    * @param {number} y
+   * @return {boolean} Always returns true for simplifed switch handling.
    * @private
    */
   move_: function(x, y) {
@@ -128,11 +130,13 @@ Polymer({
     };
     settings.display.systemDisplayApi.overscanCalibrationAdjust(
         this.displayId, delta);
+    return true;
   },
 
   /**
    * @param {number} x
    * @param {number} y
+   * @return {boolean} Always returns true for simplifed switch handling.
    * @private
    */
   resize_: function(x, y) {
@@ -144,5 +148,6 @@ Polymer({
     };
     settings.display.systemDisplayApi.overscanCalibrationAdjust(
         this.displayId, delta);
+    return true;
   }
 });