Extensions Docs Server: Highlight links on left nav

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

Index: chrome/common/extensions/docs/server2/static/js/sidebar.js
diff --git a/chrome/common/extensions/docs/server2/static/js/sidebar.js b/chrome/common/extensions/docs/server2/static/js/sidebar.js
index da5ad4c273fc86b788d395ab593e9edb1095cfa5..ac164cf3ee4ea6e078a88cf6b90ce9ce4c02c1a8 100644
--- a/chrome/common/extensions/docs/server2/static/js/sidebar.js
+++ b/chrome/common/extensions/docs/server2/static/js/sidebar.js
@@ -58,14 +58,20 @@
     var toc = document.getElementById('gc-toc');
     if (!toc)
       return;
-    var items = toc.getElementsByTagName('li');
+    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.

     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';