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 532418a01d4054c21e630a04ccefc4ce8a719018..8f952456618f9ae719b61fa724ba97f73b4afa92 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|.
@@ -24,21 +20,20 @@ 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.
+ * Responds to route changes by expanding, collapsing, and scrolling to
+ * sections. Expanded sections take up the full height of the container. At
+ * most one section should be expanded at any given time.
+ * TODO(michaelpg): Combine with RoutableBehaviorImpl.
  * @polymerBehavior Polymer.MainPageBehavior
  */
 var MainPageBehaviorImpl = {
-  /** @type {?Element} The scrolling container. */
-  scroller: null,
-
   /** @override */
   attached: function() {
-    if (this.domHost && this.domHost.parentNode.tagName == 'PAPER-HEADER-PANEL')
-      this.scroller = this.domHost.parentNode.$.mainContainer;
-    else
-      this.scroller = document.body; // Used in unit tests.
+  /** @type {!HTMLElement} */

[Dan Beam, 2016/08/05 00:18:05]

indent off

+    this.scroller = (this.domHost && this.domHost.parentNode.tagName ==

[Dan Beam, 2016/08/05 00:18:05]

what's the functional difference here?  why are you making a ternary?

+        'PAPER-HEADER-PANEL') ?
+        this.domHost.parentNode.$.mainContainer :

[Dan Beam, 2016/08/05 00:18:05]

yeppppppp this'll trigger that presubmit

+        document.body; // Used in unit tests.
   },
 
   /**
@@ -55,295 +50,170 @@ var MainPageBehaviorImpl = {
   },
 
   /**
-   * Animates the card in |section|, expanding it to fill the page.
-   * @param {!SettingsSectionElement} section
+   * Transistions to the current route if the |deferredTransition_| flag is set.

[Dan Beam, 2016/08/05 00:47:07]

Transitions

+   * @private
    */
-  expandSection: function(section) {
-    // If another section's card is expanding, cancel that animation first.
-    var expanding = this.$$('.expanding');
-    if (expanding) {
-      if (expanding == section)
-        return;
+  runDeferredTransition_: function() {

[Dan Beam, 2016/08/05 00:47:06]

this makes it seem as if "deferredTransition_" is a @type {Transition} that's
been set previously.  but it's actually a boolean.  can you name this
wasDeferred_ or something less ambiguous?

+    // TODO(michaelpg): Manage transition lifetime better. crbug.com/624145
+    if (!this.deferredTransition_)
+      return;
+    this.deferredTransition_ = false;
+    this.transitionToRoute_();
+  },
 
-      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));
+  /**
+   * Transitions to the current route based on the state of the current page
+   * (i.e., expands and collapses sections as necessary). Must only be called
+   * when no other transition is running or pending.
+   * @private
+   */
+  transitionToRoute_: function() {
+    assert(!this.currentSectionTransition_);
+
+    var expandedSection = /** @type {?SettingsSectionElement} */(
+        this.$$('settings-section.expanded'));
+    if (expandedSection) {
+      if (expandedSection.section == this.currentRoute.section &&
+          this.currentRoute.subpage.length > 0) {
+        // settings-animated-pages controls visibility of subpages in sections.

[Dan Beam, 2016/08/05 00:47:06]

can you describe this in a different, more high-level way. possibly with
concrete examples.  as in: this happens when a dialog or subpage is shown.

+        return;
       }
 
+      // Collapse this section (and check for a deferred transition when
+      // finished).
+      this.collapseSection(expandedSection).then(
+          this.runDeferredTransition_.bind(this));
+
+      // If the new route is a sub-page, we'll have to expand that next.
+      this.deferredTransition_ = this.currentRoute.subpage.length > 0;
       return;
     }
 
-    if (this.$$('.collapsing') && this.animations['section']) {
-      // Finish the collapse animation before expanding.
-      this.finishAnimation('section', function() {
-        this.startExpandSection_(section);
-      }.bind(this));
+    if (this.currentRoute.section.length == 0)

[Dan Beam, 2016/08/05 00:47:06]

nit: can you just use truthiness for section or other strings?  makes it easier
to tell they're not arrays like subpage is

+      return;
+
+    var section = this.getSection_(this.currentRoute.section);
+    if (!section)
+      return;
+
+    // Expand the section if the route is a subpage (and check for a deferred
+    // transition when finished).
+    if (this.currentRoute.subpage.length > 0) {
+      this.expandSection(section).then(this.runDeferredTransition_.bind(this));
       return;
     }
 

[Dan Beam, 2016/08/05 00:47:06]

can you put a comment saying which cases actually make it here?

-    this.startExpandSection_(section);
+    this.scrollToSection_();
   },
 
   /**
-   * Helper function to set up and start the expand animation.
+   * Animates the card in |section|, expanding it to fill the page.
    * @param {!SettingsSectionElement} section
+   * @return {!Promise} Promise when the transition finishes or cancels.
    */
-  startExpandSection_: function(section) {
-    if (section.classList.contains('expanded'))
-      return;
+  expandSection: function(section) {
+    assert(!this.currentSectionTransition_);
 
-    // Freeze the scroller and save its position.
-    this.listScrollTop_ = this.scroller.scrollTop;
+    // The overscroll must not shrink during the transition.
+    this.fire('overscroll-suppress');
 
+    // Prevent additional scrolling and keep the expanding section clipped.
+    this.scroller.style.overflow = '';
     var scrollerWidth = this.scroller.clientWidth;
+    this.mainScrollTop_ = this.scroller.scrollTop;
     this.scroller.style.overflow = 'hidden';
-    // Adjust width to compensate for scroller.
+
+    // Set the width to account for the missing scrollbar.
     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);
+    this.currentSectionTransition_ =
+        new settings.OpenSectionTransition(section, this.scroller);
+
+    return this.currentSectionTransition_.play().then(function(success) {
+      this.currentSectionTransition_ = null;
+      if (success) {
+        // Hide other sections and scroll to the top of the subpage.
+        this.classList.add('showing-subpage');
+        this.toggleOtherSectionsHidden_(section.section, true);
+        this.scroller.scrollTop = 0;
+        // Notify that the page is fully expanded.
+        this.fire('subpage-expand');
+      } else {
+        // The open transition cancelled, so scroll back to our old location.
+        this.scroller.scrollTop = this.mainScrollTop_;
+      }
 
-    // Expand the section's card to fill the parent.
-    var animationPromise = this.playExpandSection_(section);
+      // Overscroll can now be recalculated.
+      this.fire('overscroll-recalc');
 
-    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 = '';
+      // Restore the scrollbar.
       this.scroller.style.width = '';
+      this.scroller.style.overflow = '';
     }.bind(this));
   },
 
   /**
    * Animates the card in |section|, collapsing it back into its section.
    * @param {!SettingsSectionElement} section
+   * @return {!Promise} Promise when the transition finishes or cancels.
    */
   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);
-        });
-      }
-      return;
-    }
-
-    if (!section.classList.contains('expanded'))
-      return;
-
-    this.toggleOtherSectionsHidden_(section.section, false);
+    assert(!this.currentSectionTransition_);
+    assert(section.classList.contains('expanded'));
 
