dom-ui settings: display bubbles for sub-page search results....

chrome/browser/resources/options/search_page.js

Index: chrome/browser/resources/options/search_page.js
===================================================================
--- chrome/browser/resources/options/search_page.js	(revision 71996)
+++ chrome/browser/resources/options/search_page.js	(working copy)
@@ -6,6 +6,77 @@
   const OptionsPage = options.OptionsPage;
 
   /**
+   * Encapsulated handling of a search bubble.
+   * @constructor
+   */
+  function SearchBubble(text) {
+    var el = cr.doc.createElement('div');
+    SearchBubble.decorate(el);
+    el.textContent = text;
+    return el;
+  }
+
+  SearchBubble.decorate = function(el) {
+    el.__proto__ = SearchBubble.prototype;
+    el.decorate();
+  };
+
+  SearchBubble.prototype = {
+    __proto__: HTMLDivElement.prototype,
+
+    decorate: function() {
+      this.className = 'search-bubble';
+
+      // We create a timer to periodically update the position of the bubble.
+      // While this isn't all that desirable, it's the only sure-fire way of
+      // making sure the bubbles stay in the correct location as sections
+      // may dynamically change size at any time.
+      var self = this;
+      this.intervalId = setInterval(this.updatePosition.bind(this), 250);
+    },
+
+    /**
+     * Clear the interval timer and remove the element from the page.
+     */
+    dispose: function() {
+      clearInterval(this.intervalId);
+
+      var parent = this.parentNode;
+      if (parent)
+        parent.removeChild(this);
+    },
+
+    /**
+     * Update the position of the bubble.  Called at creation time and then
+     * periodically while the bubble remains visible.
+     */
+    updatePosition: function() {
+      // This bubble is 'owned' by the next sibling.
+      var owner = this.nextSibling;
+
+      // If there isn't an offset parent, we have nothing to do.
+      if (!owner.offsetParent)
+        return;
+
+      // Position the bubble below the location of the owner.
+      var left = owner.offsetLeft + owner.offsetWidth / 2 -
+          this.offsetWidth / 2;
+      var top = owner.offsetTop + owner.offsetHeight;

[arv (Not doing code reviews), 2011/01/21 23:19:31]

How do we handle the case where the bubble is displayed outside the viewport?

Can chrome/browser/resources/shared/js/cr/ui/position_util.js help you?

[csilv, 2011/01/24 19:17:34]

We don't do anything special, the bubble will be positioned off-screen.  The
extra logic in position_util.js is not needed here (it's more useful for popup
menus/controls).  The positioning logic is slightly different as well (left or
right aligned, rather than centered.)

+
+      // Update the position in the CSS.  Cache the last values for
+      // best performance.
+      if (left != this.lastLeft) {
+        this.style.left = left + 'px';
+        this.lastLeft = left;
+      }
+      if (top != this.lastTop) {
+        this.style.top = top + 'px';
+        this.lastTop = top;
+      }
+    }
+  }
+
+  /**
    * Encapsulated handling of the search page.
    * @constructor
    */
@@ -108,17 +179,15 @@
         // Update the visible state of all top-level elements that are not
         // sections (ie titles, button strips).  We do this before changing
         // the page visibility to avoid excessive re-draw.
-        var length = page.pageDiv.childNodes.length;
+        var length = page.pageDiv.children.length;
         var childDiv;
         for (var i = 0; i < length; i++) {

[arv (Not doing code reviews), 2011/01/21 23:19:31]

The following pattern is slightly easier on the eyes.

for (var i = 0, child; child = children[i]; i++) {
  
}

but the current for loops are ok with me.

[csilv, 2011/01/24 19:17:34]

Done.

-          childDiv = page.pageDiv.childNodes[i];
-          if (childDiv.nodeType == document.ELEMENT_NODE) {
-            if (active) {
-              if (childDiv.nodeName.toLowerCase() != 'section')
-                childDiv.classList.add('search-hidden');
-            } else {
-              childDiv.classList.remove('search-hidden');
-            }
+          childDiv = page.pageDiv.children[i];
+          if (active) {
+            if (childDiv.nodeName != 'SECTION')

[arv (Not doing code reviews), 2011/01/21 23:19:31]

Use tagName instead

[csilv, 2011/01/24 19:17:34]

Done.

+              childDiv.classList.add('search-hidden');
+          } else {
+            childDiv.classList.remove('search-hidden');
           }
         }
 
@@ -129,9 +198,11 @@
         }
       }
 
