Adjustments to the fatnav and sidenav, and other CSS updates

chrome/common/extensions/docs/static/js/article.js

Index: chrome/common/extensions/docs/static/js/article.js
diff --git a/chrome/common/extensions/docs/static/js/article.js b/chrome/common/extensions/docs/static/js/article.js
index 1c8ef09f409ee44053c6c329d9394e53fad7e403..dc6093e835d2133d6d41b04d32ea5e20d9a0399c 100644
--- a/chrome/common/extensions/docs/static/js/article.js
+++ b/chrome/common/extensions/docs/static/js/article.js
@@ -13,7 +13,6 @@ var isLargerThanMobileQuery =
     window.matchMedia('screen and (min-width: 581px)');
 
 var sidebar = document.querySelector('.inline-toc');
-var sidebarOffsetTop = null;
 var articleBody = document.querySelector('[itemprop="articleBody"]');
 
 // Bomb out unless we're on an article page and have a TOC.
@@ -21,6 +20,10 @@ if (!(sidebar && articleBody)) {
   return;
 }
 
+var toc = sidebar.querySelector('#toc');
+var tocOffsetTop = null;

[not at google - send to devlin, 2014/06/03 17:44:00]

caching this doesn't save much, and I'm not sure what will happen if
toggleStickySidenav() is called before it's initialized, which seems very
possible.

can you make this a function instead?

[pearlchen, 2014/06/03 22:52:13]

But then this value would have to be calculated (sidebar.offsetParent.offsetTop
+ toc.offsetTop) on nearly every scroll event, rather than on just page load +
window resize.

I will update it so it initializes to the value that it's suppose to be.

On 2014/06/03 17:44:00, kalman wrote:

[not at google - send to devlin, 2014/06/05 17:09:40]

what's the problem with adding up a couple of values each time?

[pearlchen, 2014/06/05 19:38:46]

Doesn't continual reading of the DOM affect performance?

On 2014/06/05 17:09:40, kalman wrote:

+var debounceTimeout;
+
 function addPermalink(el) {
   el.classList.add('has-permalink');
   var id = el.id || el.textContent.toLowerCase().replace(' ', '-');
@@ -37,16 +40,23 @@ function addPermalinkHeadings(container) {
   }
 }
 
+function toggleStickySidenav(){
+   window.scrollY >= tocOffsetTop ? toc.classList.add('sticky') :
+                                       toc.classList.remove('sticky');

[not at google - send to devlin, 2014/06/03 17:44:00]

you can write this as toc.classList.toggle('sticky', window.scrollY >=
tocOffsetTop);

[pearlchen, 2014/06/05 19:38:47]

Done.

+}
+
 function onScroll(e) {
-  window.scrollY >= sidebarOffsetTop ? sidebar.classList.add('sticky') :
-                                       sidebar.classList.remove('sticky');
+  window.clearTimeout( debounceTimeout );

[not at google - send to devlin, 2014/06/03 17:44:00]

no spaces

[pearlchen, 2014/06/05 19:38:47]

Done.

+  debounceTimeout = window.setTimeout(toggleStickySidenav);
+  toggleStickySidenav();
 }
 
 function onMediaQuery(e) {
   if (e.matches) {
     // On tablet & desktop, show permalinks, manage TOC position.
     document.body.classList.remove('no-permalink');
-    sidebarOffsetTop = sidebar.offsetParent.offsetTop
+    sidebarOffsetTop = sidebar.offsetParent.offsetTop;
+    tocOffsetTop = sidebarOffsetTop + toc.offsetTop;
     document.addEventListener('scroll', onScroll);
   } else {
     // On mobile, hide permalinks. TOC is hidden, doesn't need to scroll.
@@ -62,14 +72,33 @@ articleBody.addEventListener('click', function(e) {
   }
 });
 
-sidebar.querySelector('#toc').addEventListener('click', function(e) {
-  var parent = e.target.parentElement;
-  if (e.target.localName == 'a' && parent.classList.contains('toplevel')) {
-    // Allow normal link click if  h2 toplevel heading doesn't have h3s.
-    if (parent.querySelector('.toc')) {
-      e.preventDefault();
-      parent.classList.toggle('active');
+toc.addEventListener('click', function(e) {
+  if (e.target.localName === 'a') {

[pearlchen, 2014/06/03 22:52:13]

Simplified this click callback by changing if to:
if (e.target.classList.contains('hastoc') &&
!e.target.parentElement.classList.contains('active')) {

+
+    // Clicking on subnav menu item where it's toplevel li is already open:
+    // do nothing
+    if (e.target.parentElement.parentElement.parentElement.

[not at google - send to devlin, 2014/06/03 17:44:00]

this looks like it could easily regress. could you use querySelector[All], maybe
like

[].indexOf.call(toc.querySelectorAll('.active a')) >= 0

[pearlchen, 2014/06/03 22:52:13]

This always come out false for me. However, I think I've simplified this part so
check out my next CL upload.

On 2014/06/03 17:44:00, kalman wrote:

[not at google - send to devlin, 2014/06/05 17:09:40]

i missed out the .length in my code sample, sorry

+          classList.contains('active')){
+      return;
+    }
+
+    // Clicking on a toplevel menu item:
+
+    // if already open, do nothing
+    if (e.target.parentElement.classList.contains('active')){

[not at google - send to devlin, 2014/06/03 17:44:00]

in fact maybe the selector only needs to be '.active *' then this check is
covered too?

+      return;
+    }
+
+    // otherwise, close any previously open subnavs
+    [].forEach.call(toc.querySelectorAll('.active'), function(li){

[not at google - send to devlin, 2014/06/03 17:44:00]

function(li) { not function(li){

[pearlchen, 2014/06/05 19:38:47]

Done.

+      li.classList.remove('active');
+    });
+
+    // then open the clicked one
+    if (e.target.classList.contains('hastoc')) {

[not at google - send to devlin, 2014/06/03 17:44:00]

do you need to step up the parent chain to find the element with 'hastoc'?

+      e.target.parentElement.classList.add('active');
     }
+
   }
 });
 
@@ -82,6 +111,7 @@ sidebar.querySelector('#toc').addEventListener('click', function(e) {
 
 isLargerThanMobileQuery.addListener(onMediaQuery);
 onMediaQuery(isLargerThanMobileQuery);
+toggleStickySidenav();
 
 addPermalinkHeadings(articleBody);