-    var scrollerWidth = this.scroller.clientWidth;
+    // TODO(michaelpg): Minimize horizontal motion when scrollbar changes for
+    // the common cases.
     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)';
 
-    this.playCollapseSection_(section).then(function() {
-      this.unfreezeSection_(section);
-      this.scroller.style.overflow = '';
-      this.scroller.style.width = '';
-      section.classList.remove('collapsing');
-    }.bind(this));
-  },
+    // Set up the close transition first, which takes the section out of the
+    // flow, before showing everything.
+    this.currentSectionTransition_ =
+        new settings.CloseSectionTransition(section, this.scroller);
+    this.currentSectionTransition_.setUp();
 
-  /**
-   * 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;
-  },
+    this.toggleOtherSectionsHidden_(section.section, false);
+    this.classList.remove('showing-subpage');
+    this.fire('overscroll-recalc');
 
-  /**
-   * 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 = '';
-    });
-    return promise;
+    return new Promise(function(resolve, reject) {
+      // Wait for the other sections to show up so we can scroll properly.
+      setTimeout(function() {
+        // Allow overscroll adjustment so it changes now rather than after the
+        // transition has begun.
+
+        var newSection = this.currentRoute.section &&
+            this.getSection_(this.currentRoute.section);
+
+        // Scroll to the section if indicated by the route. TODO(dschuyler): Is
+        // this the right behavior, or should we return to the previous scroll
+        // position?
+        if (newSection)
+          newSection.scrollIntoView();
+        else
+          this.scroller.scrollTop = this.mainScrollTop_;
+
+        this.currentSectionTransition_.play().then(function(success) {
+          this.currentSectionTransition_ = null;
+          if (!success) {
+            // The collapse was canceled, so ensure the page is still set up for
+            // a full-height section.
+            this.fire('subpage-expand');
+          }
+
+          // Restore the scrollbar.
+          this.scroller.style.overflow = '';
+          this.scroller.style.width = '';

[Dan Beam, 2016/08/05 00:47:06]

marginally useful: make a lockScrolling_() and unlockScrolling_() that sets
overflow to 'hidden' or '' and possibly dabbles with width in some cases?

+          resolve();
+        }.bind(this));
+      }.bind(this));
+    }.bind(this));
   },
 };
 
 
 /** @polymerBehavior */
 var MainPageBehavior = [
-  TransitionBehavior,
   MainPageBehaviorImpl
 ];
 
@@ -371,43 +241,42 @@ var RoutableBehaviorImpl = {
         function() {
           // If the current section changes while we are waiting for the page to
           // be ready, scroll to the newest requested section.
-          this.getSection_(this.currentRoute.section).scrollIntoView();
+          var section = this.getSection_(this.currentRoute.section);
+          if (section)
+            section.scrollIntoView();
         }.bind(this));
   },
 
-  /** @private */
+  /**
+   * @param {?settings.Route} newRoute
+   * @param {?settings.Route} oldRoute
+   * @private
+   */
   currentRouteChanged_: function(newRoute, oldRoute) {
-    var newRouteIsSubpage = newRoute && newRoute.subpage.length;
-    var oldRouteIsSubpage = oldRoute && oldRoute.subpage.length;
-
-    if (!oldRoute && newRouteIsSubpage) {
+    if (!oldRoute && newRoute && newRoute.subpage.length > 0) {
       // 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));
+      this.deferredTransition_ = true;
+      setTimeout(this.runDeferredTransition_.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);
+    if (this.currentSectionTransition_) {
+      // Transition to the current route when the current one finishes.
+      this.deferredTransition_ = true;
+
+      // If the current transition is opening a section we want to close, cancel
+      // that transition immediately.
+      if (this.currentSectionTransition_ instanceof
+          settings.OpenSectionTransition &&
+          (this.currentSectionTransition_.section != newRoute.section ||
+           newRoute.subpage.length == 0)) {

[Dan Beam, 2016/08/05 00:47:07]

can you use some temporary variables for this?

+        this.currentSectionTransition_.cancel();
       }
-
-      // Scrolls to the section if this main page contains the route's section.
-      if (newRoute && newRoute.section && this.getSection_(newRoute.section))
-        this.scrollToSection_();
+      return;
     }
+
+    this.transitionToRoute_();
   },
 
   /**