MD Settings: Never underscroll

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

 // Copyright 2015 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.
 
 /**
  * @fileoverview
  * 'settings-main' displays the selected settings page.
  */
 Polymer({
   is: 'settings-main',
@@ skipping 67 matching lines @@
     }.bind(this));
 
   },
 
   /** @private */
   overscrollChanged_: function() {
     if (!this.overscroll_ && this.boundScroll_) {
       this.parentNode.scroller.removeEventListener('scroll', this.boundScroll_);
       this.boundScroll_ = null;
     } else if (this.overscroll_ && !this.boundScroll_) {
-      this.boundScroll_ = this.scrollEventListener_.bind(this);
+      this.boundScroll_ = this.setOverscroll_.bind(this, 0);
       this.parentNode.scroller.addEventListener('scroll', this.boundScroll_);
     }
   },
 
-  /** @private */
-  scrollEventListener_: function() {
+  /**
+   * Sets the overscroll padding. Never forces a scroll, i.e., always leaves
+   * any currently visible overflow as-is.
+   * @param {number=} minHeight The minimum overscroll height needed.

[Dan Beam, 2016/08/09 23:26:40]

opt_minHeight

[michaelpg, 2016/08/09 23:46:09]

Done.

+   */
+  setOverscroll_: function(opt_minHeight) {
     var scroller = this.parentNode.scroller;
     var overscroll = this.$.overscroll;
     var visibleBottom = scroller.scrollTop + scroller.clientHeight;
     var overscrollBottom = overscroll.offsetTop + overscroll.scrollHeight;
     // How much of the overscroll is visible (may be negative).
     var visibleOverscroll = overscroll.scrollHeight -
                             (overscrollBottom - visibleBottom);
-    this.overscroll_ = Math.max(0, visibleOverscroll);
+    if (typeof opt_minHeight == 'undefined')
+      opt_minHeight = 0;
+    this.overscroll_ = Math.max(opt_minHeight, visibleOverscroll);

[Dan Beam, 2016/08/09 23:26:40]

nit: opt_minHeight || 0

[michaelpg, 2016/08/09 23:46:09]

Done.

   },
 
   /**
    * @param {boolean} opened Whether the menu is expanded.
    * @return {string} Which icon to use.
    * @private
    */
   arrowState_: function(opened) {
     return opened ? 'settings:arrow-drop-up' : 'cr:arrow-drop-down';
   },
@@ skipping 28 matching lines @@
             this.pageVisibility.advancedSettings !== false);
         this.advancedToggleExpanded_ = true;
       }
     }
 
     // Wait for any other changes prior to calculating the overflow padding.
     setTimeout(function() {
       // Ensure any dom-if reflects the current properties.
       Polymer.dom.flush();
 
-      this.overscroll_ = this.overscrollHeight_();
+      this.setOverscroll_(this.overscrollHeight_());
     }.bind(this));
   },
 
   /**
    * Return the height that the overscroll padding should be set to.
    * This is 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
    */
   overscrollHeight_: function() {
     var route = settings.getCurrentRoute();
     if (route.subpage.length != 0 || this.showPages_.about)
       return 0;
 
     var query = 'settings-section[section="' + route.section + '"]';
     var topSection = this.$$('settings-basic-page').$$(query);
     if (!topSection && this.showPages_.advanced)
       topSection = this.$$('settings-advanced-page').$$(query);
 
     if (!topSection || !topSection.offsetParent)
       return 0;
 
     // Offset to the selected section (relative to the scrolling window).
     let sectionTop = topSection.offsetParent.offsetTop + topSection.offsetTop;
     // The height of the selected section and remaining content (sections).
     let heightOfShownSections = this.$.overscroll.offsetTop - sectionTop;
     return Math.max(0, this.parentNode.scrollHeight - heightOfShownSections);

[Dan Beam, 2016/08/09 23:26:40]

so can we simplify any of this yet?

[michaelpg, 2016/08/09 23:46:09]

dschuyler? I think this is still necessary to *grow* the overscroll. We could
move the max(0, *) to setOverscroll_ but I don't mind it here.

[dschuyler, 2016/08/10 00:06:26]

I think that something called overscrollHeight_ should 
return a value >= 0.


Maybe some blank lines would help the readability?


Or maybe the comments are making it look 'heavy', maybe 'lighten'
it with something like:

var heightOfShownSections = this.$.overscroll.offsetTop - 
    topSection.offsetParent.offsetTop - topSection.offsetTop;

return Math.max(0, this.parentNode.scrollHeight - heightOfShownSections);

?

[michaelpg, 2016/08/10 03:12:27]

Ack (unchanged)
Did some things:

* s/topSection/section
* s/heightOfShownSections/distanceToBottom
* s/scrollHeight/clientHeight (they're the same; we want the client height,
anyway)
* offshored the settings-section querySelector to the pages, which already have
that method

   },
 
   /** @private */
   toggleAdvancedPage_: function() {
     this.fire('toggle-advanced-page', !this.advancedToggleExpanded_);
   },
 
   /**
    * Navigates to the default search page (if necessary).
    * @private
@@ skipping 41 matching lines @@
 
   /**
    * @param {(boolean|undefined)} visibility
    * @return {boolean} True unless visibility is false.
    * @private
    */
   showAdvancedSettings_: function(visibility) {
     return visibility !== false;
   },
 });