Settings Rewrite: Make subpage scrolling work.

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 123ce46ceadfdd45909321023bbc8713a40289c4..bc9d0749ea4a076d74e727e788131c49c9ef2231 100644
--- a/chrome/browser/resources/settings/settings_page/settings_section.js
+++ b/chrome/browser/resources/settings/settings_page/settings_section.js
@@ -4,20 +4,20 @@
 
 /**
  * @fileoverview
- * 'cr-settings-section' shows a paper material themed section with a header
+ * 'settings-section' shows a paper material themed section with a header
  * which shows its page title.
  *
  * Example:
  *
- *    <cr-settings-section page-title="[[pageTitle]]">
+ *    <settings-section page-title="[[pageTitle]]">
  *      <!-- Insert your section controls here -->
- *    </cr-settings-section>

[Dan Beam, 2015/09/25 23:18:57]

you should probably do this separately

[tommycli, 2015/09/25 23:36:40]

Done.

+ *    </settings-section>
  *
  * @group Chrome Settings Elements
- * @element cr-settings-section
+ * @element settings-section
  */
 Polymer({
-  is: 'cr-settings-section',
+  is: 'settings-section',
 
   behaviors: [
     Polymer.NeonAnimationRunnerBehavior,
@@ -50,7 +50,6 @@ Polymer({
      */
     expandContainer: {
       type: Object,
-      notify: true,
     },
 
     animationConfig: {
@@ -69,6 +68,10 @@ Polymer({
     },
   },
 
+  listeners: {
+    'expand-animation-complete': 'onExpandAnimationComplete_',
+  },
+
   /** @private */
   expanded_: false,
 
@@ -83,7 +86,18 @@ Polymer({
 
     var visible = expanded || this.currentRoute.section == '';
     this.$.section.elevation = visible ? 1 : 0;
+
+    // Remove 'hidden' class immediately, but defer adding it if we are invisble
+    // until the animation is complete.
+    if (visible)
+      this.hidden = false;
   },
+
+  /** @private */
+  onExpandAnimationComplete_: function() {
+    this.hidden = this.currentRoute.section != '' &&
+                  this.currentRoute.section != this.section;
+  }
 });
 
 Polymer({
@@ -94,32 +108,36 @@ Polymer({
   ],
 
   configure: function(config) {
-    var node = config.node;
-    var containerRect = node.expandContainer.getBoundingClientRect();
-    var nodeRect = node.getBoundingClientRect();
+    var section = config.node.$.section;
+    var containerRect = config.node.expandContainer.getBoundingClientRect();
+    var sectionRect = section.getBoundingClientRect();
 
-    // Save section's original height.
-    node.unexpandedHeight = nodeRect.height;
+    // Set placeholder height so the page does not reflow during animation.
+    // TODO(tommycli): For URLs that directly load subpages, this does not work.
+    var placeholder = config.node.$.placeholder;
+    placeholder.style.top = section.offsetTop + 'px';
+    placeholder.style.height = section.offsetHeight + 'px';
 
-    var headerHeight = node.$.header.getBoundingClientRect().height;
-    var newTop = containerRect.top - headerHeight;
-    var newHeight = containerRect.height + headerHeight;
+    config.node.classList.add('neon-animating');
 
-    node.style.position = 'fixed';
-    node.style.zIndex = '1';
-
-    this._effect = new KeyframeEffect(node, [
-      {'top': nodeRect.top + 'px', 'height': nodeRect.height + 'px'},
-      {'top': newTop + 'px', 'height': newHeight + 'px'},
+    this._effect = new KeyframeEffect(section, [
+      {'top': sectionRect.top + 'px', 'height': sectionRect.height + 'px'},
+      {'top': containerRect.top + 'px', 'height': containerRect.height + 'px'},
     ], this.timingFromConfig(config));
     return this._effect;
   },
 
   complete: function(config) {
-    config.node.style.position = 'absolute';
-    config.node.style.top =
-        -config.node.$.header.getBoundingClientRect().height + 'px';
-    config.node.style.bottom = 0;
+    config.node.classList.remove('neon-animating');

[Dan Beam, 2015/09/25 23:18:02]

what is config.node?  can you make a short, descriptive variable alias to it and
re-use it?

[tommycli, 2015/09/25 23:36:40]

Done. Sure. Not sure what to call it other than 'element'. It's the polymer
element as described in the file.

[Dan Beam, 2015/09/25 23:38:45]

it's at least a parent of a section, right?  sectionWrapper?  sectionParent?

[tommycli, 2015/09/26 00:17:33]

Done.

+    config.node.classList.add('expanded');
+    config.node.expandContainer.classList.add('expanded');
+
+    for (var i = 0; i < config.node.parentNode.children.length; ++i) {
+      if (config.node.parentNode.children[i].nodeName == config.node.nodeName)

[Dan Beam, 2015/09/25 23:18:02]

i'm confused.

can this be .querySelectorAll('tag-name')?  I think that'd be far less
confusing.  or is this supposed to be siblings with the same tagName?

[tommycli, 2015/09/25 23:36:40]

Done.

+        config.node.parentNode.children[i].fire('expand-animation-complete');
+    }
+
+    config.node.expandContainer.scrollTop = 0;
   }
 });
 
@@ -131,30 +149,25 @@ Polymer({
   ],
 
   configure: function(config) {
-    var node = config.node;
-
-    var oldRect = node.getBoundingClientRect();
+    var section = config.node.$.section;

[Dan Beam, 2015/09/25 23:18:02]

move lower, closer to where it's actually used

[tommycli, 2015/09/25 23:36:40]

Done.

+    var oldRect = config.node.expandContainer.getBoundingClientRect();
 
-    // Temporarily set position to static to determine new height.
-    node.style.position = '';
-    var newTop = node.getBoundingClientRect().top;
+    config.node.classList.remove('expanded');
+    config.node.expandContainer.classList.remove('expanded');
 
-    // TODO(tommycli): This value is undefined when the user navigates to a
-    // subpage directly by URL instead of from the settings root. Find a better
-    // method than using 200 as a dummy height.
-    var newHeight = node.unexpandedHeight || 200;
+    // Get the placeholder coordinates before reflowing.
+    var newRect = config.node.$.placeholder.getBoundingClientRect();
 
-    node.style.position = 'fixed';
+    config.node.classList.add('neon-animating');
 
-    this._effect = new KeyframeEffect(node, [
+    this._effect = new KeyframeEffect(section, [
       {'top': oldRect.top + 'px', 'height': oldRect.height + 'px'},
-      {'top': newTop + 'px', 'height': newHeight + 'px'},
+      {'top': newRect.top + 'px', 'height': newRect.height + 'px'},
     ], this.timingFromConfig(config));
     return this._effect;
   },
 
   complete: function(config) {
-    config.node.style.position = '';
-    config.node.style.zIndex = '0';
+    config.node.classList.remove('neon-animating');
   }
 });