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

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

Index: chrome/browser/resources/settings/settings_main/settings_main.js
diff --git a/chrome/browser/resources/settings/settings_main/settings_main.js b/chrome/browser/resources/settings/settings_main/settings_main.js
index 676429b994c7533a5426cb7ced6b98673c6cadf0..15f104c01d543c589581cbcf56da071b57e9a965 100644
--- a/chrome/browser/resources/settings/settings_main/settings_main.js
+++ b/chrome/browser/resources/settings/settings_main/settings_main.js
@@ -47,6 +47,16 @@ Polymer({
       },
     },
 
+    /**
+     * 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,
@@ -60,6 +70,15 @@ Polymer({
     },
 
     /**
+     * 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}
      */
@@ -112,13 +131,12 @@ Polymer({
   },
 
   /**
-   * @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 */
@@ -198,6 +216,13 @@ Polymer({
    * @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;
 
@@ -223,8 +248,15 @@ Polymer({
           }
 
           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));