[MD settings] hide scrollbar during animation to subpage

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

Index: chrome/browser/resources/settings/settings_page/main_page_behavior.js
diff --git a/chrome/browser/resources/settings/settings_page/main_page_behavior.js b/chrome/browser/resources/settings/settings_page/main_page_behavior.js
index 8fdf5f7e6e61f2e949d67e6f8acaa3df9fce936d..dd57065ed1bd1890c0403750f17dedb17ca49860 100644
--- a/chrome/browser/resources/settings/settings_page/main_page_behavior.js
+++ b/chrome/browser/resources/settings/settings_page/main_page_behavior.js
@@ -11,6 +11,16 @@
 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}
@@ -25,6 +35,10 @@ var MainPageBehaviorImpl = {
   /** @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')
@@ -34,6 +48,17 @@ var MainPageBehaviorImpl = {
   },
 
   /**
+   * 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
    */
@@ -57,6 +82,9 @@ var MainPageBehaviorImpl = {
         !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)