Separate Settings and Help from History and Extensions when settings-in-

chrome/browser/resources/uber/uber_frame.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.
 
 // This file contains the navigation controls that are visible on the left side
 // of the uber page. It exists separately from uber.js so that it may be loaded
 // in an iframe. Iframes can be layered on top of each other, but not mixed in
 // with page content, so all overlapping content on uber must be framed.
 
 <include src="../../../../ui/webui/resources/js/util.js"></include>
@@ skipping 59 matching lines @@
   }
 
   /**
    * Changes the selected nav control.
    * @param {Object} params Must contain pageId.
    */
   function changeSelection(params) {
     var navItem =
         document.querySelector('li[controls="' + params.pageId + '"]');
     setSelection(navItem);
+    showNavItems();

[Dan Beam, 2014/06/06 21:21:23]

shouldn't this only be invoked if IsSettingsWindowEnabled()?

[michaelpg, 2014/06/07 02:12:15]

I've made the nav items hidden to prevent the flicker you'd see in 
--enable-settings-window. Otherwise all nav items would show and then
two would disappear.

Because of that, showNavItems needs to be called at least once.
changeSelection is called the first time the page loads to set the
initial selection.

The alternative is to
1. use .style.display instead of hidden attribute
2. set display attributes to "none" if --enable-settings-window 
3. add "isSettingsWindow" to the data source for uber_frame.js
4. if !isSettingsWindow call showNavItems from changeSelection

which in the case of !isSettingsWindow would be faster for some
definition of speed. But this is simpler and I didn't think I should
be concerned about iterating over a document with 4 <li>s.

   }
 
   /**
    * Sets selection on the given nav item.
    * @param {boolean} newSelection The item to be selected.
    */
   function setSelection(newSelection) {
     var lastSelectedNavItem = document.querySelector('li.selected');
     if (lastSelectedNavItem !== newSelection) {
       newSelection.classList.add('selected');
       if (lastSelectedNavItem)

[Dan Beam, 2014/06/06 21:21:23]

^ it would seem that there might not be a selected nav item from this code

[michaelpg, 2014/06/07 02:12:15]

The page starts out without a selected nav item.

         lastSelectedNavItem.classList.remove('selected');
     }
   }
 
   /**
+   * Shows nav items belonging to the same group as the selected item.
+   */
+  function showNavItems() {
+    var navItems = document.querySelectorAll('li');
+    var selectedNavItem = document.querySelector('li.selected');

[Dan Beam, 2014/06/06 21:21:23]

what happens if this returns null?

[Dan Beam, 2014/06/06 21:21:23]

nit: make a

  function getSelectedNavItem() {
    return document.querySelector('li.selected');
  }

a re-use here and other places

[michaelpg, 2014/06/07 02:12:15]

Done.

[michaelpg, 2014/06/07 02:12:15]

bad things.. but.. it can't because of the ordering in changeSelection.
if the selected navItem didn't exist, setSelection would have thrown
TypeError before this call.

+
+    for (var i = 0; i < navItems.length; ++i) {
+      navItems[i].hidden = (navItems[i].getAttribute('group') !=
+                            selectedNavItem.getAttribute('group'));

[Dan Beam, 2014/06/06 21:21:23]

nit: needless ()

[Dan Beam, 2014/06/06 21:21:23]

you don't ever actually use selectedNavItem itself, I'd just recommend

  var selectedNavItem = getSelectectedNavItem();
  if (!selectedNavItem)  // or assert(selectedNavItem)
    return;

  var selectedGroup = selectedNavItem.getAttribute('group');
  // use |selectedGroup|

[michaelpg, 2014/06/07 02:12:15]

Done.

+    }
+  }
+
+  /**
    * Adjusts this frame's content to scrolls from the outer frame. This is done
    * to obscure text in RTL as a user scrolls over the content of this frame (as
    * currently RTL scrollbars still draw on the right).
    * @param {number} scroll document.body.scrollLeft of the content frame.
    */
   function adjustToScroll(scrollLeft) {
     assert(isRTL());
     document.body.style.webkitTransform = 'translateX(' + -scrollLeft + 'px)';
   }
 
@@ skipping 40 matching lines @@
   }
 
   return {
     onLoad: onLoad,
     setNavigationOverride: setNavigationOverride,
   };
 
 });
 
 document.addEventListener('DOMContentLoaded', uber_frame.onLoad);