MD Settings: adjust iron-list focus row behaviors.

chrome/browser/resources/settings/focus_row_behavior.js

Index: chrome/browser/resources/settings/focus_row_behavior.js
diff --git a/chrome/browser/resources/settings/focus_row_behavior.js b/chrome/browser/resources/settings/focus_row_behavior.js
new file mode 100644
index 0000000000000000000000000000000000000000..3f5a53cf554f544b19d3c436babee6993303d23e
--- /dev/null
+++ b/chrome/browser/resources/settings/focus_row_behavior.js
@@ -0,0 +1,170 @@
+// Copyright 2017 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.
+
+/**
+ * @param {{lastFocused: Object}} listItem

[Dan Beam, 2017/03/18 00:09:56]

why {lastFocused: Object} instead of just an Element?

[scottchen, 2017/03/18 01:05:47]

I believe this will make sure that the element passed in has "lastFocused"
defined as a property.

+ * @constructor
+ * @implements {cr.ui.FocusRow.Delegate}
+ */
+function FocusRowDelegate(listItem) {
+  this.listItem = listItem;

[Dan Beam, 2017/03/18 00:09:56]

can this be @private?

[scottchen, 2017/03/18 01:05:47]

Done.

+}
+
+FocusRowDelegate.prototype = {
+  /**
+   * @override
+   * @param {!cr.ui.FocusRow} row
+   * @param {!Event} e
+   */
+  onFocus: function(row, e) {
+    this.listItem.lastFocused = e.path[0];
+  },
+
+  /**
+   * @override
+   * @param {!cr.ui.FocusRow} row The row that detected a keydown.
+   * @param {!Event} e
+   * @return {boolean} Whether the event was handled.
+   */
+  onKeydown: function(row, e) {
+    // Prevent iron-list from changing the focus on enter.
+    if (e.key == 'Enter')
+      e.stopPropagation();
+
+    return false;
+  },
+};
+
+/**
+ * @param {!Element} root
+ * @param {cr.ui.FocusRow.Delegate} delegate
+ * @constructor
+ * @extends {cr.ui.FocusRow}
+ */
+function VirtualFocusRow(root, delegate) {
+  cr.ui.FocusRow.call(this, root, /* boundary */ null, delegate);
+}
+
+VirtualFocusRow.prototype = {
+  __proto__: cr.ui.FocusRow.prototype
+};
+
+
+/**
+ * Any element that is being used as an iron-list row item can extend this
+ * behavior, which encapsulates focus controls of mouse and keyboards.
+ * To use this behavior:
+ *    - The parent element should pass a "last-focused" attribute double-bound
+ *      to the row items, to track the last-focused element across rows.
+ *    - There must be a container in the extending element with the
+ *      [focus-row-container] attribute that contains all focusable controls.
+ *    - On each of the focusable controls, there must be a [focus-row-control]
+ *      attribute, and a [type=] attribute unique for each type of control.
+ *
+ * @polymerBehavior
+ */
+var FocusRowBehavior = {
+  properties: {
+    /** @private {VirtualFocusRow} */
+    row_: Object,
+
+    /** @private {boolean} */
+    mouseFocused_: Boolean,
+
+    /** @type {Element} */
+    lastFocused: {
+      type: Object,
+      notify: true,
+    },
+
+    /** @type {number} */
+    ironListTabIndex: {
+      type: Number,
+      observer: 'ironListTabIndexChanged_',
+    },
+  },
+
+  /** @override */
+  attached: function() {
+    this.classList.add('no-outline');
+
+    Polymer.RenderStatus.afterNextRender(this, function() {
+      var rowContainer = this.root.querySelector('[focus-row-container]');
+      assert(!!rowContainer);
+      this.row_ = new VirtualFocusRow(rowContainer, new FocusRowDelegate(this));
+      this.row_.makeActive(this.ironListTabIndex == 0);

[Dan Beam, 2017/03/18 00:09:56]

nit: this.ironListTabIndexChanged_();

[scottchen, 2017/03/18 01:05:47]

Done.

+      this.addItems_();
+
+      // Adding listeners asynchronously to reduce blocking time, since this
+      // behavior will be used by items in potentially long lists.
+      this.listen(this, 'focus', 'onFocus_');
+      this.listen(this, 'dom-change', 'onDomChange_');

[hcarmona, 2017/03/17 23:55:51]

I think you forgot to change it here

[scottchen, 2017/03/18 01:05:47]

Done.

+      this.listen(this, 'mousedown', 'onMouseDown_');
+      this.listen(this, 'blur', 'onBlur_');
+    });
+  },
+
+  /** @override */
+  detached: function() {
+    this.unlisten(this, 'focus', 'onFocus_');
+    this.unlisten(this, 'dom-change', 'addItems_');
+    this.unlisten(this, 'mousedown', 'onMouseDown_');
+    this.unlisten(this, 'blur', 'onBlur_');
+    if (this.row_)
+      this.row_.destroy();
+  },
+
+  /** @private */
+  addItems_: function() {
+    if (this.row_) {
+      this.row_.destroy();
+
+      var controls = this.root.querySelectorAll('[focus-row-control]');
+
+      for (var i = 0; i < controls.length; i++) {
+        this.row_.addItem(
+            controls[i].getAttribute('type'),
+            /** @type {HTMLElement} */ (controls[i]));
+      }
+    }
+  },
+
+  /** @private */
+  onFocus_: function() {
+    if (this.mouseFocused_) {
+      this.mouseFocused_ = false;  // Consume and reset flag.
+      return;
+    }
+
+    if (this.lastFocused)
+      this.row_.getEquivalentElement(this.lastFocused).focus();
+    else {

[Dan Beam, 2017/03/18 00:09:56]

needs curlies for if

if (this.lastFocused) {

} else {

[scottchen, 2017/03/18 01:05:47]

Style guide says "Omit curly braces for single-line if statements". Or does it
not apply if any of the if-else-statements in the chain has curlies?

And for future reference, what if it's:

if
 ...
else if
 ..
else if
 ..
else {
 ..
 ..
}

Do we then add curly for all the one-line if and else-ifs?

[hcarmona, 2017/03/20 18:32:50]

+1 to dbeam's comment. I don't think the coding standard is clear on this case.
But it looks a bit odd if only part of the if chain has curlies.

[scottchen, 2017/03/20 21:42:07]

Acknowledged.

+      var firstFocusable = assert(this.row_.getFirstFocusable());
+      firstFocusable.focus();
+    }
+
+    this.tabIndex = -1;
+  },
+
+  /** @private */
+  ironListTabIndexChanged_: function() {
+    if (this.row_)
+      this.row_.makeActive(this.ironListTabIndex == 0);
+  },
+
+  /** @private */
+  onDomChange_: function() {

[hcarmona, 2017/03/17 23:55:51]

You can also delete the function if you're not using it anymore

[scottchen, 2017/03/18 01:05:47]

Done.

+    this.addItems_();
+  },
+
+  /** @private */
+  onMouseDown_: function() {
+    this.mouseFocused_ = true;  // Set flag to not do any control-focusing.
+  },
+
+  /** @private */
+  onBlur_: function() {
+    this.mouseFocused_ = false;  // Reset flag since it's not active anymore.
+  }
+};