Move settings-section expand/collapse animations to settings-section

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

Index: chrome/browser/resources/settings/settings_page/settings_section.js
diff --git a/chrome/browser/resources/settings/settings_page/settings_section.js b/chrome/browser/resources/settings/settings_page/settings_section.js
index 1149e2ea81b7aa0fd017246548977f7e8916be2b..be96b4c8d3e1a7a5dd99b7a71fe71f2faa3eb114 100644
--- a/chrome/browser/resources/settings/settings_page/settings_section.js
+++ b/chrome/browser/resources/settings/settings_page/settings_section.js
@@ -6,6 +6,8 @@
  * @fileoverview
  * 'settings-section' shows a paper material themed section with a header
  * which shows its page title.
+

[Dan Beam, 2016/08/05 04:14:38]

 *

[michaelpg, 2016/08/05 05:16:08]

Done.

+ * The section can expand vertically to fill its container's padding edge.
  *
  * Example:
  *
@@ -13,7 +15,12 @@
  *      <!-- Insert your section controls here -->
  *    </settings-section>
  */
-Polymer({
+
+// Fast out, slow in.
+var EASING_FUNCTION = 'cubic-bezier(0.4, 0, 0.2, 1)';
+var EXPAND_DURATION = 350;
+
+var SettingsSectionElement = Polymer({
   is: 'settings-section',
 
   properties: {
@@ -23,18 +30,208 @@ Polymer({
     currentRoute: Object,
 
     /**
-     * The section is expanded to a full-page view when this property matches
+     * The section name should match a name specified in route.js. The
+     * MainPageBehavior will expand this section if this section name matches
      * currentRoute.section.
-     *
-     * The section name must match the name specified in settings_router.js.
      */
-    section: {
-      type: String,
-    },
+    section: String,
 
-    /**
-     * Title for the page header and navigation menu.
-     */
+    /** Title for the section header. */
     pageTitle: String,
   },
+
+  /**
+   * Freezes the section's height so its card can be removed from the flow
+   * without affecting the layout of the surrounding sections.
+   * @private
+   */
+  freezeDimensions: function() {

[michaelpg, 2016/08/05 02:09:11]

The contents of the following functions were moved from main_page_behavior.js.
The animation logic is overhauled in my next patch, so the existing code doesn't
need to be re-reviewed here.

[Dan Beam, 2016/08/05 04:14:38]

does this need a

if (this.classList.contains('frozen'))
  return;

or

assert(!this.classList.contains('frozen'));

[michaelpg, 2016/08/05 05:16:08]

Probably? I'm reluctant to make changes like that. I know there are ways of
breaking navigation currently, so there are already incorrect logic/assumptions
somewhere, so I don't want to make functional changes that might make things
even worse. Since this will be rewritten and all.

+    var card = this.$.card;
+    this.style.height = this.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;

[michaelpg, 2016/08/05 02:09:11]

I could take care of this TODO now with a property on <settings-section>, but
I'm reluctant to make more changes to the original code. LMK if you'd like that
in this CL.

[Dan Beam, 2016/08/05 04:14:38]

Acknowledged.

+
+      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';
+    this.classList.add('frozen');
+  },
+
+  /** After freezeDimensions, restores the section to its normal height. */
+  unfreezeDimensions: function() {
+    if (!this.classList.contains('frozen'))
+      return;
+    this.classList.remove('frozen');
+    this.$.card.style.top = '';
+    this.$.card.style.height = '';
+    this.$.card.style.width = '';
+    this.style.height = '';
+  },
+
+  /**
+   * @return {boolean} True if the section is currently rendered and not
+   *     already expanded or transitioning.
+   */
+  canAnimateExpand: function() {
+    return !this.classList.contains('expanded');
+  },
+
+  /**
+   * Animates the section expanding to fill the container. The section is fixed
+   * in the viewport during the animation. The section adds the "expanding"
+   * class while the animation plays.
+   *
+   * @param {!HTMLElement} container The scrolling container to fill.
+   * @return {?SettingsSectionElement.AnimationConfig}
+   */
+  animateExpand: function(container) {
+    var card = this.$.card;
+
+    // The card should start at the top of the page.
+    var targetTop = container.getBoundingClientRect().top;
+
+    this.classList.add('expanding');
+
+    // Nothing to animate.
+    if (!card.style.top || !card.style.height)
+      return null;
+
+    // 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.
+    return {card: card, keyframes: keyframes, options: options};
+  },
+
+  /**
+   * Cleans up after animateExpand().
+   * @param {boolean} finished Whether the animation finished successfully.
+   */
+  cleanUpAnimateExpand: function(finished) {
+    if (finished) {
+      this.classList.add('expanded');
+      this.$.card.style.top = '';
+      this.$.header.hidden = true;
+      this.style.height = '';
+    }
+
+    this.classList.remove('expanding');
+    this.$.card.style.height = '';
+    this.$.card.style.width = '';
+  },
+
+  /** @return {boolean} True if the section is currently expanded. */
+  canAnimateCollapse: function() {
+    return this.classList.contains('expanded');
+  },
+
+  /**
+   * Collapses an expanded section's card back into position in the main page.
+   * Adds the "expanding" class during the animation.
+   * @param {!HTMLElement} container The scrolling container the card fills.
+   * @param {number} prevScrollTop scrollTop of the container before this
+   *     section expanded.
+   * @return {?SettingsSectionElement.AnimationConfig}
+   */
+  animateCollapse: function(container, prevScrollTop) {

[Dan Beam, 2016/08/05 04:14:38]

both animateCollapse and animateExpand are weird to me.

they both have side effects, but also return animation configs.  it'd be clearer
to separate if possible.

[michaelpg, 2016/08/05 05:16:08]

Acknowledged. I'll work on that in a followup CL.

+    this.$.header.hidden = false;
+
+    var startingTop = container.getBoundingClientRect().top;
+
+    var card = this.$.card;
+    var cardHeightStart = card.clientHeight;
+    var cardWidthStart = card.clientWidth;
+
+    this.classList.add('collapsing');
+    this.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_))

[Dan Beam, 2016/08/05 04:14:38]

nit: use isNaN or Number.isNan() consistently (I vote for isNaN because it's
shorter)

[michaelpg, 2016/08/05 05:16:08]

Done (cut and pasted code)

I prefer Number.isNaN because it does what it says on the tin.

+      return null;
+
+    // Restore the section to its proper height to make room for the card.
+    this.style.height = this.clientHeight + card.origHeight_ + 'px';

[Dan Beam, 2016/08/05 04:14:38]

nit: use parens with differently typed args that end up as a string consistently

i.e.

<num> + <num> + <string>

or

(<num> - <num>) + <string>

everywhere

[michaelpg, 2016/08/05 05:16:08]

Done (cut and pasted code)

+
+    // TODO(michaelpg): this should be in MainPageBehavior(), but we need to
+    // wait until the full page height is available (setting the section
+    // height).
+    container.scrollTop = prevScrollTop;
+
+    // The card is unpositioned, so use its position as the ending state,
+    // but account for scroll.
+    var targetTop = card.getBoundingClientRect().top - container.scrollTop;
+
+    // Account for the section header.
+    var headerStyle = getComputedStyle(this.$.header);
+    targetTop += this.$.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';
+
+    return {card: card, keyframes: keyframes, options: options};
+  },
+
+  /**
+   * Cleans up after animateCollapse().
+   * @param {boolean} finished Whether the animation finished successfully.
+   */
+  cleanUpAnimateCollapse: function(finished) {
+    if (finished)
+      this.$.card.style.width = '';
+  },
 });
+
+/**
+ * Information needed by TransitionBehavior to schedule animations.
+ * @typedef {{

[michaelpg, 2016/08/05 02:09:11]

This typedef is temporary, the animations will eventually be called by
settings-section instead of MainPageBehavior.

[Dan Beam, 2016/08/05 04:14:38]

Acknowledged.

+ *   card: !HTMLElement,
+ *   keyframes: !Array<!Object>,
+ *   options: !KeyframeEffectOptions
+ * }}
+ */
+SettingsSectionElement.AnimationConfig;