[MD settings] end of page padding in content area

chrome/browser/resources/settings/settings_main/settings_main.js

Index: chrome/browser/resources/settings/settings_main/settings_main.js
diff --git a/chrome/browser/resources/settings/settings_main/settings_main.js b/chrome/browser/resources/settings/settings_main/settings_main.js
index 11cd9dd684eebba61b2ff7ec7ffed466e63f3575..a9e490114d447c5f725a8595c878144ca4c7a5d1 100644
--- a/chrome/browser/resources/settings/settings_main/settings_main.js
+++ b/chrome/browser/resources/settings/settings_main/settings_main.js
@@ -63,6 +63,12 @@ Polymer({
     document.addEventListener('toggle-advanced-page', function(e) {
       this.showAdvancedPage_ = e.detail;
       this.isAdvancedMenuOpen_ = e.detail;
+      this.currentRoute = {

[michaelpg, 2016/06/30 17:13:22]

can this be submitted separately?

[dschuyler, 2016/07/01 19:21:22]

Done.

+        page: this.isAdvancedMenuOpen_ ? 'advanced' : 'basic',
+        section: '',
+        subpage: [],
+        url: '',

[Dan Beam, 2016/06/30 06:45:54]

can you just omit URL?

[dschuyler, 2016/06/30 23:52:18]

We could change the SettingsRoute typedef to not require
the url. My hope is that requiring the URL
will help others specify routes correctly in
settings_router.js. Setting the url here seems
like a small price to have help getting the 
routes correct. WDYT

[Dan Beam, 2016/06/30 23:59:15]

how are all the other assignments to currentRoute compiling?
https://cs.chromium.org/search/?q=%22currentroute+%3D%22&sq=package:chromium&...
forcing folks to copy 1 more key that's partially used doesn't seem like an
improvement.

1) the URL is set based on this code, which doesn't care about the "url" key:
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/settin...

2) the URLs you're specifying are wrong, as far as I can tell:
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/settin...

so how does using url "help getting the routes correct"?

if you really want to make this better, we should make named routes, i.e.

Routes = {
  BASIC: {
    url: '/',
    ...
  },
  ADVANCED: {
    url: '/advanced',
    ...
  },
},

routes_: [Routes.BASIC, Routes.ADVANCED, ...],

and then from somewhere that needs to navigate:

this.currentRoute = Routes.BASIC;

[dschuyler, 2016/07/01 19:21:22]

Done.

[dschuyler, 2016/07/01 19:21:22]

Acknowledged.

+      };
       if (this.showAdvancedPage_) {
         scrollWhenReady(
             function() {
@@ -102,6 +108,46 @@ Polymer({
         newRoute.page == 'advanced';
 
     this.style.height = isSubpage ? '100%' : '';
+
+    this.$.overscroll.style.paddingBottom =
+        (this.parentNode.scrollHeight - this.lastSettingsElementHeight_()) +
+        'px';
+  },
+
+  /**
+   * Return the height of the last item in the settings content area. Used to
+   * determine how much padding to apply to the end of the content so that the
+   * last element may align with the top of the content area.
+   * @return {number}
+   * @private
+   */
+  lastSettingsElementHeight_: function() {
+    /**
+     * @param {Element} element
+     * @return {number}
+     */
+    function calcHeight(element) {
+      var style = getComputedStyle(element);
+      return element.offsetHeight + parseFloat(style.marginTop) +
+          parseFloat(style.marginBottom);
+    }
+
+    Polymer.dom.flush();

[Dan Beam, 2016/06/30 06:45:54]

why is this required?

[dschuyler, 2016/07/01 19:21:21]

I moved it up to where this is called from
and commented it. I'm using the flush to 
apply the dom-if prior to calculating sizes.

[Dan Beam, 2016/07/02 00:17:31]

can't we just use async() to act after this instead?

[dschuyler, 2016/07/02 00:23:55]

Done.

+    if (!this.currentRoute || this.currentRoute.subpage.length != 0 ||
+        this.showAboutPage_) {

[Dan Beam, 2016/06/30 06:45:54]

can this be done earlier?

[dschuyler, 2016/07/01 19:21:22]

Done.

+      return 0;
+    }
+
+    if (this.showAdvancedPage_) {
+      return calcHeight(this.$$('settings-advanced-page').$$(
+                        '[section=reset]'));

[Dan Beam, 2016/06/30 06:45:54]

why the reset section?

[dschuyler, 2016/07/01 19:21:22]

Changed to fetching last settings-section.

+    }
+
+    var toggleContainer = this.$$('#toggleContainer');
+    if (toggleContainer)
+      return calcHeight(toggleContainer);
+
+    return 0;
   },
 
   /** @private */