MD Settings: Hide the advanced page toggle control during searching.

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 25 matching lines @@
     },
 
     /**
      * Controls which main pages are displayed via dom-ifs.
      * @type {!{about: boolean, basic: boolean, advanced: boolean}}
      * @private
      */
     showPages_: {
       type: Object,
       value: function() {
         return {about: false, basic: false, advanced: false};

[Dan Beam, 2016/08/10 23:17:41]

can we just add the advanced toggle to this?

       },
     },
 
+    /**
+     * The main pages that were displayed before search was initiated.
+     * @type {?{about: boolean, basic: boolean, advanced: boolean}}
+     * @private
+     */
+    previousShowPages_: {
+      type: Object,
+      value: null,
+    },
+
     /** @private */
     showNoResultsFound_: {
       type: Boolean,
       value: false,
     },
 
     toolbarSpinnerActive: {
       type: Boolean,
       value: false,
       notify: true,
     },
 
     /**
+     * Whether search results are being displayed. Used to hide the advanced
+     * page toggle.
+     */
+    showingSearchResults_: {

[Dan Beam, 2016/08/10 23:17:41]

why do we need this if we can just check for previousShowPages_ existing?

[dpapad, 2016/08/10 23:39:13]

I am using showingSearchResults as a boolean var from the HTML.

Perhaps we could get away with |prevousShowPages_| only, but my initial thinking
is that it will be more cryptic to have |previousShowPages_| serve a dual
purpose. For example the data binding in HTML will look like

<template is="dom-if" if="[[showAdvancedToggle_(showPages_.basic, inSubpage_,
previousShowPages_)]]">

I'll try merging those vars and upload in a new patch, so we can see if it is
better.

[dpapad, 2016/08/10 23:57:14]

Done in latest patch.

+      type: Boolean,
+      value: false,
+    },
+
+    /**
      * Dictionary defining page visibility.
      * @type {!GuestModePageVisibility}
      */
     pageVisibility: {
       type: Object,
       value: function() { return {}; },
     },
   },
 
   /** @override */
@@ skipping 32 matching lines @@
   /**
    * @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';
   },
 
   /**
-   * @param {boolean} showBasicPage
-   * @param {boolean} inSubpage
    * @return {boolean}
    * @private
    */
-  showAdvancedToggle_: function(showBasicPage, inSubpage) {
-    return showBasicPage && !inSubpage;
+  showAdvancedToggle_: function() {
+    return this.showPages_.basic && !this.inSubpage_ &&
+        !this.showingSearchResults_;

[Dan Beam, 2016/08/10 23:17:41]

why is this part of the check needed?

[dpapad, 2016/08/10 23:39:13]

This change causes the advanced toggle to be hidden when the page is displaying
search results, per the request of @tbuckley at
https://bugs.chromium.org/p/chromium/issues/detail?id=629696#c7.

   },
 
   /** @protected */
   currentRouteChanged: function(newRoute) {
     this.inSubpage_ = newRoute.subpage.length > 0;
     this.style.height = this.inSubpage_ ? '100%' : '';
 
     if (settings.Route.ABOUT.contains(newRoute)) {
       this.showPages_ = {about: true, basic: false, advanced: false};
     } else {
@@ skipping 59 matching lines @@
    */
   ensureInDefaultSearchPage_: function() {
     settings.navigateTo(settings.Route.BASIC);
   },
 
   /**
    * @param {string} query
    * @return {!Promise} A promise indicating that searching finished.
    */
   searchContents: function(query) {
+    if (!this.showingSearchResults_) {
+      // Store which pages are shown before search, so that they can be restored
+      // after the user clears the search results.
+      this.previousShowPages_ = this.showPages_;

[Dan Beam, 2016/08/10 23:17:41]

if this is available in the same scope, why do we need to bind to |this.|?

[dpapad, 2016/08/10 23:39:13]

We need to cache the previousShowPages on this, because it is used during
different invocations of searchContents(). Explaining further below.

1) user nagigates to chrome://md-settigns.
2) user invokes search 'foo', line 222 executes. Lines 257-258 do not execute
yet, because we are displaying the results of 'foo'.
3) user clicks on the 'x' icon in the search box to clear the results.
searchContents('') is called. line 222 is skipped because we are still
showingSearchResults_. After the SearchManager#search() resolves, line 257-258
executes because we are no longer showing search results.

Does that make sense? I would be happy to explain further if that is still
confusing.

+      this.showingSearchResults_ = true;
+    }
+
     this.ensureInDefaultSearchPage_();
     this.toolbarSpinnerActive = true;
 
     // Trigger rendering of the basic and advanced pages and search once ready.
     this.showPages_ = {about: false, basic: true, advanced: true};
 
     return new Promise(function(resolve, reject) {
       setTimeout(function() {
         var whenSearchDone = settings.getSearchManager().search(
             query, assert(this.$$('settings-basic-page')));
         assert(
             whenSearchDone ===
                 settings.getSearchManager().search(
                     query, assert(this.$$('settings-advanced-page'))));
 
         whenSearchDone.then(function(request) {
           resolve();
           if (!request.finished) {
             // Nothing to do here. A previous search request was canceled
             // because a new search request was issued before the first one
             // completed.
             return;
           }
 
           this.toolbarSpinnerActive = false;
+          this.showingSearchResults_ = !request.isSame('');
           this.showNoResultsFound_ =
-              !request.isSame('') && !request.didFindMatches();
+              this.showingSearchResults_ && !request.didFindMatches();
+
+          if (!this.showingSearchResults_) {
+            // Restore the pages that were shown before search was initiated.
+            this.showPages_ = assert(this.previousShowPages_);
+            this.previousShowPages_ = null;
+          }
         }.bind(this));
       }.bind(this), 0);
     }.bind(this));
   },
 
   /**
    * @param {(boolean|undefined)} visibility
    * @return {boolean} True unless visibility is false.
    * @private
    */
   showAdvancedSettings_: function(visibility) {
     return visibility !== false;
   },
 });