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..7be679b21afffedf89d13ffe9cc3d295fdc8a004 100644
--- a/chrome/common/extensions/docs/static/js/article.js
+++ b/chrome/common/extensions/docs/static/js/article.js
@@ -9,18 +9,20 @@
 // button: click logic, and when to show it.
 (function() {
 
-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.
+// Ideally, template shouldn't load this JS file on a non-article page
 if (!(sidebar && articleBody)) {
   return;
 }
 
+var isLargerThanMobileQuery =
+    window.matchMedia('screen and (min-width: 581px)');
+
+var toc = sidebar.querySelector('#toc');
+
 function addPermalink(el) {
   el.classList.add('has-permalink');
   var id = el.id || el.textContent.toLowerCase().replace(' ', '-');
@@ -37,17 +39,23 @@ function addPermalinkHeadings(container) {
   }
 }
 
+function toggleStickySidenav(){
+  // Note: Attempting to read offsetTop with sticky on causes toc overlap
+  toc.classList.remove('sticky');

[pearlchen, 2014/06/06 17:36:55]

(Documenting bug here since I don't know where else to note this.)

There is an overlap bug when tocOffsetTop is calculated when the toc is already
sticky. Fix is to remove sticky before reading offsetTop value.

Scenario #1:
1. Scroll down until TOC is sticky
2. Resize browser to be mobile width
3. Resize back to non-mobile width
4. Scroll up
--> TOC overlaps sidenav

Scenario #2:
1. Scroll down until TOC is sticky
2. Reload page
3. Scroll up
--> TOC overlaps sidenav

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

ah. so that is avoided by caching the tocOffsetTop value? ok, that's worth
avoiding.

+  var tocOffsetTop = sidebar.offsetParent.offsetTop + toc.offsetTop;

[pearlchen, 2014/06/06 17:36:56]

As requested, tocOffsetTop moved into this function as a local variable. 

Between this line and line 44, the DOM is read/written to twice on every scroll
event. Not happy about that from a performance standpoint but if having a local
variable is more important, then so be it.

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

is this performance hit by the adding/removal of the stick property? that likely
would cause a performance issue, in which case... yeah, it sounds like what you
had before is better after all. sorry about that!

[pearlchen, 2014/06/06 18:30:03]

From what I know, evening *reading* offsetTop will cause a browser reflow.

On 2014/06/06 17:40:16, kalman wrote:

+  toc.classList.toggle('sticky', window.scrollY >= tocOffsetTop);
+}
+
 function onScroll(e) {
-  window.scrollY >= sidebarOffsetTop ? sidebar.classList.add('sticky') :
-                                       sidebar.classList.remove('sticky');
+  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
     document.addEventListener('scroll', onScroll);
+    toggleStickySidenav();
   } else {
     // On mobile, hide permalinks. TOC is hidden, doesn't need to scroll.
     document.body.classList.add('no-permalink');
@@ -62,14 +70,20 @@ 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) {
+  // React only if clicking on a toplevel menu anchor item
+  // that is not currently open
+  if (e.target.classList.contains('hastoc') &&
+      !e.target.parentElement.classList.contains('active')) {
+    e.stopPropagation();
+
+    // close any previously open subnavs
+    [].forEach.call(toc.querySelectorAll('.active'), function(li) {
+      li.classList.remove('active');
+    });
+
+    // then open the clicked one
+    e.target.parentElement.classList.add('active');
   }
 });