MD Settings: Implementing modal popup/action menus.

chrome/browser/resources/settings/settings_action_menu.js

+// 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_: 0,
+
+  /**
+   * Reference to the bound window's resize listener, such that it can be
+   * removed on detach.
+   * @private {?Function}
+   */
+  onWindowResize_: null,
+
+  /** override */
+  attached: function() {
+    this.setAttribute('tabindex', 0);

[Dan Beam, 2016/10/13 05:03:01]

nit:

hostAttributes: {
  tabindex: 0,
},

[dpapad, 2016/10/13 18:18:01]

Done.

+    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/13 05:03:01]

do you need to remove these events on detached as well?

[dpapad, 2016/10/13 18:18:01]

I don't think so. Firstly because ready() is only executed once, even if you
attach/detach multiple times AFAIU.
Secondly, even if the previous is not true (which I think it is), there is no
case where we are detaching a dialog and keeping it around to attach it later. 
We only attach it once, and call showModal/close as many times as necessary
(which does not trigger attached/ready again).

+      if (e.target == this) {
+        this.close();
+        e.stopPropagation();
+      }
+    }.bind(this));
+
+    this.addEventListener('keydown', function(e) {
+      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 {?HTMLElement} The next focusable option, taking into account
+   *     disabled/hidden attributes, or null if no focusable option exists.
+   * @private
+   */
+  getNextOption_: function(step) {
+    // Storing the focused index before searching, to avoid infinite loop if all
+    // options are disabled.
+    var previousFocusedIndex = this.focusedIndex_;
+
+    var getNextOptionRec_ = function() {
+      // Advance index.
+      this.focusedIndex_ = (this.focusedIndex_ + step) % this.options_.length;
+      if (this.focusedIndex_ == -1)
+        this.focusedIndex_ = this.options_.length - 1;

[Dan Beam, 2016/10/13 05:03:01]

var numOptions = this.options_.length;
this.focusedIndex_ = (numOptions + this.focusedIndex_ + step) % numOptions;

[dpapad, 2016/10/13 18:18:00]

Done.

+
+      // Adjust index in case item is disabled/hidden.
+      var nextOption = this.options_[this.focusedIndex_];
+      if (nextOption.disabled || nextOption.hidden) {
+        return this.focusedIndex_ != previousFocusedIndex ?
+            getNextOptionRec_() : null;
+      } else {

[Dan Beam, 2016/10/13 05:03:01]

don't use else after return

[dpapad, 2016/10/13 18:18:01]

Done.

+        return this.options_[this.focusedIndex_];
+      }
+    }.bind(this);
+
+    return getNextOptionRec_();

[Dan Beam, 2016/10/13 05:03:01]

var enabled = Array.from(this.options_).filter(function(option) {
  return !(option.disabled || option.hidden);
});
// check out this logic:
https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/cr_profil...

[dpapad, 2016/10/13 18:18:01]

Getting a filtered array of the enabled elements first would make it impossible
(without searching again) to retrieve the index of the next element in the
unfiltered this.options_ array, to assign it to this.focusedIndex_. I'd be happy
to pair program a non-recursive solution if you want, but as your suggestion
stands, I don't think it would work (or I am misunderstanding it).

[dpapad, 2016/10/13 18:58:25]

Implemented a non-recursive version in latest patch. Looks much simpler.

+  },
+
+  /**
+   * Shows the menu anchored to the given rectangle.
+   * @param {!ClientRect} rect

[Dan Beam, 2016/10/13 05:03:01]

can this just take an element instead of a clientrect?

[Dan Beam, 2016/10/13 19:01:24]

ping

+   */
+  showAtLocation: function(rect) {
+    // Ensure that the correct item is focused when the dialog is shown, by
+    // setting the 'autofocus' attribute.
+    this.focusedIndex_ = 0;

[Dan Beam, 2016/10/13 05:03:01]

can we just start this at -1 and call getNextOption(1)?

[dpapad, 2016/10/13 18:18:01]

Done.

+    var nextOption = this.options_[this.focusedIndex_];
+    if (nextOption.disabled || nextOption.hidden)
+      nextOption = this.getNextOption_(1);
+    for (var option of this.options_)
+      option.removeAttribute('autofocus');
+    if (nextOption !== null)
+      nextOption.setAttribute('autofocus', true);
+
+    this.showModal();
+
+    var directionDelegate = new settings.DirectionDelegateImpl();
+    if (directionDelegate.isRtl()) {

[Dan Beam, 2016/10/13 05:03:01]

maybe this?

if (new settings.DirectionDelegateImpl().isRtl()) {

[dpapad, 2016/10/13 18:18:01]

Done.

+      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';
+  },
+});