MD Settings: Fix iron-list losing focus when items change.

third_party/polymer/v1_0/components-chromium/iron-list/iron-list-extracted.js

Index: third_party/polymer/v1_0/components-chromium/iron-list/iron-list-extracted.js
diff --git a/third_party/polymer/v1_0/components-chromium/iron-list/iron-list-extracted.js b/third_party/polymer/v1_0/components-chromium/iron-list/iron-list-extracted.js
index 9e9ec02f9c9c4286011e6eb37a6201fe99ae70aa..eb492f1e3f8a1e8cbfa63f0337238cd2d3db1c86 100644
--- a/third_party/polymer/v1_0/components-chromium/iron-list/iron-list-extracted.js
+++ b/third_party/polymer/v1_0/components-chromium/iron-list/iron-list-extracted.js
@@ -126,6 +126,14 @@
       scrollOffset: {
         type: Number,
         value: 0
+      },
+
+      /**
+       * If set to true, focus on an element will be preserved after rerender.
+       */
+      preserveFocus: {
+        type: Boolean,
+        value: false
       }
     },
 
@@ -894,6 +902,15 @@
      * to `items`, splices or updates to a single item.
      */
     _itemsChanged: function(change) {
+      var rendering = ['items', 'items.splices'].includes(change.path);

[dpapad, 2017/05/23 22:19:43]

Nit (optional): Maybe use a regex instead (to avoid creating an array)?

var rendering = /^items(\.splices){0,1}$/.test(change.path)

[scottchen, 2017/05/24 21:34:35]

Done.

+      var lastFocusedIndex, focusedElement;
+      if (rendering && this.preserveFocus) {
+        lastFocusedIndex = this._focusedIndex;
+        focusedElement = this.querySelector('* /deep/ *:focus');
+      }
+
+      var preservingFocus = rendering && this.preserveFocus && focusedElement;
+
       if (change.path === 'items') {
         this._virtualStart = 0;
         this._physicalTop = 0;
@@ -906,12 +923,11 @@
         this._physicalItems = this._physicalItems || [];
         this._physicalSizes = this._physicalSizes || [];
         this._physicalStart = 0;
-        if (this._scrollTop > this._scrollOffset) {
+        if (this._scrollTop > this._scrollOffset && !preservingFocus) {
           this._resetScrollPosition(0);
         }
         this._removeFocusedItem();
         this._debounceTemplate(this._render);
-
       } else if (change.path === 'items.splices') {
         this._adjustVirtualIndex(change.value.indexSplices);
         this._virtualCount = this.items ? this.items.length : 0;
@@ -920,6 +936,17 @@
       } else {
         this._forwardItemPath(change.path.split('.').slice(1).join('.'), change.value);
       }
+
+      // If the list was in focus when updated, preserve the focus on item.
+      if (preservingFocus) {
+        Polymer.dom.flush();
+        focusedElement.blur(); // paper- elements breaks when focused twice.
+        this._focusPhysicalItem(
+            Math.min(this.items.length - 1, lastFocusedIndex));
+        if (!this._isIndexVisible(this._focusedIndex)) {
+          this.scrollToIndex(this._focusedIndex);
+        }

[stevenjb, 2017/05/09 17:14:00]

Can we do something like:

var idx = Math.min(this.items.length - 1, lastFocusedIndex);
if (focusedElement == this._physicalItems[this._getPhysicalIndex(idx)]) {
  if (!this._isIndexRendered(idx))
    this.scrollToIndex(idx);
} else {
  this._focusPhysicalItem(idx);
}

That would avoid having to blur() then focus() the item if it's still present,
which might have side effects?

Also, it looks like _focusPhysicalItem already calls scrollToIndex?

[scottchen, 2017/05/22 22:57:31]

This would not work as intended:

  focusedElement == this._physicalItems[this._getPhysicalIndex(idx)]

Because sometimes "focusedElement" is an element within the list-item, not the
list item itself. (e.g. when exiting a cr-action-menu, we call anchor.focus(),
anchor being the paper-icon-button). In such cases, there's not a good way to
detect which list-item's descendants has focus.

However, dpapad@ did point out that a new pseudo :focus-within selector  is
implemented in tip of tree, so I will attempt to simplify this code using that.

[stevenjb, 2017/05/22 23:14:54]

It's been a while, but I think I was being hand-wavy with "if (focusedElement ==
this._physicalItems[this._getPhysicalIndex(idx)])"; 

I just meant to suggest that we should be able to detect whether the focused
item corresponds to "lastFocusedIndex" and just call scrollToIndex, instead of
the extra blur/focus.

That said if there's a better approach, by all means explore that!

[scottchen, 2017/05/23 21:43:49]

focusedItem at this point is already reset to 0 (this is what caused the bug in
the first place), so it doesn't correspond to anything useful at this point.

focusedElement is just some dom element that was focused by javascript, it
doesn't have much to do with what iron-list thinks the focusedItem is, so
scrolling to the item that contains it isn't really helpful if iron-list still
thinks focusedItem = items[0]. 

This is also why its required to call this._focusPhysicalItem() to actually
correctly set iron-list's notion of what the focusedItem is. 

focusedElement is only being retrieved for the sole purpose of calling blur() on
it, which is required just because paper- controls break if you try to focus it
while its already focused.
I looked at :focus-within more, and unfortunately that wouldn't benefit us in
this case, because I would still need the focusedElement to call blur() on it.

[stevenjb, 2017/05/23 21:53:47]

OK, lgtm then

+      }
     },
 
     /**