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

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

 (function() {
 
   var IOS = navigator.userAgent.match(/iP(?:hone|ad;(?: U;)? CPU) OS (\d+)/);
   var IOS_TOUCH_SCROLLING = IOS && IOS[1] >= 8;
   var DEFAULT_PHYSICAL_COUNT = 3;
   var HIDDEN_Y = '-10000px';
   var ITEM_WIDTH = 0;
   var ITEM_HEIGHT = 1;
   var SECRET_TABINDEX = -100;
 
@@ skipping 108 matching lines @@
        * This value can be computed using the position returned by `getBoundingClientRect()`
        * although it's preferred to use a constant value when possible.
        *
        * This property is useful when an external scrolling element is used and there's
        * some offset between the scrolling element and the list.
        * For example: a header is placed above the list.
        */
       scrollOffset: {
         type: Number,
         value: 0
+      },
+
+      /**
+       * If set to true, focus on an element will be preserved after rerender.
+       */
+      preserveFocus: {
+        type: Boolean,
+        value: false
       }
     },
 
     observers: [
       '_itemsChanged(items.*)',
       '_selectionEnabledChanged(selectionEnabled)',
       '_multiSelectionChanged(multiSelection)',
       '_setOverflow(scrollTarget, scrollOffset)'
     ],
 
@@ skipping 748 matching lines @@
         }
         el._templateInstance[this.as] = value;
       }
     },
 
     /**
      * Called when the items have changed. That is, ressignments
      * 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;
         this._virtualCount = this.items ? this.items.length : 0;
         this._collection = this.items ? Polymer.Collection.get(this.items) : null;
         this._physicalIndexForKey = {};
         this._firstVisibleIndexVal = null;
         this._lastVisibleIndexVal = null;
         this._physicalCount = this._physicalCount || 0;
         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;
 
         this._debounceTemplate(this._render);
       } 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

+      }
     },
 
     /**
      * @param {!Array<!PolymerSplice>} splices
      */
     _adjustVirtualIndex: function(splices) {
       splices.forEach(function(splice) {
         // deselect removed items
         splice.removed.forEach(this._removeItem, this);
         // We only need to care about changes happening above the current position
@@ skipping 666 matching lines @@
       this._focusPhysicalItem(this._focusedIndex + 1);
     },
 
     _didEnter: function(e) {
       this._focusPhysicalItem(this._focusedIndex);
       this._selectionHandler(e.detail.keyboardEvent);
     }
   });
 
 })();