Settings Router Refactor: Remove route.page legacy property.

chrome/browser/resources/settings/settings_menu/settings_menu.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-menu' shows a menu with a hardcoded set of pages and subpages.
  */
 Polymer({
   is: 'settings-menu',
@@ skipping 30 matching lines @@
     }.bind(this));
 
     this.$.advancedPage.addEventListener('paper-submenu-open', function() {
       this.fire('toggle-advanced-page', true);
     }.bind(this));
 
     this.$.advancedPage.addEventListener('paper-submenu-close', function() {
       this.fire('toggle-advanced-page', false);
     }.bind(this));
 
-    this.fire('toggle-advanced-page', this.currentRoute.page == 'advanced');
+    this.fire('toggle-advanced-page',
+              settings.Route.ADVANCED.contains(this.currentRoute));
   },
 
   /**
    * @param {!settings.Route} newRoute
    * @private
    */
   currentRouteChanged_: function(newRoute) {
     // Sync URL changes to the side nav menu.
-
-    if (newRoute.page == 'advanced') {
+    this.$.advancedMenu.selected = this.currentRoute.path;

[michaelpg, 2016/07/29 20:28:53]

if this works, can we just use (one-way) data binding in the HTML for these?

[tommycli, 2016/07/29 21:04:40]

Done. Nice

[michaelpg, 2016/07/29 21:32:56]

Awesome, we got rid of an entire currentRouteChanged!

+    this.$.basicMenu.selected = this.currentRoute.path;
       assert(!this.pageVisibility ||
              this.pageVisibility.advancedSettings !== false);
-      this.$.advancedMenu.selected = this.currentRoute.section;
-      this.$.basicMenu.selected = null;
-    } else if (newRoute.page == 'basic') {
-      this.$.advancedMenu.selected = null;
-      this.$.basicMenu.selected = this.currentRoute.section;
-    } else {
-      this.$.advancedMenu.selected = null;
-      this.$.basicMenu.selected = null;
-    }
   },
 
   /**
    * @param {!Node} target
    * @private
    */
   ripple_: function(target) {
     var ripple = document.createElement('paper-ripple');
     ripple.addEventListener('transitionend', function() {
       ripple.remove();
     });
     target.appendChild(ripple);
     ripple.downAction();
     ripple.upAction();
   },
 
   /**
    * @param {!Event} event
    * @private
    */
   openPage_: function(event) {
     this.ripple_(/** @type {!Node} */(event.currentTarget));
-    var page = event.currentTarget.parentNode.dataset.page;
-    if (!page)
-      return;
 
-    var section = event.currentTarget.dataset.section;
-    // TODO(tommycli): Use Object.values once Closure compilation supports it.
-    var matchingKey = Object.keys(settings.Route).find(function(key) {
-      var route = settings.Route[key];
-      // TODO(tommycli): Remove usage of page, section, and subpage properties.
-      // Routes should be somehow directly bound to the menu elements.
-      return route.page == page && route.section == section &&
-          route.subpage.length == 0;
-    });
-
-    if (matchingKey)
-      settings.navigateTo(settings.Route[matchingKey]);
+    var route = settings.getRouteForPath(event.currentTarget.dataset.path);

[michaelpg, 2016/07/29 20:28:53]

unfortunate that the menu has to do this -- settings.navigateTo(path) would be
nice to have...

[tommycli, 2016/07/29 21:04:40]

Yeah I know. Ideally I'd prefer to do

settings.navigateTo(event.currentTarget.dataset.route), but I wasn't able to
bind an object to the div element.

+    assert(route, 'settings-menu has an an entry with an invalid path');
+    settings.navigateTo(route);
   },
 
   /**
    * @param {boolean} opened Whether the menu is expanded.
    * @return {string} Which icon to use.
    * @private
    * */
   arrowState_: function(opened) {
     return opened ? 'settings:arrow-drop-up' : 'cr:arrow-drop-down';
   },
 });