Move settings-section animations into setting-section, make better

chrome/browser/resources/settings/settings_main/settings_main.js

Index: chrome/browser/resources/settings/settings_main/settings_main.js
diff --git a/chrome/browser/resources/settings/settings_main/settings_main.js b/chrome/browser/resources/settings/settings_main/settings_main.js
index 254f2614a20cbdb195c39564ef9c5b7137b97d2e..187ef2712579ea18c8c143e496dfdc9127b56593 100644
--- a/chrome/browser/resources/settings/settings_main/settings_main.js
+++ b/chrome/browser/resources/settings/settings_main/settings_main.js
@@ -10,12 +10,6 @@ Polymer({
   is: 'settings-main',
 
   properties: {
-    /** @private */
-    isAdvancedMenuOpen_: {
-      type: Boolean,
-      value: false,
-    },
-
     /**
      * Preferences state.
      */
@@ -35,28 +29,37 @@ Polymer({
     },
 
     /** @private */

[Dan Beam, 2016/07/12 02:01:54]

it might be worth making a separate CL for these computed properties changes

[michaelpg, 2016/07/20 22:10:57]

Done, see depends-on CL

-    showAdvancedPage_: {
+    showAboutPage_: {
       type: Boolean,
-      value: false,
+      computed: 'computeShowAboutPage_(currentRoute.page)',
     },
 
     /** @private */
-    showAdvancedToggle_: {
+    showBasicPage_: {
       type: Boolean,
-      value: true,
+      computed: 'computeShowBasicPage_(currentRoute.page, inSubpage_)',
     },
 
     /** @private */
-    showBasicPage_: {
+    showAdvancedPage_: {
       type: Boolean,
-      value: true,
+      computed: 'computeShowAdvancedPage_(' +
+          'currentRoute.page, advancedToggleExpanded_, inSubpage_)',
     },
 
     /** @private */
-    showAboutPage_: {
+    advancedToggleExpanded_: {
       type: Boolean,
       value: false,
     },
+
+    /**
+     * True if a section is expanded to show a subpage. Initialized using
+     * currentRoute. Toggles based on expand/collapse events so effects can
+     * be deferred until transitions complete.
+     * @private
+     */
+    inSubpage_: Boolean,
   },
 
   created: function() {
@@ -67,14 +70,13 @@ Polymer({
 
   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');
@@ -94,13 +96,15 @@ Polymer({
         function() {
           this.resolver_.resolve();
         }.bind(this));
+
+    this.inSubpage_ = !!this.currentRoute.subpage.length;
   },
 
   /**
    * @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';
   },
@@ -110,23 +114,49 @@ Polymer({
    * @private
    */
   currentRouteChanged_: function(newRoute) {
-    var isSubpage = !!newRoute.subpage.length;
+    // If we navigated to an advanced section, ensure the toggle is opened.
+    if (newRoute.page == 'advanced')
+      this.advancedToggleExpanded_ = true;
+  },
 
-    this.showAboutPage_ = newRoute.page == 'about';
+  /** @private */
+  subpageExpanded_: function() {
+    this.inSubpage_ = true;
+  },
 
-    this.showAdvancedToggle_ = !this.showAboutPage_ && !isSubpage;
+  /** @private */
+  subpageCollapsing_: function() {
+    this.inSubpage_ = false;
+  },
 
-    this.showBasicPage_ = this.showAdvancedToggle_ || newRoute.page == 'basic';
+  /** @private */
+  toggleAdvancedPage_: function() {
+    this.fire('toggle-advanced-page', !this.advancedToggleExpanded_);
+  },
 
-    this.showAdvancedPage_ =
-        (this.isAdvancedMenuOpen_ && this.showAdvancedToggle_) ||
-        newRoute.page == 'advanced';
+  /**
+   * @return {boolean}
+   * @private
+   */
+  computeShowAboutPage_: function(page) {
+    return page == 'about';
+  },
 
-    this.style.height = isSubpage ? '100%' : '';
+  /**
+   * @return {boolean}
+   * @private
+   */
+  computeShowBasicPage_: function(page, inSubpage) {
+    return page == 'basic' ||
+           page == 'advanced' && !inSubpage;

[Dan Beam, 2016/07/12 02:01:54]

please add ()s to clarify this

   },

[michaelpg, 2016/07/20 22:10:57]

Removed.

 
-  /** @private */
-  toggleAdvancedPage_: function() {
-    this.fire('toggle-advanced-page', !this.isAdvancedMenuOpen_);
+  /**
+   * @return {boolean}
+   * @private
+   */
+  computeShowAdvancedPage_: function(page, advancedToggleExpanded, inSubpage) {
+    return page == 'advanced' ||
+           page == 'basic' && advancedToggleExpanded && !inSubpage;

[Dan Beam, 2016/07/12 02:01:54]

same

[michaelpg, 2016/07/20 22:10:57]

Acknowledged.

   },
 });