Apply preserve-focus to network-list.

This is a more robust method of re-applying focus to an
iron-list after the underlying data changes.   The current
method breaks during oobe and updates the list more often
than needed.

Bug: 1031994, 1032265
Change-Id: Ibc115fa099f80c1da0fd872f00a84472b9ff8c20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1966581
Commit-Queue: Jon Mann <jonmann@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#725421}

ui/webui/resources/cr_components/chromeos/network/BUILD.gn

@@ -179,6 +179,7 @@ js_library("network_list") {
   deps = [
     ":network_list_types",
     "//ui/webui/resources/cr_elements:cr_scrollable_behavior",
+    "//ui/webui/resources/js:list_property_update_behavior",
   ]
 }
 
@@ -188,6 +189,7 @@ js_library("network_list_item") {
     "//ui/webui/resources/cr_elements/policy:cr_policy_network_behavior_mojo",
     "//ui/webui/resources/js:assert",
     "//ui/webui/resources/js:i18n_behavior",
+    "//ui/webui/resources/js/cr/ui:focus_row_behavior",
   ]
 }
 

ui/webui/resources/cr_components/chromeos/network/network_list.html

@@ -5,6 +5,8 @@
 <link rel="import" href="chrome://resources/cr_elements/shared_style_css.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/iron-flex-layout-classes.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-list/iron-list.html">
+<link rel="import" href="chrome://resources/html/list_property_update_behavior.html">
+
 
 <dom-module id="network-list">
   <template>
@@ -35,13 +37,18 @@
     </style>
     <div id="container" class="layout vertical flex" scrollable
         no-bottom-scroll-border$="[[noBottomScrollBorder]]">
-      <iron-list id="networkList" selection-enabled items="[[listItems_]]"
-          scroll-target="container" selected-item="{{selectedItem}}">
+      <iron-list id="networkList" items="[[listItems_]]"
+          scroll-target="container" preserve-focus>
         <template>
           <network-list-item item="[[item]]"
               show-technology-badge="[[showTechnologyBadge]]"
-              show-buttons="[[showButtons]]" tabindex$="[[tabIndex]]"
-              activation-unavailable="[[activationUnavailable]]">
+              show-buttons="[[showButtons]]"
+              tabindex$="[[tabIndex]]"
+              focus-row-index="[[index]]"
+              iron-list-tab-index="[[tabIndex]]"
+              activation-unavailable="[[activationUnavailable]]"
+              last-focused="{{lastFocused_}}"
+              list-blurred="{{listBlurred_}}">
           </network-list-item>
         </template>
       </iron-list>

ui/webui/resources/cr_components/chromeos/network/network_list.js

@@ -67,82 +67,48 @@ Polymer({
         return [];
       },
     },
+
+    /**
+     * Used by FocusRowBehavior to track the last focused element on a row.
+     * @private
+     */
+    lastFocused_: Object,
+
+    /**
+     * Used by FocusRowBehavior to track if the list has been blurred.
+     * @private
+     */
+    listBlurred_: Boolean,
   },
 
-  behaviors: [CrScrollableBehavior],
+  behaviors: [CrScrollableBehavior, ListPropertyUpdateBehavior],
 
   observers: ['updateListItems_(networks, customItems)'],
 
   /** @private {boolean} */
   focusRequested_: false,
 
-  /**
-   * GUID of the focused network before the list is updated.  This
-   * is used to re-apply focus to the same network if possible.
-   * @private {string}
-   */
-  focusedGuidBeforeUpdate_: '',
-
-  /**
-   * Index of the focused network before the list is updated.  This
-   * is used to re-apply focus when the previously focused network
-   * is no longer listed.
-   * @private {number}
-   */
-  focusedIndexBeforeUpdate_: -1,
-
   focus: function() {
     this.focusRequested_ = true;
     this.focusFirstItem_();
   },
 
