Tiny JavaScript live filter

Question

I'm looking for feedback on this live-filter I wrote in JavaScript. You can test it using the included snippet.

(function() {
  var searchField = document.getElementById('filter');

  var handler = function() {
    var text = searchField.value,
      nodes = document.getElementsByClassName('phase'),
      regexp = new RegExp(text, 'i');

    for (var i = 0; i < nodes.length; i++) {
      var node = nodes[i];

      node.text.search(regexp) < 0 ?
        node.parentNode.style.display = 'none' :
        node.parentNode.style.display = 'block';
    }
  };

  searchField.onkeyup = handler;
  searchField.onchange = handler;
})();

<input type="text" id="filter" />
<ul id="list">
  <li><a class="phase">Ruby on Rails</a></li>
  <li><a class="phase">The Rails Way</a></li>
  <li><a class="phase">Pickaxe Guide</a></li>
  <li><a class="phase">jQuery HTML</a></li>
  <li><a class="phase">CSS HTML AJAX</a></li>
  <li><a class="phase">HTML5 Net Set</a></li>
  <li><a class="phase">Node Easy</a></li>
  <li><a class="phase">Listing Bloop</a></li>
</ul>

Answer 1

So, a few notes:

• Your filtering is too broad. If you had other elements in the DOM with class phase, they would also be filtered, even though they aren't part of the list. Instead, it's better to limit what you're selecting -- DOM elements have querySelectorAll, getElementsBy..., etc., to ensure that you're limiting the results to that element's children.

• This can't be re-used very well -- make it more modular. You can do this by passing in the ID of the search field and the list (and any additional selectors).

• I'd suggest using classes to toggle visibility, because at some point you might actually want the styling to be different. Note that classList is only available on modern browsers (http://caniuse.com/#feat=classlist).

• Cache your lengths when iterating over an Array-like.

• There's no need for parentNode if you use textContent. Then you can query for all lis in the list and filter them instead. This also means you don't have to add classes for each list item unless you want to. In my example, I've removed the class='phase' portion of your anchors.

• I assume your items are eventually going to be links to elsewhere, so I left the anchors in the example.

var liveFilter = (function() {
  /**
   * Initializes a search field with associated list and filters it live while typing
   *
   * @param {string} searchFieldId     An ID of a search field input element
   * @param {string} listId            An ID of an element containing items that should be filtered
   * @param {object} [options]         Specifies additional options:
   *                                   * selector: specifies a query to be run against list; the results
   *                                     will be filtered according to the value of the search field. If
   *                                     not specified, the filter matches all direct children.
   *                                   * filterClass: specifies a class name to be applied to items that
   *                                     are going to be filtered out. If not specified, defaults to
   *                                     "filter-hidden".
   */
  function liveFilter( searchFieldId, listId, options) {
    
    var searchField = document.getElementById(searchFieldId),
        list = document.getElementById(listId),
        selector,
        filterClass;
    
    if (!searchField) {
      throw new Error("No search element found with id " + searchFieldId);
    }
    
    if (!list) {
      throw new Error("No list element found with id " + listId);
    }
    
    if (options !== undefined) {
      selector = options.selector;
      filterClass = options.filterClass;
    }
        
    if (filterClass === undefined) {
      filterClass = "filter-hidden";
    }

    var handler = function searchFieldChanged() {
      var text = this.value,     // this will be bound to the search field
          regexp = RegExp(text, 'i'),
          nodes = (selector === undefined) ? list.children 
                                           : list.querySelectorAll(selector),
          node;
      for (var i = 0, l = nodes.length; i < l; i++) {
        node = nodes[i];
        if (node.textContent.search(regexp) < 0) {
          // note: classList is only available on modern browsers
          node.classList.add(filterClass);
        } else {
          node.classList.remove(filterClass);
        }
      }
    };

    // input is for modern browsers
    searchField.addEventListener("input", handler, false);
  }
  
  // If using CommonJS, you'd do module.exports = liveFilter, and it could be required later.
  return liveFilter;
})();

liveFilter("filter", "list", { selector: "li", filterClass: "filter-hidden" });

.filter-hidden {
  display: none;
}

<input type="text" id="filter" />
<ul id="list">
  <li><a>Ruby on Rails</a></li>
  <li><a>The Rails Way</a></li>
  <li><a>Pickaxe Guide</a></li>
  <li><a>jQuery HTML</a></li>
  <li><a>CSS HTML AJAX</a></li>
  <li><a>HTML5 Net Set</a></li>
  <li><a>Node Easy</a></li>
  <li><a>Listing Bloop</a></li>
</ul>

Answer 2

A few comments:

1) You don't need the new keyword to use RegExp:

regexp = RegExp(text, 'i');

2) The ternary operator can be used as an expression anywhere, such as an assignment. Use textContent instead of text. Then you can use the regex test method which returns a boolean:

node.parentNode.style.display = node.textContent.test(regexp) ? 'block' : 'none';

Since this becomes a bit too long of a line, it would be better to cache the condition:

var mustShow = regexp.test(node.textContent);
node.parentNode.style.display = mustShow ? 'block' : 'none';

4) Finally, use addEventListener instead of the onevent properties. If you don't need to support ancient browsers, use the input event:

searchField.addEventListener('input', handler);

With a couple more edits, this is how it would look like:

;(function() {
  var searchField = document.getElementById('filter');

  var handler = function() {
    var nodes = document.getElementsByClassName('phase'),
        regexp = RegExp(searchField.value, 'i');

    for (var i = 0; i < nodes.length; i++) {
      var node = nodes[i];
      var mustShow = regexp.test(node.textContent);
      node.parentNode.style.display = mustShow ? 'block' : 'none';
    }
  };

  searchField.addEventListener('input', handler);
}());