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

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

+// Copyright 2016 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.
+
+cr.define('settings', function() {
+  /**
+   * Expands a card to fill its container's client height, so it covers the
+   * container from the top padding edge to the bottom padding edge.
+   * @see https://www.w3.org/TR/cssom-view-1/#terminology
+   * @constructor
+   * @implements {settings.animation.AnimationGroup}
+   * @param {!HTMLElement} card Card to expand.
+   * @param {!HTMLElement} container Container whose height the card will span.
+   */
+  function ExpandCardTransition(card, container) {
+    this.card = card;
+    this.container = container;
+
+    /** @type {?settings.animation.Animation} */
+    this.animation_ = null;

[Dan Beam, 2016/07/07 18:27:54]

why not just put this @type when initializing?

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

Done.

+  }
+
+  ExpandCardTransition.prototype = {
+    __proto__: settings.animation.AnimationGroup.prototype,
+
+    /** @override */
+    play: function() {
+      // If we're running this transition before anything else, we won't have
+      // the information we need, so just expand immediately and finish.
+      if (typeof this.card.origHeight_ == 'undefined' ||
+          this.card.style.top == '' || this.card.style.height == '') {
+        this.finished = Promise.resolve();
+        return this.finished;
+      }
+
+      // Align the card with the container's padding edge.
+      var startingTop = this.card.getBoundingClientRect().top;
+
+      // Target position is the container's top edge in the viewport.
+      var targetTop = this.container.getBoundingClientRect().top;
+
+      // The target height shouldn't use the container's current height, because
+      // the container may resize when the window resizes. And height: 100%
+      // wouldn't use the container's height because we're position: fixed.
+      //
+      // Instead, find the part of the window height *not* used by the container
+      // (e.g., for a toolbar) and calc the height dynamically. This assumes the
+      // excluded height stays constant.
+      var excludedHeight = window.innerHeight - this.container.clientHeight;
+      var targetHeight = 'calc(100% - ' + excludedHeight + 'px)';
+
+      // Expand the card. The card's height must be 100% of the container's
+      // height or taller, so we use minHeight rather than height.
+      var keyframes = [{
+        top: startingTop + 'px',
+        minHeight: this.card.style.height,
+        easing: settings.animation.Timing.EASING,
+      }, {
+        top: targetTop + 'px',
+        minHeight: 'calc(100% - ' + targetTop + 'px)',
+      }];
+      var options = /** @type {!KeyframeEffectOptions} */({
+        duration: settings.animation.Timing.DURATION,
+      });
+

[Dan Beam, 2016/07/07 18:27:54]

as in here:

/** @type {?settings.animation.Animation} */

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

Done.

+      this.animation_ =
+          new settings.animation.Animation(this.card, keyframes, options);
+      this.finished = this.animation_.finished;
+      return this.finished;
+    },
+
+    /** @override */
+    finish: function() {
+      if (!this.animation_)
+        return;
+      this.animation_.finish();
+      this.animation_ = null;
+    },
+
+    /** @override */
+    cancel: function() {
+      if (!this.animation_)
+        return;
+      this.animation_.cancel();
+      this.animation_ = null;
+    },
+
+    /** @override */
+    finished: null,

[Dan Beam, 2016/07/07 18:27:54]

why is this necessary?

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

it's required. otherwise:

##
/foo/src/chrome/browser/resources/settings/settings_page/expand_card_transition.js:15:
ERROR - property finished on interface settings.animation.AnimationGroup is not
implemented by type settings.ExpandCardTransition

+  };
+
+  /**
+   * Collapses an expanded card back into its original section.
+   * @constructor
+   * @implements {settings.animation.AnimationGroup}
+   * @param {!SettingsSectionElement} section Section that contains the card.
+   * @param {!HTMLElement} container Container the card currently spans.
+   */
+  function CollapseCardTransition(section, container) {
+    /** @type {!HTMLElement} */
+    this.card = section.$['card'];

[Dan Beam, 2016/07/07 18:27:54]

should card be @private (i.e. card_)?

[Dan Beam, 2016/07/07 18:27:54]

this.card = assert(section.$['card']);

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

Done.

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

Done.

+    /** @type {!SettingsSectionElement} */
+    this.section = section;
+    this.container = container;
+
+    /** @type {?settings.animation.Animation} */
+    this.animation_ = null;
+  }
+
+  CollapseCardTransition.prototype = {
+    __proto__: settings.animation.AnimationGroup.prototype,
+
+    /** Sets up the transition before other page content has been unhidden. */
+    setUp: function() {
+      this.card.style.width = this.card.clientWidth + 'px';
+      this.card.style.height = this.card.clientHeight + 'px';
+    },
+
+    /** @override */
+    play: function() {
+      var startingTop = this.container.getBoundingClientRect().top;
+
+      // The card is unpositioned, so use its position as the ending state,
+      // but account for scroll.
+      var targetTop = this.card.getBoundingClientRect().top -
+          this.container.scrollTop;
+
+      // Account for the section header.
+      var headerStyle = getComputedStyle(this.section.$['header']);
+      targetTop += this.section.$['header'].offsetHeight +
+          parseFloat(headerStyle.marginBottom) +
+          parseFloat(headerStyle.marginTop);
+
+      var keyframes = [{
+        top: startingTop + 'px',
+        minHeight: this.card.style.height,
+        easing: settings.animation.Timing.EASING,
+      }, {
+        top: targetTop + 'px',
+        minHeight:
+            /** @type {{origHeight_: number}} */(this.card).origHeight_ + 'px',
+      }];
+      var options = /** @type {!KeyframeEffectOptions} */({
+        duration: settings.animation.Timing.DURATION,
+      });
+
+      this.card.style.height = '';
+      this.animation_ =
+          new settings.animation.Animation(this.card, keyframes, options);
+      this.finished = this.animation_.finished;
+      // Clean up after the animation finishes or cancels.
+      this.finished.catch(function() {}).then(function() {
+        this.card.style.width = '';
+      }.bind(this));
+      return this.finished;
+    },
+
+    /** @override */
+    finish: function() {
+      if (!this.animation_)
+        return;
+      this.animation_.finish();
+      this.animation_ = null;
+    },
+
+    /** @override */
+    cancel: function() {
+      if (!this.animation_)
+        return;
+      this.animation_.cancel();
+      this.animation_ = null;
+    },

[Dan Beam, 2016/07/07 18:27:54]

why can't we make a @protected |this.animation| in the base class and put both
finish and cancel there (if they're the same between both)?

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

the base interface is AnimationGroup; it doesn't require that all
implementations have some single web animation (case in point,
{Open,Close}SectionTransition).

I thought about making an intermediate class just for
{Expand,Collapse}CardTransition to extend, e.g. "SingleAnimation", but figured
I'd wait until there are more instances.

+
+    /** @override */
+    finished: null,
+  };
+
+  return {
+    CollapseCardTransition: CollapseCardTransition,
+    ExpandCardTransition: ExpandCardTransition,
+  };
+});