-  /** @private */
-  saveFocus_: function() {
-    if (this.shadowRoot.activeElement &&
-        this.shadowRoot.activeElement.is === 'network-list-item') {
-      const focusedNetwork = /** @type {!NetworkList.NetworkListItem} */ (
-          this.shadowRoot.activeElement);
-      if (focusedNetwork.item && focusedNetwork.item.guid) {
-        this.focusedGuidBeforeUpdate = focusedNetwork.item.guid;
-        this.focusedIndexBeforeUpdate_ = this.listItems_.findIndex(
-            n => n.guid === this.focusedGuidBeforeUpdate);
-        return;
-      }
-    }
-    this.focusedGuidBeforeUpdate = '';
-    this.focusedIndexBeforeUpdate_ = -1;
-  },
-
-  /** @private */
-  restoreFocus_: function() {
-    if (this.focusedGuidBeforeUpdate) {
-      let currentIndex = this.listItems_.findIndex(
-          (n) => n.guid === this.focusedGuidBeforeUpdate);
-      if (currentIndex < 0) {
-        currentIndex = this.focusedIndexBeforeUpdate_ < this.listItems_.length ?
-            this.focusedIndexBeforeUpdate_ :
-            0;
-      }
-      this.$.networkList.focusItem(currentIndex);
-    } else if (this.focusRequested_) {
-      this.async(function() {
-        this.focusFirstItem_();
-      });
-    }
-  },
-
   /** @private */
   updateListItems_: function() {
-    this.saveScroll(this.$.networkList);
-    this.saveFocus_();
+    if (!this.listItems_) {
+      // TODO(https://crbug.com/1034794) Determine how |listItems_| can be
+      // undefined when initializing OOBE.
+      return;
+    }
     const beforeNetworks =
         this.customItems.filter(n => n.showBeforeNetworksList == true);
     const afterNetworks =
         this.customItems.filter(n => n.showBeforeNetworksList == false);
-    this.listItems_ = beforeNetworks.concat(this.networks, afterNetworks);
-    this.restoreScroll(this.$.networkList);
+    const newList = beforeNetworks.concat(this.networks, afterNetworks);
+
+    this.updateList('listItems_', item => item.guid, newList);
+
     this.updateScrollableContents();
-    this.restoreFocus_();
   },
 
   /** @private */
@@ -155,28 +121,4 @@ Polymer({
     item.focus();
     this.focusRequested_ = false;
   },
-
-  /**
-   * Use iron-list selection (which is not the same as focus) to trigger
-   * tap (requires selection-enabled) or keyboard selection.
-   * @private
-   */
-  selectedItemChanged_: function() {
-    if (this.selectedItem) {
-      this.onItemAction_(this.selectedItem);
-    }
-  },
-
-  /**
-   * @param {!NetworkList.NetworkListItemType} item
-   * @private
-   */
-  onItemAction_: function(item) {
-    if (item.hasOwnProperty('customItemName')) {
-      this.fire('custom-item-selected', item);
-    } else {
-      this.fire('selected', item);
-      this.focusRequested_ = true;
-    }
-  },
 });

ui/webui/resources/cr_components/chromeos/network/network_list_item.html

@@ -9,6 +9,7 @@
 <link rel="import" href="chrome://resources/cr_elements/shared_style_css.html">
 <link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html">
 <link rel="import" href="chrome://resources/html/i18n_behavior.html">
+<link rel="import" href="chrome://resources/html/cr/ui/focus_row_behavior.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/iron-flex-layout-classes.html">
 
 <dom-module id="network-list-item">
@@ -20,6 +21,7 @@
       }
 
       #divOuter {
+        height: 100%;
         padding-inline-end: var(--cr-icon-ripple-padding);
       }
 
@@ -55,39 +57,59 @@
       cr-policy-indicator {
         padding: 0 var(--cr-controlled-by-spacing);
       }
+
+      #wrapper {
+        height: 100%;
+      }
     </style>
-    <div id="divOuter"
-        class="layout horizontal center flex" actionable>
-      <template is="dom-if" if="[[networkState]]">
-        <network-icon is-list-item
-            show-technology-badge="[[showTechnologyBadge]]"
-            network-state="[[networkState]]">
-        </network-icon>
-      </template>
-      <template is="dom-if" if="[[item.polymerIcon]]">
-        <iron-icon icon="[[item.polymerIcon]]"></iron-icon>
-      </template>
-      <div id="divText" class="layout horizontal flex">
-        <div aria-hidden="true">[[getItemName_(item)]]</div>
-        <div id="networkStateText"
-            hidden$="[[!isStateTextVisible_(networkState)]]"
-            active$="[[isStateTextActive_(networkState,
-                         activationUnavailable)]]">
-          [[getNetworkStateText_(networkState, activationUnavailable)]]
+    <div id="wrapper" focus-row-container
+         class="layout horizontal center flex">
+      <div id="divOuter"
+           class="layout horizontal center flex"
+           actionable
+           focus-row-control
+           selectable
+           aria-label$="[[rowLabel]]"
+           role="button"
+           focus-type="rowWrapper"
+           on-keydown="onKeydown_"
+           on-click="onSelected_">
+        <template is="dom-if" if="[[networkState]]">
+          <network-icon is-list-item
+              show-technology-badge="[[showTechnologyBadge]]"
+              network-state="[[networkState]]">
+          </network-icon>
+        </template>
+        <template is="dom-if" if="[[item.polymerIcon]]">
+          <iron-icon icon="[[item.polymerIcon]]"></iron-icon>
+        </template>
+        <div id="divText" class="layout horizontal flex">
+          <div aria-hidden="true">[[getItemName_(item)]]</div>
+          <div id="networkStateText"
+              hidden$="[[!isStateTextVisible_(networkState)]]"
+              active$="[[isStateTextActive_(networkState,
+                           activationUnavailable)]]">
+            [[getNetworkStateText_(networkState, activationUnavailable)]]
+          </div>
         </div>
