MD Settings: refactor some main page properties, fix Advanced toggle

chrome/browser/resources/settings/settings_main/settings_main.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-main' displays the selected settings page.
  */
 Polymer({
   is: 'settings-main',
 
   properties: {
-    /** @private */
-    isAdvancedMenuOpen_: {
-      type: Boolean,
-      value: false,
-    },
-
     /**
      * Preferences state.
      */
     prefs: {
       type: Object,
       notify: true,
     },
 
     /**
      * The current active route.
      * @type {!SettingsRoute}
      */
     currentRoute: {
       type: Object,
       notify: true,
       observer: 'currentRouteChanged_',
     },
 
     /** @private */
-    showAdvancedPage_: {
+    advancedToggleExpanded_: {
       type: Boolean,
       value: false,
     },
 
     /** @private */
-    showAdvancedToggle_: {
-      type: Boolean,
-      value: true,
-    },
+    inSubpage_: Boolean,
 
     /** @private */
-    showBasicPage_: {
-      type: Boolean,
-      value: true,
-    },
+    showAboutPage_: Boolean,
 
     /** @private */
-    showAboutPage_: {
-      type: Boolean,
-      value: false,
-    },
+    showBasicPage_: Boolean,
+
+    /** @private */
+    showAdvancedPage_: Boolean,
 
     toolbarSpinnerActive: {
       type: Boolean,
       value: false,
       notify: true,
     },
   },
 
   /** @override */
   created: function() {
     /** @private {!PromiseResolver} */
     this.resolver_ = new PromiseResolver;
     settings.main.rendered = this.resolver_.promise;
   },
 
   /** @override */
   ready: function() {
     settings.getSearchManager().setCallback(function(isRunning) {
       this.toolbarSpinnerActive = isRunning;
     }.bind(this));
   },
 
   /** @override */
   attached: function() {
     document.addEventListener('toggle-advanced-page', function(e) {
-      this.showAdvancedPage_ = e.detail;
-      this.isAdvancedMenuOpen_ = e.detail;
+      this.advancedToggleExpanded_ = e.detail;
       this.currentRoute = {
-        page: this.isAdvancedMenuOpen_ ? 'advanced' : 'basic',
+        page: this.advancedToggleExpanded_ ? 'advanced' : 'basic',
         section: '',
         subpage: [],
       };
-      if (this.showAdvancedPage_) {
+      if (this.advancedToggleExpanded_) {
         doWhenReady(
             function() {
               var advancedPage = this.$$('settings-advanced-page');
               return !!advancedPage && advancedPage.scrollHeight > 0;
             }.bind(this),
             function() {
               this.$$('#toggleContainer').scrollIntoView();
             }.bind(this));
       }
     }.bind(this));
 
     doWhenReady(
         function() {
           var basicPage = this.$$('settings-basic-page');
           return !!basicPage && basicPage.scrollHeight > 0;
         }.bind(this),
         function() {
           this.resolver_.resolve();
         }.bind(this));
   },
 
   /**
    * @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';
   },
 
   /**
    * @param {!SettingsRoute} newRoute
    * @private
    */
   currentRouteChanged_: function(newRoute) {
-    var isSubpage = !!newRoute.subpage.length;
+    this.inSubpage_ = newRoute.subpage.length > 0;
+    this.showAboutPage_ = newRoute.page == 'about';

[dpapad, 2016/07/20 21:30:08]

Drive-by question: Would it make more sense to package the three booleans to a
single Polymer property?

showingPages_: {
  type: Boolean,
  value: {basic: false, advanced: false, about: false},
}

This way you can replace all 3 booleans at once, instead of flipping them one by
one. Maybe that invokes less DOM updates?

[michaelpg, 2016/07/20 22:25:39]

Fine idea, done.

+    if (this.showAboutPage_) {
+      this.showBasicPage_ = this.showAdvancedPage_ = false;
+    } else if (newRoute.page == 'basic') {
+      this.showBasicPage_ = true;
+      this.showAdvancedPage_ = !this.inSubpage_ && this.advancedToggleExpanded_;
+    } else if (newRoute.page == 'advanced') {
+      this.showBasicPage_ = !this.inSubpage_;
+      this.showAdvancedPage_ = this.advancedToggleExpanded_ = true;
+    } else {
+      assertNotReached('Invalid page ' + newRoute.page);
+    }
 
-    this.showAboutPage_ = newRoute.page == 'about';
-
-    this.showAdvancedToggle_ = !this.showAboutPage_ && !isSubpage;
-
-    this.showBasicPage_ = this.showAdvancedToggle_ || newRoute.page == 'basic';
-
-    this.showAdvancedPage_ =
-        (this.isAdvancedMenuOpen_ && this.showAdvancedToggle_) ||
-        newRoute.page == 'advanced';
-
-    this.style.height = isSubpage ? '100%' : '';
+    this.style.height = this.inSubpage_ ? '100%' : '';
   },
 
   /** @private */
   toggleAdvancedPage_: function() {
-    this.fire('toggle-advanced-page', !this.isAdvancedMenuOpen_);
+    this.fire('toggle-advanced-page', !this.advancedToggleExpanded_);
   },
 
   /**
    * Navigates to the default search page (if necessary).
    * @private
    */
   ensureInDefaultSearchPage_: function() {
     if (this.currentRoute.page != 'basic' ||
         this.currentRoute.section != '' ||
         this.currentRoute.subpage.length != 0) {
@@ skipping 16 matching lines @@
           query, assert(this.$$('settings-basic-page')));
     }.bind(this), 0);
 
     this.showAdvancedPage_ = true;
     setTimeout(function() {
       settings.getSearchManager().search(
           query, assert(this.$$('settings-advanced-page')));
     }.bind(this), 0);
   },
 });