-      // After hiding all page content, remove any highlighted matches.
-      if (!active)
+      // After hiding all page content, remove any search results.
+      if (!active) {
         this.unhighlightMatches_();
+        this.removeSearchBubbles_();
+      }
     },
 
     /**
@@ -141,9 +212,11 @@
      */
     setSearchText_: function(text) {
       var foundMatches = false;
+      var bubbleControls = [];
 
-      // Remove any highlighted matches.
+      // Remove any prior search results.
       this.unhighlightMatches_();
+      this.removeSearchBubbles_();
 
       // Generate search text by applying lowercase and escaping any characters
       // that would be problematic for regular expressions.
@@ -157,7 +230,7 @@
 
       // Initialize all sections.  If the search string matches a title page,
       // show sections for that page.
-      var page, pageMatch, childDiv;
+      var page, pageMatch, childDiv, length;
       var pagesToSearch = this.getSearchablePages_();
       for (var key in pagesToSearch) {
         page = pagesToSearch[key];
@@ -167,10 +240,10 @@
         }
         if (pageMatch)
           foundMatches = true;
-        for (var i = 0; i < page.pageDiv.childNodes.length; i++) {
-          childDiv = page.pageDiv.childNodes[i];
-          if (childDiv.nodeType == document.ELEMENT_NODE &&
-              childDiv.nodeName == 'SECTION') {
+        length = page.pageDiv.children.length;
+        for (var i = 0; i < length; i++) {
+          childDiv = page.pageDiv.children[i];
+          if (childDiv.nodeName == 'SECTION') {

[arv (Not doing code reviews), 2011/01/21 23:19:31]

tagName

[csilv, 2011/01/24 19:17:34]

Done. (here and elsewhere)

             if (pageMatch) {
               childDiv.classList.remove('search-hidden');
             } else {
@@ -181,6 +254,20 @@
       }
 
       if (searchText.length) {
+        // Search all top-level sections for anchored string matches.
+        for (var key in pagesToSearch) {
+          page = pagesToSearch[key];
+          length = page.pageDiv.children.length;
+          for (var i = 0; i < length; i++) {
+            childDiv = page.pageDiv.children[i];
+            if (childDiv.nodeName == 'SECTION' &&
+                this.performReplace_(regEx, replaceString, childDiv)) {
+              childDiv.classList.remove('search-hidden');
+              foundMatches = true;
+            }
+          }
+        }
+
         // Search all sub-pages, generating an array of top-level sections that
         // we need to make visible.
         var subPagesToSearch = this.getSearchableSubPages_();
@@ -188,31 +275,22 @@
         for (var key in subPagesToSearch) {
           page = subPagesToSearch[key];
           if (this.performReplace_(regEx, replaceString, page.pageDiv)) {
+            // Reveal the section for this search result.
             section = page.associatedSection;
             if (section)
               section.classList.remove('search-hidden');
-            controls = page.associatedControls;
+
+            // Identify any controls that should have bubbles.
+            var controls = page.associatedControls;
             if (controls) {
-              // TODO(csilv): highlight each control.
+              length = controls.length;
+              for (var i = 0; i < length; i++)
+                bubbleControls.push(controls[i]);
             }
 
             foundMatches = true;
           }
         }
-
-        // Search all top-level sections for anchored string matches.
-        for (var key in pagesToSearch) {
-          page = pagesToSearch[key];
-          for (var i = 0; i < page.pageDiv.childNodes.length; i++) {
-            childDiv = page.pageDiv.childNodes[i];
-            if (childDiv.nodeType == document.ELEMENT_NODE &&
-                childDiv.nodeName == 'SECTION' &&
-                this.performReplace_(regEx, replaceString, childDiv)) {
-              childDiv.classList.remove('search-hidden');
-              foundMatches = true;
-            }
-          }
-        }
       }
 
       // Configure elements on the search results page based on search results.
@@ -226,6 +304,11 @@
         $('searchPageInfo').classList.add('search-hidden');
         $('searchPageNoMatches').classList.remove('search-hidden');
       }
+
+      // Create search balloons for sub-page results.
+      length = bubbleControls.length;
+      for (var i = 0; i < length; i++)
+        this.createSearchBubble_(bubbleControls[i], text);
     },
 
     /**
@@ -302,6 +385,38 @@
     },
 
     /**
+     * Creates a search result bubble attached to an element.
+     * @param {Element} element An HTML element, usually a button.
+     * @param {string} text A string to show in the bubble.
+     * @private
+     */
+    createSearchBubble_: function(element, text) {

[arv (Not doing code reviews), 2011/01/21 23:19:31]

I would just make this a local function instead

+      // avoid appending multiple ballons to a button.
+      var sibling = element.previousSibling;

[arv (Not doing code reviews), 2011/01/21 23:19:31]

previousElementSibling so you can skip testing for classList

[csilv, 2011/01/24 19:17:34]

Done.

+      if (sibling && sibling.classList &&
+          sibling.classList.contains('search-bubble'))
+        return;
+
+      var parent = element.parentNode;

[arv (Not doing code reviews), 2011/01/21 23:19:31]

parentElement

[csilv, 2011/01/24 19:17:34]

Done.

+      if (parent) {
+        var bubble = SearchBubble(text);

[arv (Not doing code reviews), 2011/01/21 23:19:31]

missing new

[csilv, 2011/01/24 19:17:34]

Done.

+        parent.insertBefore(bubble, element);
+        bubble.updatePosition();
+      }
+    },
+
+    /**
+     * Removes all search match bubbles.
+     * @private
+     */
+    removeSearchBubbles_: function() {
+      var elements = document.querySelectorAll('.search-bubble');
+      var length = elements.length;
+      for (var i = 0; i < length; i++)
+        elements[i].dispose();
+    },
+
+    /**
      * Builds a list of top-level pages to search.  Omits the search page and
      * all sub-pages.
      * @returns {Array} An array of pages to search.