MD Settings: Fix subpage navigation focus for bluetooth+internet

chrome/browser/resources/settings/internet_page/internet_page.js

 // Copyright 2015 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.
 
 /**
  * @fileoverview
  * 'settings-internet-page' is the settings page containing internet
  * settings.
  */
 Polymer({
@@ skipping 70 matching lines @@
      * List of third party VPN providers.
      * @type {!Array<!chrome.networkingPrivate.ThirdPartyVPNProperties>}
      * @private
      */
     thirdPartyVpnProviders_: {
       type: Array,
       value: function() {
         return [];
       }
     },
+
+    /**
+     * @type {!Map<string, string>}

[dpapad, 2017/04/25 21:25:19]

Ni: Use shorthand syntax,
@private {!Map<string, string>}

[stevenjb, 2017/04/25 23:32:53]

Done.

+     * @private
+     */
+    focusConfig_: {
+      type: Object,
+      value: function() {
+        return new Map();
+      },
+    },
   },
 
+  /** @private {string} Type of last detail page visited. */
+  detailType_: '',
+
   // Element event listeners
   listeners: {
     'device-enabled-toggled': 'onDeviceEnabledToggled_',
     'show-detail': 'onShowDetail_',
     'show-known-networks': 'onShowKnownNetworks_',
     'show-networks': 'onShowNetworks_',
   },
 
   // chrome.networkingPrivate listeners
   /** @private {Function} */
   onExtensionAddedListener_: null,
 
   /** @private {Function} */
   onExtensionRemovedListener_: null,
 
   /** @private {Function} */
   onExtensionDisabledListener_: null,
 
   /** @override */
   attached: function() {
-    this.onExtensionAddedListener_ = this.onExtensionAddedListener_ ||
-        this.onExtensionAdded_.bind(this);
+    this.onExtensionAddedListener_ =
+        this.onExtensionAddedListener_ || this.onExtensionAdded_.bind(this);
     chrome.management.onInstalled.addListener(this.onExtensionAddedListener_);
     chrome.management.onEnabled.addListener(this.onExtensionAddedListener_);
 
-    this.onExtensionRemovedListener_ = this.onExtensionRemovedListener_ ||
-        this.onExtensionRemoved_.bind(this);
+    this.onExtensionRemovedListener_ =
+        this.onExtensionRemovedListener_ || this.onExtensionRemoved_.bind(this);
     chrome.management.onUninstalled.addListener(
         this.onExtensionRemovedListener_);
 
     this.onExtensionDisabledListener_ = this.onExtensionDisabledListener_ ||
         this.onExtensionDisabled_.bind(this);
     chrome.management.onDisabled.addListener(this.onExtensionDisabledListener_);
 
     chrome.management.getAll(this.onGetAllExtensions_.bind(this));
 
     this.networkingPrivate.getGlobalPolicy(function(policy) {
       this.globalPolicy_ = policy;
     }.bind(this));
   },
 
   /** @override */
   detached: function() {
     chrome.management.onInstalled.removeListener(
         assert(this.onExtensionAddedListener_));
     chrome.management.onEnabled.removeListener(
         assert(this.onExtensionAddedListener_));
     chrome.management.onUninstalled.removeListener(
         assert(this.onExtensionRemovedListener_));
     chrome.management.onDisabled.removeListener(
         assert(this.onExtensionDisabledListener_));
   },
 
   /**
    * settings.RouteObserverBehavior
    * @param {!settings.Route} route
+   * @param {!settings.Route} oldRoute
    * @protected
    */
-  currentRouteChanged: function(route) {
+  currentRouteChanged: function(route, oldRoute) {
     if (route == settings.Route.INTERNET_NETWORKS) {
       // Handle direct navigation to the networks page,
       // e.g. chrome://settings/internet/networks?type=WiFi
       var queryParams = settings.getQueryParameters();
       var type = queryParams.get('type');
       if (type)
         this.subpageType_ = type;
     } else if (route == settings.Route.KNOWN_NETWORKS) {
       // Handle direct navigation to the known networks page,
       // e.g. chrome://settings/internet/knownNetworks?type=WiFi
       var queryParams = settings.getQueryParameters();
       var type = queryParams.get('type');
       if (type)
         this.knownNetworksType_ = type;
+    } else if (route != settings.Route.INTERNET && route.path != '/') {

[dpapad, 2017/04/25 21:25:19]

Is this equivalent to the following?

} else if (route != settings.Route.INTERNET && route != settings.Route.BASIC) {

[stevenjb, 2017/04/25 23:32:53]

Looks like it. Done.

+      // If we are navigating to a non internet section, do not set focus.
+      return;
+    }
+
+    // Focus the subpage arrow where appropriate.
+    if (oldRoute == settings.Route.NETWORK_DETAIL ||

[dpapad, 2017/04/25 21:25:19]

Is this equivalent to the following?

if (!settings.Route.INTERNET.contains(oldRoute))
  return;

// Do remaining stuff here

[stevenjb, 2017/04/25 23:32:53]

Handy. Yes. Done.

+        oldRoute == settings.Route.INTERNET_NETWORKS ||
+        oldRoute == settings.Route.KNOWN_NETWORKS) {
+      var selector;
+      if (route == settings.Route.INTERNET_NETWORKS) {

[dpapad, 2017/04/25 21:25:19]

Nit (optional): This can be compacted as follows:

// Comment here
var selector = route == settings.Route.INTERNET_NETWORKS ?
    '* /deep/ #networkList' :
    '* /deep/ #' + this.detailType_ + ' /deep/ .subpage-arrow';

[stevenjb, 2017/04/25 23:32:53]

Acknowledged.

+        // iron-list makes the correct timing to focus an item in the list
+        // very complicated, and the item may not exist, so just focus the
+        // entire list for now.
+        selector = '* /deep/ #networkList';
+      } else if (this.detailType_) {
+        selector = '* /deep/ #' + this.detailType_ + ' /deep/ .subpage-arrow';
+      }
+      if (selector && this.querySelector(selector))

[dpapad, 2017/04/25 21:25:19]

When would querySelector(selector) return nothing here?

[stevenjb, 2017/04/25 23:32:53]

If an Ethernet cable were unplugged while visiting an Ethernet detail page.

+        this.focusConfig_.set(oldRoute.path, selector);

[dpapad, 2017/04/25 21:25:19]

I don't fully understand the updates to the focusConfig_ map here, but it seems
that we might be leaving an obsolete entry in the map. Shouldn't there be an

else {
  this.focusConfig_.delete(oldRoute.path);
}

[stevenjb, 2017/04/25 23:32:53]

Done.

(I took this approach because of the dynamic nature of the Network section.
Originally I had ambitions to highlight the subpage button for the corresponding
network when returning from a details subpage, but iron-list behavior kicked my
ass so I am punting for now and just focusing the entire list in that case. We
still have they dynamic nature of the available types mentioned above though).

+      this.detailType_ = '';
     }
   },
 
   /**
    * Event triggered by a device state enabled toggle.
    * @param {!{detail: {enabled: boolean,
    *                    type: chrome.networkingPrivate.NetworkType}}} event
    * @private
    */
   onDeviceEnabledToggled_: function(event) {
     if (event.detail.enabled)
       this.networkingPrivate.enableNetworkType(event.detail.type);
     else
       this.networkingPrivate.disableNetworkType(event.detail.type);
   },
 
   /**
    * @param {!{detail: !CrOnc.NetworkStateProperties}} event
    * @private
    */
   onShowDetail_: function(event) {
+    this.detailType_ = event.detail.Type;
     var params = new URLSearchParams;
     params.append('guid', event.detail.GUID);
     params.append('type', event.detail.Type);
     if (event.detail.Name)
       params.append('name', event.detail.Name);
     settings.navigateTo(settings.Route.NETWORK_DETAIL, params);
   },
 
   /**
    * @param {!{detail: {type: string}}} event
    * @private
    */
   onShowNetworks_: function(event) {
+    this.detailType_ = event.detail.Type;
     var params = new URLSearchParams;
     params.append('type', event.detail.Type);
     this.subpageType_ = event.detail.Type;
     settings.navigateTo(settings.Route.INTERNET_NETWORKS, params);
   },
 
   /**
    * @return {string}
    * @private
    */
   getNetworksPageTitle_: function() {
     return this.i18n('OncType' + this.subpageType_);
   },
 
   /**
    * @param {!{detail: {type: string}}} event
    * @private
    */
   onShowKnownNetworks_: function(event) {
+    this.detailType_ = event.detail.Type;
     var params = new URLSearchParams;
     params.append('type', event.detail.Type);
     this.knownNetworksType_ = event.detail.type;
     settings.navigateTo(settings.Route.KNOWN_NETWORKS, params);
   },
 
   /**
    * Event triggered when the 'Add connections' div is tapped.
    * @param {Event} event
    * @private
@@ skipping 93 matching lines @@
   onExtensionDisabled_: function(extension) {
     this.onExtensionRemoved_(extension.id);
   },
 
   /**
    * @param {!chrome.networkingPrivate.DeviceStateProperties} deviceState
    * @return {boolean}
    * @private
    */
   deviceIsEnabled_: function(deviceState) {
-    return !!deviceState && deviceState.State ==
-        chrome.networkingPrivate.DeviceStateType.ENABLED;
+    return !!deviceState &&
+        deviceState.State == chrome.networkingPrivate.DeviceStateType.ENABLED;
   },
 
   /**
    * @param {!chrome.networkingPrivate.GlobalPolicy} globalPolicy
    * @return {boolean}
    */
   allowAddConnection_: function(globalPolicy) {
     return !globalPolicy.AllowOnlyPolicyNetworksToConnect;
   },
 
   /**
    * @param {!chrome.networkingPrivate.ThirdPartyVPNProperties} provider
    * @return {string}
    */
   getAddThirdParrtyVpnLabel_: function(provider) {
     return this.i18n('internetAddThirdPartyVPN', provider.ProviderName);
   }
 });