MD Settings: Remove subpage animation when landing directly on it.

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
  */
@@ skipping 17 matching lines @@
     inSearchMode: {
       type: Boolean,
       value: false,
       observer: 'inSearchModeChanged_',
     },
   },
 
   /** @type {?HTMLElement} The scrolling container. */
   scroller: null,
 
-  listeners: {
-    'neon-animation-finish': 'onNeonAnimationFinish_'
-  },
+  listeners: {'neon-animation-finish': 'onNeonAnimationFinish_'},
 
   /** @override */
   attached: function() {
     this.scroller = this.domHost ? this.domHost.parentNode : document.body;
   },
 
   /**
    * 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
@@ skipping 32 matching lines @@
     // Also always scroll when coming from either the About or root page.
     var scrollToSection = !settings.lastRouteChangeWasPopstate() ||
         oldRouteWasSection || oldRoute == settings.Route.BASIC ||
         oldRoute == settings.Route.ABOUT;
 
     if (oldRoute && (oldRoute.isSubpage() || newRoute.isSubpage()))
       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
+    if (!oldRoute || this.scrollHeight == 0) {
+      document.body.querySelector('settings-ui::shadow #container')

[michaelpg, 2017/04/22 04:01:58]

::shadow makes me sad, is there no other way to get to the container?

[scottchen, 2017/04/26 17:26:40]

I changed settings-ui to pass a "shouldHideContainer" flag that can be changed
by mainPageBehavior.

+          .setAttribute('hidden', true);

[scottchen, 2017/04/19 18:05:22]

without this line, the basic page will briefly appear before landing on the
subpage:
https://drive.google.com/a/google.com/file/d/0ByIc0PEad7DmN1IzQnR0TDFYQkE/vie...

[michaelpg, 2017/04/22 04:01:58]

Yeah, it's probably necessary unless we want to make bigger changes. It would be
nice if we could call toggleOtherSectionsHidden, but that doesn't work if the
other sections aren't stamped yet >_<

[scottchen, 2017/04/26 17:26:40]

Acknowledged.

+      setTimeout(
+          this.tryTransitionToSection_.bind(this, scrollToSection, true));
+    } else
       this.tryTransitionToSection_(scrollToSection);
   },
 
   /**
    * When exiting search mode, we need to make another attempt to scroll to
    * the correct section, since it has just been re-rendered.
    * @private
    */
   inSearchModeChanged_: function(inSearchMode) {
     if (!this.isAttached)
       return;
 
     if (!inSearchMode)
       this.tryTransitionToSection_(!settings.lastRouteChangeWasPopstate());
   },
 
   /**
    * If possible, transitions to the current route's section (by expanding or
    * scrolling to it). If another transition is running, finishes or cancels
    * that one, then schedules this function again. This ensures the current
    * section is quickly shown, without getting the page into a broken state --
    * if currentRoute changes in between calls, just transition to the new route.
    * @param {boolean} scrollToSection
    * @private
    */
-  tryTransitionToSection_: function(scrollToSection) {
+  tryTransitionToSection_: function(scrollToSection, immediate) {
+    document.body.querySelector('settings-ui::shadow #container')

[michaelpg, 2017/04/22 04:01:58]

Does this only take effect on the first call? Perhaps it's better to unset this
up on line 95, in the setTimeout (the same place we add the attribute) before
calling tryTransitionToSection.

e.g., something like
  setTimeout(function() {
    document.body......removeAttribute('hidden');
    this.tryTransitionToSection_(scrollToSection, true);
  }.bind(this));

[scottchen, 2017/04/26 17:26:40]

Done.

+        .removeAttribute('hidden');
     var currentRoute = settings.getCurrentRoute();
     var currentSection = this.getSection(currentRoute.section);
 
     // If an animation is already playing, try finishing or canceling it.
     if (this.currentAnimation_) {
       this.maybeStopCurrentAnimation_();
       // Either way, this function will be called again once the current
       // animation ends.
       return;
     }
 
     var promise;
     var expandedSection = /** @type {?SettingsSectionElement} */(
         this.$$('settings-section.expanded'));
     if (expandedSection) {
       // If the section shouldn't be expanded, collapse it.
       if (!currentRoute.isSubpage() || expandedSection != currentSection) {
         promise = this.collapseSection_(expandedSection);
       } else {
         // Scroll to top while sliding to another subpage.
         this.scroller.scrollTop = 0;
       }
     } else if (currentSection) {
       // Expand the section into a subpage or scroll to it on the main page.
       if (currentRoute.isSubpage())
-        promise = this.expandSection_(currentSection);
+        promise = this.expandSection_(currentSection, immediate);
       else if (scrollToSection)
         currentSection.scrollIntoView();
     } else if (
         this.tagName == 'SETTINGS-BASIC-PAGE' &&
         settings.Route.ADVANCED.contains(currentRoute) &&
         // Need to exclude routes that correspond to 'non-sectioned' children of
         // ADVANCED, otherwise tryTransitionToSection_ will recurse endlessly.
         !currentRoute.isNavigableDialog) {
       assert(currentRoute.section);
       promise = this.$$('#advancedPageTemplate').get();
@@ skipping 42 matching lines @@
     // Immediately finish the current collapse animation so that can happen.
     this.currentAnimation_.finish();
   },
 
   /**
    * Animates the card in |section|, expanding it to fill the page.
    * @param {!SettingsSectionElement} section
    * @return {!Promise} Resolved when the transition is finished or canceled.
    * @private
    */
-  expandSection_: function(section) {
+  expandSection_: function(section, immediate) {
     assert(this.scroller);
 
+    if (immediate) {
+      section.immediateExpand(this.scroller);
+
+      this.classList.add('showing-subpage');
+      this.toggleOtherSectionsHidden_(section.section, true);
+      this.scroller.scrollTop = 0;

[michaelpg, 2017/04/22 04:01:58]

when is this not starting at 0?

[scottchen, 2017/04/26 17:26:41]

Never, this is not necessary. Removing.

+      // Notify that the page is fully expanded.
+      this.fire('subpage-expand');
+      return;
+    }
+
     if (!section.canAnimateExpand()) {
       // Try to wait for the section to be created.

[michaelpg, 2017/04/22 04:01:58]

We *might* not need this anymore if the goal is to show sub-pages immediately on
load, so you could replace this with the body of the `if` above and not have to
add the |immediate| parameter.

(But maybe it's still necessary if we haven't stamped Advanced yet, and we
navigate to an Advanced subpage?)

[scottchen, 2017/04/26 17:26:40]

Using !section.canAnimateExpand() to indicate first-load doesn't seem explicit
enough - canAnimateExpand() can also return false if it's half way through the
expanding animation, so I think it's safer to keep them separate.

       return new Promise(function(resolve, reject) {
         setTimeout(resolve);
       });
     }
 
     // Save the scroller position before freezing it.
     this.origScrollTop_ = this.scroller.scrollTop;
     this.fire('freeze-scroll', true);
 
     // Freeze the section's height so its card can be removed from the flow.
@@ skipping 120 matching lines @@
     return /** @type {?SettingsSectionElement} */(
         this.$$('settings-section[section="' + section + '"]'));
   },
 };
 
 /** @polymerBehavior */
 var MainPageBehavior = [
   settings.RouteObserverBehavior,
   MainPageBehaviorImpl,
 ];