Merge pull request #416 from canjs/sidebar-fix

Fix an issue with groups not being expanded when a page in a non-Core package is selected

Author: chasenlehara

sidebar/README.md

@@ -0,0 +1,34 @@
+# Sidebar Docs
+
+## Architecture
+
+Most of the logic for the sidebar is broken up into three files:
+
+- [page-model.js](page-model.js) represents any documented module/page/etc.
+- [sidebar.viewmodel.js](sidebar.viewmodel.js) has everything else that’s needed for the template
+- [sidebar.events.js](sidebar.events.js) handles button & links clicks and the animations
+
+### Page model
+
+In addition to storing the data about each module/page/etc., the page model
+is responsible for some stateful information about how the page is being
+displayed in the template:
+
+- `parentPage` is set by the view-model when the page is initialized
+- `unsortedChildren` contains _all_ the children before they’re sorted
+- `sortedChildren` contains _all_ the children after sorting
+- `childrenInCoreCollection` contains only the children that are part of `can-core`
+- `isCollapsible` is only true for pages like Observables, Views, etc.
+- `isCollapsed` uses local storage to track whether the user has collapsed a section
+- `visibleChildren` returns either `childrenInCoreCollection` or `sortedChildren`, depending on `isCollapsed`
+
+Additionally, the `collapse` method is used to update local storage when
+`isCollapsed` is changed by the user. It should only be called when
+initiated by the user; any other (programatic) changes to `isCollapsed`
+should not be persisted to local storage.
+
+## Debugging
+
+The demo files make it easier to debug issues without having to rebuild the
+entire site. For example, you can test how the sidebar starts with an
+initial page by changing `selectedPageName: 'canjs'` in [demo.js](demo.js).

sidebar/sidebar.viewmodel.js

@@ -98,10 +98,16 @@ module.exports = DefineMap.extend({
       if (selectedPage) {
         if (selectedPage.isCollapsed && selectedPage.visibleChildren.length === 0) {
           selectedPage.isCollapsed = false;
-        } else {
-          var selectedParent = selectedPage.parentPage;
-          if (selectedParent && selectedParent.isCollapsible && selectedPage.collection !== 'can-core') {
-            selectedParent.isCollapsed = false;
+
+        } else if (selectedPage.collection !== 'can-core') {
+          // If the page is not in the core collection, walk up the parents to
+          // make sure they are not collapsed.
+          var parent = (selectedPage) ? selectedPage.parentPage : null;
+          while (parent) {
+            if (parent.isCollapsible) {
+              parent.isCollapsed = false;
+            }
+            parent = parent.parentPage;
           }
         }
       }

sidebar/test.js

@@ -157,6 +157,17 @@ QUnit.test('When a child item is selected, its parent should not be collapsed',
   assert.notOk(vm.shouldShowExpandCollapseButton(canAjaxParent), 'parent does not show expand/collapse button');
 });
 
+QUnit.test('When a grandchild item is selected, its grandparent should not be collapsed', function(assert) {
+  var vm = new ViewModel({searchMap: searchMap});
+  var pageMap = vm.pageMap;
+  var grandchildPage = pageMap['can-view-live.attr'];
+  var grandparentPage = grandchildPage.parentPage.parentPage;
+  assert.ok(grandparentPage.isCollapsed, 'grandparent is collapsed beforehand');
+  vm.selectedPage = grandchildPage;
+  assert.notOk(grandparentPage.isCollapsed, 'grandparent is not collapsed afterwards');
+  assert.notOk(vm.shouldShowExpandCollapseButton(grandparentPage), 'grandparent does not show expand/collapse button');
+});
+
 QUnit.test('When a purpose group page is selected, its expand/collapse button should be shown', function(assert) {
   var vm = new ViewModel({searchMap: searchMap});
   var canObservablesPage = vm.pageMap['can-observables'];