MD Settings: Implementing modal popup/action menus.

chrome/browser/resources/settings/settings_action_menu.js

Index: chrome/browser/resources/settings/settings_action_menu.js
diff --git a/chrome/browser/resources/settings/settings_action_menu.js b/chrome/browser/resources/settings/settings_action_menu.js
new file mode 100644
index 0000000000000000000000000000000000000000..090c803bc57faf0f5de483aac243a0ebfd099abf
--- /dev/null
+++ b/chrome/browser/resources/settings/settings_action_menu.js
@@ -0,0 +1,142 @@
+// Copyright 2016 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.
+
+Polymer({
+  is: 'settings-action-menu',
+  extends: 'dialog',
+
+  /**
+   * List of all options in this action menu.
+   * @private {?NodeList<!Element>}
+   */
+  options_: null,
+
+  /**
+   * Index of the currently focused item.
+   * @private {number}
+   */
+  focusedIndex_: -1,
+
+  /**
+   * Reference to the bound window's resize listener, such that it can be
+   * removed on detach.
+   * @private {?Function}
+   */
+  onWindowResize_: null,
+
+  hostAttributes: {
+    tabindex: 0,
+  },
+
+  /** override */
+  attached: function() {
+    this.options_ = this.querySelectorAll('.dropdown-item');
+
+    this.onWindowResize_ = function() {
+      if (this.open)
+        this.close();
+    }.bind(this);
+
+    window.addEventListener('resize', this.onWindowResize_);
+  },
+
+  /** override */
+  detached: function() {
+    window.removeEventListener('resize', this.onWindowResize_);
+  },
+
+  /** @override */
+  ready: function() {
+    this.addEventListener('click', function(e) {

[Dan Beam, 2016/10/14 03:51:13]

should this be equivalent to on-tap in some way?  does this work with touch?

[Dan Beam, 2016/10/14 03:51:13]

can you move these both to:

listeners: {
  'click': 'onClick_',
  'keydown': 'onKeyDown_',
},

so we re-use more polymer magic (and they're attached() and detached()
correctly?)

[dpapad, 2016/10/14 17:39:23]

Done.

[dpapad, 2016/10/14 17:49:09]

Changed to tap, even though both seemed to work using just 'click' (using the
device emulator).

+      if (e.target == this) {
+        this.close();
+        e.stopPropagation();
+      }
+    }.bind(this));
+
+    this.addEventListener('keydown', function(e) {

[Dan Beam, 2016/10/14 03:51:13]

can we use iron-a11y-keys-behavior?

behaviors: [Polymer.IronA11yKeysBehavior],

keyBindings: {
  'down': 'onUpOrDown_',
  'tab': 'close',
  'up': 'onUpOrDown_',
},

onUpOrDown_: function(e) {
  var next = this.getNextOption_(this.keyboardEventMatchesKeys('up') ? -1 : 1);
  if (next)
    next.focus();
  e.preventDefault();
},

[dpapad, 2016/10/14 17:39:23]

What is the benefit in doing this? By reading the documentation and looking at
the class itself

- part of that class is simply dealing with browser differences.
  "The element takes care of browser differences with respect to Keyboard
events"
- It defines a lot of enums that we don't need,
- Deals with key combos, which we don't need.
- It is is 500 LOC vs 14 LOC currently needed here.

So, I prefer not to use it, because I don't see a benefit from using it.

+      if (e.key == 'Tab') {
+        this.close();
+        return;
+      }
+
+      if (e.key !== 'ArrowDown' && e.key !== 'ArrowUp')
+        return;
+
+      var nextOption = this.getNextOption_(e.key == 'ArrowDown' ? 1 : - 1);
+      if (nextOption)
+        nextOption.focus();
+
+      e.preventDefault();
+    }.bind(this));
+  },
+
+  /**
+   * @param {number} step -1 for getting previous option (up), 1 for getting
+   *     next option (down).
+   * @return {?Element} The next focusable option, taking into account
+   *     disabled/hidden attributes, or null if no focusable option exists.
+   * @private
+   */
+  getNextOption_: function(step) {
+    // Using a counter to ensure no infinite loop occurs if all elements are
+    // hidden/disabled.
+    var counter = 0;
+    var nextOption = null;
+    var numOptions = this.options_.length;
+
+    do {
+      this.focusedIndex_ =
+          (numOptions + this.focusedIndex_ + step) % numOptions;
+      nextOption = this.options_[this.focusedIndex_];
+      if (nextOption.disabled || nextOption.hidden)
+        nextOption = null;
+      counter++;
+    } while (!nextOption && counter < numOptions);
+
+    return nextOption;
+  },
+
+  /**
+   * Shows the menu anchored to the given element.
+   * @param {!Element} anchorElement
+   */
+  showAt: function(anchorElement) {
+    var rect = anchorElement.getBoundingClientRect();
+
+    // Ensure that the correct item is focused when the dialog is shown, by
+    // setting the 'autofocus' attribute.
+    this.focusedIndex_ = -1;
+    var nextOption = this.getNextOption_(1);
+
+    /** @suppress {checkTypes} */
+    (function() {
+      for (var option of this.options_)
+        option.removeAttribute('autofocus');
+    }.bind(this))();

[Dan Beam, 2016/10/14 03:51:13]

why can't you just use NodeList#forEach?
https://developer.mozilla.org/en-US/docs/Web/API/NodeList/forEach

options.forEach(function(option) {
  option.removeAttribute('autofocus');
});

if you can't (for some reason), this is cleaner:

}.call(this));

as is this:

(function(options) {
  for (var option of options)
    option.removeAttribute('autofocus');
})(this.options_);

[dpapad, 2016/10/14 17:39:23]

Because compiler.....
(ERROR) Error in: settings_action_menu.js
##
/usr/local/google/home/dpapad/workspace/chromium1/src/chrome/browser/resources/settings/settings_action_menu.js:123:
ERROR - Property forEach never defined on NodeList<Element>
##     this.options_.forEach(function(option) {
Done.

+
+    if (nextOption !== null)

[Dan Beam, 2016/10/14 03:51:13]

if (nextOption)

[dpapad, 2016/10/14 17:39:23]

Done.

+      nextOption.setAttribute('autofocus', true);
+
+    this.showModal();
+
+    if (new settings.DirectionDelegateImpl().isRtl()) {
+      var right = window.innerWidth - rect.left - this.offsetWidth;
+      this.style.right = right + 'px';
+    } else {
+      var left = rect.right - this.offsetWidth;
+      this.style.left = left + 'px';
+    }
+
+    // Attempt to show the menu starting from the top of the rectangle and
+    // extending downwards. If that does not fit within the window, fallback to
+    // starting from the bottom and extending upwards.
+    var top = rect.top + this.offsetHeight <= window.innerHeight ?
+        rect.top :
+        rect.bottom - this.offsetHeight - Math.max(
+            rect.bottom - window.innerHeight, 0);
+
+    this.style.top = top + 'px';
+  },
+});