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

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 4b6442c44c82c15ddccf7898de684603d1012755..fd4298f3181079cca7dc5d43aa7a9c9c55239bec 100644
--- a/chrome/browser/resources/settings/settings_page/main_page_behavior.js
+++ b/chrome/browser/resources/settings/settings_page/main_page_behavior.js
@@ -2,10 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Fast out, slow in.
-var EASING_FUNCTION = 'cubic-bezier(0.4, 0, 0.2, 1)';
-var EXPAND_DURATION = 350;
-
 /**
  * Calls |readyTest| repeatedly until it returns true, then calls
  * |readyCallback|.
@@ -36,11 +32,9 @@ var MainPageBehaviorImpl = {
    */
   sectionSelector: '',
 
-  /** @type {?Element} The scrolling container. */
-  scroller: null,
-
   /** @override */
   attached: function() {
+    /** @type {!HTMLElement} The scrolling container. */
     this.scroller = this.domHost && this.domHost.parentNode.$.mainContainer;
   },
 
@@ -62,77 +56,57 @@ var MainPageBehaviorImpl = {
    * @param {!SettingsSectionElement} section
    */
   expandSection: function(section) {
-    // If another section's card is expanding, cancel that animation first.
-    var expanding = this.$$('.expanding');
-    if (expanding) {
-      if (expanding == section)
+    // TODO(michaelpg): Manage transition lifetime better.
+    // crbug.com/624145
+    if (this.openSectionTransition_) {
+      if (this.openSectionTransition_.section == section)
         return;
 
-      if (this.animations['section']) {
-        // Cancel the animation, then call startExpandSection_.
-        this.cancelAnimation('section', function() {
-          this.startExpandSection_(section);
-        }.bind(this));
-      } else {
-        // The animation must have finished but its promise hasn't resolved yet.
-        // When it resolves, collapse that section's card before expanding
-        // this one.
-        setTimeout(function() {
-          this.collapseSection(
-              /** @type {!SettingsSectionElement} */(expanding));
-          this.finishAnimation('section', function() {
-            this.startExpandSection_(section);
-          }.bind(this));
-        }.bind(this));
-      }
-
+      // Cancel the other section expanding.
+      this.openSectionTransition_.cancel();
+      // After everything, schedule this function again to open the section.
+      this.openSectionTransition_.finished
+          .then(function() {
+            // The animation already finished, so close that section.
+            this.collapseSection(this.openSectionTransition_.section);
+          }).catch(function() {})
+          .then(this.expandSection.bind(this, section));
       return;
     }
-
-    if (this.$$('.collapsing') && this.animations['section']) {
-      // Finish the collapse animation before expanding.
-      this.finishAnimation('section', function() {
-        this.startExpandSection_(section);
-      }.bind(this));
+    if (this.closeSectionTransition_) {
+      // Finish collapsing the section, then expand this section.
+      this.closeSectionTransition_.finished
+          .then(this.expandSection.bind(this, section));
       return;
     }
 
-    this.startExpandSection_(section);
-  },
-
-  /**
-   * Helper function to set up and start the expand animation.
-   * @param {!SettingsSectionElement} section
-   */
-  startExpandSection_: function(section) {
-    if (section.classList.contains('expanded'))
-      return;
+    this.openSectionTransition_ =
+        new settings.OpenSectionTransition(section, this.scroller);
 
     // Freeze the scroller and save its position.
-    this.listScrollTop_ = this.scroller.scrollTop;
-
+    this.scroller.listScrollTop_ = this.scroller.scrollTop;
     var scrollerWidth = this.scroller.clientWidth;
     this.scroller.style.overflow = 'hidden';
+
     // Adjust width to compensate for scroller.
     var scrollbarWidth = this.scroller.clientWidth - scrollerWidth;
     this.scroller.style.width = 'calc(100% - ' + scrollbarWidth + 'px)';
 
-    // Freezes the section's height so its card can be removed from the flow.
-    this.freezeSection_(section);
-
-    // Expand the section's card to fill the parent.
-    var animationPromise = this.playExpandSection_(section);
-
-    animationPromise.then(function() {
-      this.scroller.scrollTop = 0;
-      this.toggleOtherSectionsHidden_(section.section, true);
-    }.bind(this), function() {
-      // Animation was canceled; restore the section.
-      this.unfreezeSection_(section);
-    }.bind(this)).then(function() {
-      this.scroller.style.overflow = '';
-      this.scroller.style.width = '';
-    }.bind(this));
+    this.openSectionTransition_.play()
+        .then(function() {

[dschuyler, 2016/07/07 18:46:21]

This looks like a different style than is
commonly done: where the .then is on the 
following line rather than the line above.
Is this a whoops or an intentional deviation?
(Not saying this must change, just asking
in case it's a whoops).

[michaelpg, 2016/07/08 00:19:02]

Thanks for asking, but yes, this was very intentional; my impression was that
this was easier to read ("then, catch, then"). I'm open to suggestions, though!

What I have: http://pastebin.com/raw/74znLD6h

What we normally do: http://pastebin.com/raw/TcUCAASm

(remove "/raw" for syntax highlighting, but it sucks)

Another alt: http://pastebin.com/raw/7KvPa8kK

+          this.toggleOtherSectionsHidden_(section.section, true);
+          this.scroller.scrollTop = 0;
+          this.classList.add('showing-subpage');
+          this.fire('subpage-expand');
+        }.bind(this))
+        .catch(function() {
+          this.scroller.scrollTop = this.scroller.listScrollTop_;
+        }.bind(this))
+        .then(function() {
+          this.scroller.style.width = '';
+          this.scroller.style.overflow = '';
+          this.openSectionTransition_ = null;
+        }.bind(this));
   },
 
   /**
@@ -140,213 +114,60 @@ var MainPageBehaviorImpl = {
    * @param {!SettingsSectionElement} 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']) {
-        this.cancelAnimation('section');
-      } else {
-        // The animation must have finished but its promise hasn't finished
-        // resolving; try again asynchronously.
-        this.async(function() {
-          this.collapseSection(section);
-        });
-      }
+    if (this.closeSectionTransition_) {
+      assert(this.closeSectionTransition_.section == section);
+      this.closeSectionTransition_.cancel();
       return;
     }
 
-    if (!section.classList.contains('expanded'))
+    if (this.openSectionTransition_) {
+      assert(this.openSectionTransition_.section == section);
+      this.openSectionTransition_.cancel();
       return;
+    }
 
-    this.toggleOtherSectionsHidden_(section.section, false);
+    assert(section.classList.contains('expanded'));
 
     var scrollerWidth = this.scroller.clientWidth;
     this.scroller.style.overflow = 'hidden';
+
     // Adjust width to compensate for scroller.
+    // TODO(michaelpg): Minimize horizontal motion when scrollbar changes for
+    // the common cases.
     var scrollbarWidth = this.scroller.clientWidth - scrollerWidth;
     this.scroller.style.width = 'calc(100% - ' + scrollbarWidth + 'px)';
 
-    this.playCollapseSection_(section).then(function() {
-      this.unfreezeSection_(section);
-      this.scroller.style.overflow = '';
-      this.scroller.style.width = '';
-      section.classList.remove('collapsing');
-    }.bind(this));
-  },
-
-  /**
-   * Freezes a section's height so its card can be removed from the flow without
-   * affecting the layout of the surrounding sections.
-   * @param {!SettingsSectionElement} section
-   * @private
-   */
-  freezeSection_: function(section) {
-    var card = section.$.card;
-    section.style.height = section.clientHeight + 'px';
-
-    var cardHeight = card.offsetHeight;
-    var cardWidth = card.offsetWidth;
-    // If the section is not displayed yet (e.g., navigated directly to a
-    // sub-page), cardHeight and cardWidth are 0, so do not set the height or
-    // width explicitly.
-    // TODO(michaelpg): Improve this logic when refactoring
-    // settings-animated-pages.
-    if (cardHeight && cardWidth) {
-      // TODO(michaelpg): Temporary hack to store the height the section should
-      // collapse to when it closes.
-      card.origHeight_ = cardHeight;
-
-      card.style.height = cardHeight + 'px';
-      card.style.width = cardWidth + 'px';
-    } else {
-      // Set an invalid value so we don't try to use it later.
-      card.origHeight_ = NaN;
-    }
-
-    // Place the section's card at its current position but removed from the
-    // flow.
-    card.style.top = card.getBoundingClientRect().top + 'px';
-    section.classList.add('frozen');
-  },
-
-  /**
-   * After freezeSection_, restores the section to its normal height.
-   * @param {!SettingsSectionElement} section
-   * @private
-   */
-  unfreezeSection_: function(section) {
-    if (!section.classList.contains('frozen'))
-      return;
-    var card = section.$.card;
-    section.classList.remove('frozen');
-    card.style.top = '';
-    card.style.height = '';
-    card.style.width = '';
-    section.style.height = '';
-  },
-
-  /**
-   * Expands the card in |section| to fill the page.
-   * @param {!SettingsSectionElement} section
-   * @return {!Promise}
-   * @private
-   */
-  playExpandSection_: function(section) {
-    var card = section.$.card;
-
-    // The card should start at the top of the page.
-    var targetTop = this.scroller.getBoundingClientRect().top;
-
-    section.classList.add('expanding');
-
-    // Expand the card, using minHeight. (The card must span the container's
-    // client height, so it must be at least 100% in case the card is too short.
-    // If the card is already taller than the container's client height, we
-    // don't want to shrink the card to 100% or the content will overflow, so
-    // we can't use height, and animating height wouldn't look right anyway.)
-    var keyframes = [{
-      top: card.style.top,
-      minHeight: card.style.height,
-      easing: EASING_FUNCTION,
-    }, {
-      top: targetTop + 'px',
-      minHeight: 'calc(100% - ' + targetTop + 'px)',
-    }];
-    var options = /** @type {!KeyframeEffectOptions} */({
-      duration: EXPAND_DURATION
-    });
-    // TODO(michaelpg): Change elevation of sections.
-    var promise;
-    if (keyframes[0].top && keyframes[0].minHeight)
-      promise = this.animateElement('section', card, keyframes, options);
-    else
-      promise = Promise.resolve();
-
-    promise.then(function() {
-      section.classList.add('expanded');
-      card.style.top = '';
-      this.style.margin = 'auto';
-      section.$.header.hidden = true;
-      section.style.height = '';
-    }.bind(this), function() {
-      // The animation was canceled; catch the error and continue.
-    }).then(function() {
-      // Whether finished or canceled, clean up the animation.
-      section.classList.remove('expanding');
-      card.style.height = '';
-      card.style.width = '';
-    });
-
-    return promise;
-  },
-
-  /**
-   * Collapses the card in |section| back to its normal position.
-   * @param {!SettingsSectionElement} section
-   * @return {!Promise}
-   * @private
-   */
-  playCollapseSection_: function(section) {
-    var card = section.$.card;
-
-    this.style.margin = '';
-    section.$.header.hidden = false;
-
-    var startingTop = this.scroller.getBoundingClientRect().top;
-
-    var cardHeightStart = card.clientHeight;
-    var cardWidthStart = card.clientWidth;
-
-    section.classList.add('collapsing');
-    section.classList.remove('expanding', 'expanded');
-
-    // If we navigated here directly, we don't know the original height of the
-    // section, so we skip the animation.
-    // TODO(michaelpg): remove this condition once sliding is implemented.
-    if (isNaN(card.origHeight_))
-      return Promise.resolve();
-
-    // Restore the section to its proper height to make room for the card.
-    section.style.height = section.clientHeight + card.origHeight_ + 'px';
-
-    // TODO(michaelpg): this should be in collapseSection(), but we need to wait
-    // until the full page height is available (setting the section height).
-    this.scroller.scrollTop = this.listScrollTop_;
-
-    // The card is unpositioned, so use its position as the ending state,
-    // but account for scroll.
-    var targetTop = card.getBoundingClientRect().top - this.scroller.scrollTop;
-
-    // Account for the section header.
-    var headerStyle = getComputedStyle(section.$.header);
-    targetTop += section.$.header.offsetHeight +
-        parseInt(headerStyle.marginBottom, 10) +
-        parseInt(headerStyle.marginTop, 10);
-
-    var keyframes = [{
-      top: startingTop + 'px',
-      minHeight: cardHeightStart + 'px',
-      easing: EASING_FUNCTION,
-    }, {
-      top: targetTop + 'px',
-      minHeight: card.origHeight_ + 'px',
-    }];
-    var options = /** @type {!KeyframeEffectOptions} */({
-      duration: EXPAND_DURATION
-    });
-
-    card.style.width = cardWidthStart + 'px';
-    var promise = this.animateElement('section', card, keyframes, options);
-    promise.then(function() {
-      card.style.width = '';
+    // Allow time for the dom-ifs in settings-main to re-render.
+    // TODO(michaelpg): Use a readiness signal (e.g., from the router) rather
+    // than firing events for settings-main and running this function async.
+    this.fire('subpage-collapsing');
+    this.async(function() {
+      // Set up the close transition first to take the section out of the flow
+      // before showing everything.
+      this.closeSectionTransition_ =
+          new settings.CloseSectionTransition(section, this.scroller);
+      this.closeSectionTransition_.setUp();
+
+      this.toggleOtherSectionsHidden_(section.section, false);
+      this.classList.remove('showing-subpage');
+      this.scroller.scrollTop = this.scroller.listScrollTop_;
+
+      this.closeSectionTransition_.play()
+          .catch(function() {
+            this.fire('subpage-expand');
+          }.bind(this))
+          .then(function() {
+            this.scroller.style.overflow = '';
+            this.scroller.style.width = '';
+            this.closeSectionTransition_ = null;
+          }.bind(this));
     });
-    return promise;
   },
 };
 
 
 /** @polymerBehavior */
 var MainPageBehavior = [
-  TransitionBehavior,
   MainPageBehaviorImpl
 ];