[MD settings] hide scrollbar during animation to subpage

chrome/browser/resources/settings/settings_page/main_page_behavior.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.
 
 /**
  * Responds to route changes by expanding, collapsing, or scrolling to sections
  * on the page. Expanded sections take up the full height of the container. At
  * most one section should be expanded at any given time.
  * @polymerBehavior MainPageBehavior
  */
 var MainPageBehaviorImpl = {
   properties: {
     /**
+     * Help CSS to alter style during the horizontal swipe animation.
+     * Note that this is unrelated to the |currentAnimation_| (which refers to
+     * the vertical exapand animation).
+     */
+    isSubpageAnimating: {
+      reflectToAttribute: true,
+      type: Boolean,
+    },
+
+    /**
      * Whether a search operation is in progress or previous search results are
      * being displayed.
      * @private {boolean}
      */
     inSearchMode: {
       type: Boolean,
       value: false,
       observer: 'inSearchModeChanged_',
     },
   },
 
   /** @type {?HTMLElement} The scrolling container. */
   scroller: null,
 
+  listeners: {
+    'neon-animation-finish': 'onNeonAnimationFinish_'
+  },
+
   /** @override */
   attached: function() {
     if (this.domHost && this.domHost.parentNode.tagName == 'PAPER-HEADER-PANEL')
       this.scroller = this.domHost.parentNode.scroller;
     else
       this.scroller = document.body; // Used in unit tests.
   },
 
   /**
+   * Remove the is-animating attribute once the animation is complete.
+   * This may catch animations finishing more often than needed, which is not
+   * known to cause any issues (e.g. when animating from a shallower page to a
+   * deeper page; or when transitioning to the main page).
+   * @private
+   */
+  onNeonAnimationFinish_: function() {
+    this.isSubpageAnimating = false;
+  },
+
+  /**
    * @param {!settings.Route} newRoute
    * @param {settings.Route} oldRoute
    */
   currentRouteChanged: function(newRoute, oldRoute) {
     var oldRouteWasSection =
         !!oldRoute && !!oldRoute.parent && !!oldRoute.section &&
         oldRoute.parent.section != oldRoute.section;
 
     // Always scroll to the top if navigating from a section to the root route
     // or when navigating to the About page.
     if (this.scroller &&
         ((oldRouteWasSection && newRoute == settings.Route.BASIC) ||
          newRoute == settings.Route.ABOUT)) {
       this.scroller.scrollTop = 0;
       return;
     }
 
     // Scroll to the section except for back/forward. Also scroll for any
     // in-page back/forward navigations (from a section or the root page).
     var scrollToSection =
         !settings.lastRouteChangeWasPopstate() || oldRouteWasSection ||
         oldRoute == settings.Route.BASIC;
 
+    if (oldRoute && newRoute.isSubpage() && oldRoute.depth > newRoute.depth)

[Dan Beam, 2017/03/21 07:12:50]

i'm still confused.

wouldn't we want to handle chopping off overflow when navigating from a subpage
to a subsubpage as well as from subsubpage to subpage?

does this accomplish that?  seems like it only addresses subsubpage -> subpage

[dschuyler, 2017/03/21 18:19:18]

To the best of my knowledge an erroneous scroll bar does not appear during that
transition. Even running a 1/10 speed (using the animation dev control) that
transition looks ok. I think I was looking at it as a hack/hammer to be used
sparingly, but I named that var isSubpageAnimating which is more general than a
var name like hideErroneousScrollbar. It sounds like I should either change the
var name, comment the existing var more clearly, or maybe change the test to
something like:
if (oldRoute && (oldRoute.isSubpage() || newRoute.isSubpage())
since a subpage is actually animating while going into a sub-subpage.

[Dan Beam, 2017/03/21 20:39:28]

how about name hideOverflow?
yeah, this ^ conditional is what i was thinking but what about loading
chrome://settings/content/cookies directly?  do we need to hide overflow for
this?  navigating directly to a subpage in a situation in which !oldRoute might
be true.

[dschuyler, 2017/03/22 00:58:50]

That would work for me :), but it seems to be 'presentational', which this style
guide advises against:
https://google.github.io/styleguide/htmlcssguide.html#ID_and_Class_Naming.

I'll make the isSubpageAnimation accurate instead.
 
This is done. So it is now really an 'is a subpage animating' variable. 
When going directly to the cookies page, it does the vertical expand animation
rather than a horizontal swipe animation (so the horizontal swipe issue hasn't
been seen there).

+      this.isSubpageAnimating = true;
+
     // For previously uncreated pages (including on first load), allow the page
     // to render before scrolling to or expanding the section.
     if (!oldRoute || this.scrollHeight == 0)
       setTimeout(this.tryTransitionToSection_.bind(this, scrollToSection));
     else
       this.tryTransitionToSection_(scrollToSection);
   },
 
   /**
    * When exiting search mode, we need to make another attempt to scroll to
@@ skipping 234 matching lines @@
     return /** @type {?SettingsSectionElement} */(
         this.$$('settings-section[section="' + section + '"]'));
   },
 };
 
 /** @polymerBehavior */
 var MainPageBehavior = [
   settings.RouteObserverBehavior,
   MainPageBehaviorImpl,
 ];