Extensions Docs Server: Highlight links on left nav

chrome/common/extensions/docs/server2/static/js/sidebar.js

 // Copyright (c) 2012 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.
 
 (function() {
   /**
    * Toggles the display of nodes given the status of their associated controls.
    *
    * For each node passed to this function, check to see if a toggle has been
    * inserted into the node's parent.  If yes, change the state of the toggle
@@ skipping 40 matching lines @@
    * toggle control is clicked, any <ul> elements who are siblings of the
    * control are hidden/revealed as appropriate given the control's state.
    *
    * If a list item possesses the class "leftNavSelected" its ancestor <ul> is
    * revealed by default (it represents the current page).
    */
   function initSidebar() {
     var toc = document.getElementById('gc-toc');
     if (!toc)
       return;
+    var links = toc.getElementsByTagName('a');
+    var selectedNode = null;

[not at google - send to devlin, 2012/08/13 23:17:21]

nit: some separation between steps here would be nice. Like, above "var links
=",

// Figure out which of the links has been selected so that we can style it
differently.

[cduvall, 2012/08/13 23:46:10]

Done.

+    for (var i = 0; i < links.length; i++) {
+      if (links[i].href == document.location.href) {

[not at google - send to devlin, 2012/08/13 23:17:21]

href isn't quite right. If somebody navigates directly to an anchor then that
will be reflected in document.location.href, but not in links[i].href.

Better perhaps would be to compare links[i].getAttribute('href') (should be a
path) versus document.location.pathname.

also, nit: do

if (!condition)
  continue;
...
break;

rather than

if (condition) {
  ...
  break;
}

[cduvall, 2012/08/13 23:46:10]

Done.

+        links[i].className = 'leftNavSelected';
+        links[i].href = 'javascript:void(0);';

[not at google - send to devlin, 2012/08/13 23:17:21]

if you do removeAttribute('href') it would convey the intent more clearly I
think, and you wouldn't need to manually disable the link styling in CSS.

[cduvall, 2012/08/13 23:46:10]

Done.

+        selectedNode = links[i];
+        break;
+      }
+    }
     var items = toc.getElementsByTagName('li');

[not at google - send to devlin, 2012/08/13 23:17:21]

nit: as above,

// Blah.

[cduvall, 2012/08/13 23:46:10]

Done.

-    var selectedNode = null;
     for (var i = 0; i < items.length; i++) {
       var item = items[i];
-      if (item.className == 'leftNavSelected') {
-        selectedNode = item;
-      } else if (item.firstChild &&
-                 item.firstChild.tagName == 'SPAN') {
+      if (item.firstChild && item.firstChild.tagName == 'SPAN') {

[not at google - send to devlin, 2012/08/13 23:17:21]

if (!item.firstChild || item.firstChild.tagName != 'SPAN')
  continue;

?

[cduvall, 2012/08/13 23:46:10]

Done.

         // Only assign toggles to text nodes in the sidebar.
         var a = document.createElement('a');
         a.className = 'toggle selected';
         a.appendChild(document.createTextNode(' '));
         a.onclick = function() {
           toggleList(this.parentNode.getElementsByTagName('ul'));
         };
         item.firstChild.onclick = function() {
           toggleList(this.parentNode.getElementsByTagName('ul'));
         };
         item.insertBefore(a, item.firstChild);
         toggleList(item.getElementsByTagName('ul'));
       }
     }
     if (selectedNode) {
       revealAncestor(selectedNode);
     }
   };
 
   initSidebar();
 })()