MD Settings: Refactor RoutableBehavior into MainPageBehavior

chrome/browser/resources/settings/settings_page/main_page_behavior.js

Index: chrome/browser/resources/settings/settings_page/main_page_behavior.js
diff --git a/chrome/browser/resources/settings/settings_page/main_page_behavior.js b/chrome/browser/resources/settings/settings_page/main_page_behavior.js
index ce711bfe13144e64092d39b4ec800812d6d58b02..5e408b2ec9c7e96a2709b7a7133e4003c5d440a2 100644
--- a/chrome/browser/resources/settings/settings_page/main_page_behavior.js
+++ b/chrome/browser/resources/settings/settings_page/main_page_behavior.js
@@ -20,10 +20,10 @@ function doWhenReady(readyTest, readyCallback) {
 }
 
 /**
- * Provides animations to expand and collapse individual sections in a page.
- * Expanded sections take up the full height of the container. At most one
- * section should be expanded at any given time.
- * @polymerBehavior Polymer.MainPageBehavior
+ * Responds to route changes by expanding, collapsing, or scrolling to sections
+ * on the page. Expanded sections take up the full height of the container. At
+ * most one section should be expanded at any given time.
+ * @polymerBehavior MainPageBehavior
  */
 var MainPageBehaviorImpl = {
   /** @type {?HTMLElement} The scrolling container. */
@@ -38,23 +38,49 @@ var MainPageBehaviorImpl = {
   },
 
   /**
-   * Hides or unhides the sections not being expanded.
-   * @param {string} sectionName The section to keep visible.
-   * @param {boolean} hidden Whether the sections should be hidden.
-   * @private
+   * @param {!settings.Route} newRoute
+   * @param {!settings.Route} oldRoute
    */
-  toggleOtherSectionsHidden_: function(sectionName, hidden) {
-    var sections = Polymer.dom(this.root).querySelectorAll(
-        'settings-section');
-    for (var section of sections)
-      section.hidden = hidden && (section.section != sectionName);
+  currentRouteChanged: function(newRoute, oldRoute) {

[michaelpg, 2016/08/06 03:20:37]

Same code, just moved up to the other public functions and given a proper /**
annotation */.

+    var newRouteIsSubpage = newRoute && newRoute.subpage.length;
+    var oldRouteIsSubpage = oldRoute && oldRoute.subpage.length;
+
+    if (!oldRoute && newRouteIsSubpage) {
+      // Allow the page to load before expanding the section. TODO(michaelpg):
+      // Time this better when refactoring settings-animated-pages.
+      setTimeout(function() {
+        var section = this.getSection_(newRoute.section);
+        if (section)
+          this.expandSection_(section);
+      }.bind(this));
+      return;
+    }
+
+    if (newRouteIsSubpage) {
+      if (!oldRouteIsSubpage || newRoute.section != oldRoute.section) {
+        var section = this.getSection_(newRoute.section);
+        if (section)
+          this.expandSection_(section);
+      }
+    } else {
+      if (oldRouteIsSubpage) {
+        var section = this.getSection_(oldRoute.section);
+        if (section)
+          this.collapseSection_(section);
+      }
+
+      // Scrolls to the section if this main page contains the route's section.
+      if (newRoute && newRoute.section && this.getSection_(newRoute.section))
+        this.scrollToSection_();
+    }
   },
 
   /**
    * Animates the card in |section|, expanding it to fill the page.
    * @param {!SettingsSectionElement} section
+   * @private
    */
-  expandSection: function(section) {
+  expandSection_: function(section) {
     // If another section's card is expanding, cancel that animation first.
     var expanding = this.$$('.expanding');
     if (expanding) {
@@ -71,7 +97,7 @@ var MainPageBehaviorImpl = {
         // When it resolves, collapse that section's card before expanding
         // this one.
         setTimeout(function() {
-          this.collapseSection(
+          this.collapseSection_(
               /** @type {!SettingsSectionElement} */(expanding));
           this.finishAnimation('section', function() {
             this.startExpandSection_(section);
@@ -129,10 +155,44 @@ var MainPageBehaviorImpl = {
   },
 
   /**
+   * Expands the card in |section| to fill the page.
+   * @param {!SettingsSectionElement} section
+   * @return {!Promise}
+   * @private
+   */
+  playExpandSection_: function(section) {

[michaelpg, 2016/08/06 03:20:37]

Exact same code, just moved up one.

+    // We must be attached.
+    assert(this.scroller);
+
+    var promise;
+    var animationConfig = section.animateExpand(this.scroller);
+    if (animationConfig) {
+      promise = this.animateElement('section', animationConfig.card,
+          animationConfig.keyframes, animationConfig.options);
+    } else {
+      promise = Promise.resolve();
+    }
+
+    var finished;
+    promise.then(function() {
+      finished = true;
+      this.style.margin = 'auto';
+    }.bind(this), function() {
+      // The animation was canceled; catch the error and continue.
+      finished = false;
+    }).then(function() {
+      section.cleanUpAnimateExpand(finished);
+    });
+
+    return promise;
+  },
+
+  /**
    * Animates the card in |section|, collapsing it back into its section.
    * @param {!SettingsSectionElement} section
+   * @private
    */
-  collapseSection: function(section) {
+  collapseSection_: function(section) {
     // If the section's card is still expanding, cancel the expand animation.
     if (section.classList.contains('expanding')) {
       if (this.animations['section']) {
@@ -141,7 +201,7 @@ var MainPageBehaviorImpl = {
         // The animation must have finished but its promise hasn't finished
         // resolving; try again asynchronously.
         this.async(function() {
-          this.collapseSection(section);
+          this.collapseSection_(section);
         });
       }
       return;
@@ -167,39 +227,6 @@ var MainPageBehaviorImpl = {
   },
 
   /**
-   * Expands the card in |section| to fill the page.
-   * @param {!SettingsSectionElement} section
-   * @return {!Promise}
-   * @private
-   */
-  playExpandSection_: function(section) {
-    // We must be attached.
-    assert(this.scroller);
-
-    var promise;
-    var animationConfig = section.animateExpand(this.scroller);
-    if (animationConfig) {
-      promise = this.animateElement('section', animationConfig.card,
-          animationConfig.keyframes, animationConfig.options);
-    } else {
-      promise = Promise.resolve();
-    }
-
-    var finished;
-    promise.then(function() {
-      finished = true;
-      this.style.margin = 'auto';
-    }.bind(this), function() {
-      // The animation was canceled; catch the error and continue.
-      finished = false;
-    }).then(function() {
-      section.cleanUpAnimateExpand(finished);
-    });
-
-    return promise;
-  },
-
-  /**
    * Collapses the card in |section| back to its normal position.
    * @param {!SettingsSectionElement} section
    * @return {!Promise}
@@ -228,21 +255,7 @@ var MainPageBehaviorImpl = {
     });
     return promise;
   },
-};
-
-
-/** @polymerBehavior */
-var MainPageBehavior = [
-  TransitionBehavior,
-  MainPageBehaviorImpl
-];
 
-
-/**
- * TODO(michaelpg): integrate slide animations.
- * @polymerBehavior RoutableBehavior
- */
-var RoutableBehaviorImpl = {
   /** @private */
   scrollToSection_: function() {
     doWhenReady(
@@ -256,39 +269,17 @@ var RoutableBehaviorImpl = {
         }.bind(this));
   },
 
-  /** @private */
-  currentRouteChanged: function(newRoute, oldRoute) {
-    var newRouteIsSubpage = newRoute && newRoute.subpage.length;
-    var oldRouteIsSubpage = oldRoute && oldRoute.subpage.length;
-
-    if (!oldRoute && newRouteIsSubpage) {
-      // Allow the page to load before expanding the section. TODO(michaelpg):
-      // Time this better when refactoring settings-animated-pages.
-      setTimeout(function() {
-        var section = this.getSection_(newRoute.section);
-        if (section)
-          this.expandSection(section);
-      }.bind(this));
-      return;
-    }
-
-    if (newRouteIsSubpage) {
-      if (!oldRouteIsSubpage || newRoute.section != oldRoute.section) {
-        var section = this.getSection_(newRoute.section);
-        if (section)
-          this.expandSection(section);
-      }
-    } else {
-      if (oldRouteIsSubpage) {
-        var section = this.getSection_(oldRoute.section);
-        if (section)
-          this.collapseSection(section);
-      }
-
-      // Scrolls to the section if this main page contains the route's section.
-      if (newRoute && newRoute.section && this.getSection_(newRoute.section))
-        this.scrollToSection_();
-    }
+  /**
+   * Hides or unhides the sections not being expanded.
+   * @param {string} sectionName The section to keep visible.
+   * @param {boolean} hidden Whether the sections should be hidden.
+   * @private
+   */
+  toggleOtherSectionsHidden_: function(sectionName, hidden) {

[michaelpg, 2016/08/06 03:20:37]

Again, exact same code from the top, just put down here with the private section
helper functions.

+    var sections = Polymer.dom(this.root).querySelectorAll(
+        'settings-section');
+    for (var section of sections)
+      section.hidden = hidden && (section.section != sectionName);
   },
 
   /**
@@ -303,10 +294,9 @@ var RoutableBehaviorImpl = {
   },
 };
 
-
 /** @polymerBehavior */
-var RoutableBehavior = [
-  MainPageBehavior,
+var MainPageBehavior = [
   settings.RouteObserverBehavior,
-  RoutableBehaviorImpl
+  TransitionBehavior,
+  MainPageBehaviorImpl,
 ];