+        <template is="dom-if" if="[[isPolicySource(networkState.source)]]">
+          <cr-policy-indicator indicator-type="[[getIndicatorTypeForSource(
+              networkState.source)]]">
+          </cr-policy-indicator>
+        </template>
+        <template is="dom-if" if="[[isSubpageButtonVisible_(networkState, showButtons)]]" restamp>
+          <div>
+            <cr-icon-button class="subpage-arrow"
+                id="subpage-button"
+                on-click="onSubpageArrowClick_"
+                tabindex$="[[tabindex]]"
+                aria-label$="[[buttonLabel]]"
+                focus-row-control
+                focus-type="subpageButton">
+            </cr-icon-button>
+          </div>
+        </template>
       </div>
-      <template is="dom-if" if="[[isPolicySource(networkState.source)]]">
-        <cr-policy-indicator indicator-type="[[getIndicatorTypeForSource(
-            networkState.source)]]">
-        </cr-policy-indicator>
-      </template>
-      <template is="dom-if"
-          if="[[isSubpageButtonVisible_(networkState, showButtons)]]" restamp>
-        <cr-icon-button class="subpage-arrow" id="subpage-button"
-            on-click="onSubpageArrowClick_" tabindex$="[[tabindex]]"
-            aria-label$="[[buttonLabel]]">
-        </cr-icon-button>
-      </template>
     </div>
   </template>
   <script src="network_list_item.js"></script>

ui/webui/resources/cr_components/chromeos/network/network_list_item.js

@@ -13,6 +13,7 @@ Polymer({
   behaviors: [
     CrPolicyNetworkBehaviorMojo,
     I18nBehavior,
+    cr.ui.FocusRowBehavior,
   ],
 
   properties: {
@@ -45,7 +46,6 @@ Polymer({
     tabindex: {
       type: Number,
       value: -1,
-      reflectToAttribute: true,
     },
 
     /**
@@ -53,11 +53,10 @@ Polymer({
      * added as an attribute on this top-level network-list-item, and can
      * be used by any sub-element which applies it.
      */
-    ariaLabel: {
+    rowLabel: {
       type: String,
       notify: true,
-      reflectToAttribute: true,
-      computed: 'getAriaLabel_(item, networkState)',
+      computed: 'getRowLabel_(item, networkState)',
     },
 
     buttonLabel: {
@@ -155,15 +154,16 @@ Polymer({
    * @return {string}
    * @private
    */
-  getAriaLabel_: function() {
+  getRowLabel_: function() {
     const NetworkType = chromeos.networkConfig.mojom.NetworkType;
     const OncSource = chromeos.networkConfig.mojom.OncSource;
     const SecurityType = chromeos.networkConfig.mojom.SecurityType;
     const status = this.getNetworkStateText_();
     const isManaged = this.item.source === OncSource.kDevicePolicy ||
         this.item.source === OncSource.kUserPolicy;
-    const index = this.parentElement.items.indexOf(this.item) + 1;
-    const total = this.parentElement.items.length;
+    const list = this.parentElement;
+    const index = list.items.indexOf(this.item) + 1;
+    const total = list.items.length;
     switch (this.item.type) {
       case NetworkType.kCellular:
         if (isManaged) {
@@ -324,13 +324,11 @@ Polymer({
   onKeydown_: function(event) {
     // The only key event handled by this element is pressing Enter when the
     // subpage arrow is focused.
-    if (event.key != 'Enter' ||
-        !this.isSubpageButtonVisible_(this.networkState, this.showButtons) ||
-        this.$$('#subpage-button') != this.shadowRoot.activeElement) {
+    if (event.key != 'Enter' && event.key != ' ') {
       return;
     }
 
-    this.fireShowDetails_(event);
+    this.onSelected_(event);
 
     // The default event for pressing Enter on a focused button is to simulate a
     // click on the button. Prevent this action, since it would navigate a
@@ -339,6 +337,22 @@ Polymer({
     event.preventDefault();
   },
 
+  /**
+   * @param {!Event} event
+   * @private
+   */
+  onSelected_: function(event) {
+    if (this.isSubpageButtonVisible_(this.networkState, this.showButtons) &&
+        this.$$('#subpage-button') == this.shadowRoot.activeElement) {
+      this.fireShowDetails_(event);
+    } else if (this.item.hasOwnProperty('customItemName')) {
+      this.fire('custom-item-selected', this.item);
+    } else {
+      this.fire('selected', this.item);
+      this.focusRequested_ = true;
+    }
+  },
+
   /**
    * @param {!MouseEvent} event
    * @private

ui/webui/resources/cr_components/chromeos/network/network_select.js

@@ -125,7 +125,7 @@ Polymer({
     for (const network of networkList.children) {
       if (network.is === 'network-list-item' &&
           network.$$('#divText').children[0].innerText === name) {
-        return network;
+        return network.shadowRoot.getElementById('divOuter');
       }
     }
     return null;