fix(aio): sidebar folder state after select item

Closes #17245 and #17253

When the user selects a doc item in the side nav:
1) expand folder(s) leading to the selected doc item
2) on a wide display, keep other already expanded folders open
3) on narrow (mobile) display, collapse other expanded folders

Used to do (3) when wide. Issue #17245 asks for (2).

That logic was bypassed for selected node when we allowed headers to have content
because that unintentionally expanded the header’s folder when selected.
Because the selected node is no longer a header with content, removing this exclusion
also means that folders are expanded/collapsed with above logic even for API pages.

aio/e2e/app.e2e-spec.ts

@@ -27,9 +27,7 @@ describe('site App', function() {
     // Show the menu
     page.docsMenuLink.click();
 
-    // Open the tutorial header
+    // Tutorial folder should still be expanded because this test runs in wide mode
-    page.getNavItem(/tutorial/i).click();
-
     // Navigate to the tutorial introduction via a link in the sidenav
     page.getNavItem(/introduction/i).click();
     expect(page.getDocViewerText()).toMatch(/Tutorial: Tour of Heroes/i);

aio/src/app/app.component.html

@@ -18,8 +18,8 @@
 <md-sidenav-container class="sidenav-container" [class.starting]="isStarting" [class.has-floating-toc]="hasFloatingToc" role="main">
 
   <md-sidenav [ngClass]="{'collapsed': !isSideBySide }" #sidenav class="sidenav" [opened]="isOpened" [mode]="mode" (open)="updateHostClasses()" (close)="updateHostClasses()">
-    <aio-nav-menu *ngIf="!isSideBySide" [nodes]="topMenuNarrowNodes" [currentNode]="currentNodes?.TopBarNarrow"></aio-nav-menu>
+    <aio-nav-menu *ngIf="!isSideBySide" [nodes]="topMenuNarrowNodes" [currentNode]="currentNodes?.TopBarNarrow" [isWide]="false"></aio-nav-menu>
-    <aio-nav-menu [nodes]="sideNavNodes" [currentNode]="currentNodes?.SideNav" ></aio-nav-menu>
+    <aio-nav-menu [nodes]="sideNavNodes" [currentNode]="currentNodes?.SideNav" [isWide]="isSideBySide"></aio-nav-menu>
 
     <div class="doc-version" title="Angular docs version {{currentDocVersion?.title}}">
       <aio-select (change)="onDocVersionChange($event.index)" [options]="docVersions" [selected]="docVersions && docVersions[0]"></aio-select>

aio/src/app/layout/nav-item/nav-item.component.html

@@ -20,7 +20,7 @@
   </button>
 
   <div class="heading-children" [ngClass]="classes">
-    <aio-nav-item *ngFor="let node of node.children" [level]="level + 1"
+    <aio-nav-item *ngFor="let node of node.children" [level]="level + 1" [isWide]="isWide"
     [node]="node" [selectedNodes]="selectedNodes"></aio-nav-item>
   </div>
 </div>

aio/src/app/layout/nav-item/nav-item.component.spec.ts

@@ -1,159 +1,196 @@
-
+import { TestBed } from '@angular/core/testing';
-import { SimpleChange, SimpleChanges } from '@angular/core';
+import { By } from '@angular/platform-browser';
+import { SimpleChange, SimpleChanges, NO_ERRORS_SCHEMA } from '@angular/core';
 
 import { NavItemComponent } from './nav-item.component';
 import { NavigationNode } from 'app/navigation/navigation.model';
 
-// Testing the component class behaviors, independent of its template
+describe('NavItemComponent', () => {
-// No dependencies. Just new it and test :)
-// Let e2e tests verify how it displays.
-describe('NavItemComponent (class-only)', () => {
 
-  let component: NavItemComponent;
+  // Testing the component class behaviors, independent of its template
+  // No dependencies. Just new it and test :)
+  // Let e2e tests verify how it displays.
+  describe('(class-only)', () => {
 
-  let selectedNodes: NavigationNode[];
+    let component: NavItemComponent;
-  let setClassesSpy: jasmine.Spy;
 
-  function initialize(nd: NavigationNode) {
+    let selectedNodes: NavigationNode[];
-    component.node = nd;
+    let setClassesSpy: jasmine.Spy;
-    onChanges(); // Angular calls when initializing the component
-  }
 
-  // Enough to triggers component's ngOnChange method
+    function initialize(nd: NavigationNode) {
-  function onChanges() {
+      component.node = nd;
-    component.ngOnChanges({node: <SimpleChange><any> 'anything' });
+      onChanges(); // Angular calls when initializing the component
-  }
+    }
 
-  beforeEach(() => {
+    // Enough to triggers component's ngOnChange method
+    function onChanges() {
+      component.ngOnChanges({node: <SimpleChange><any> 'anything' });
+    }
 
-    component = new NavItemComponent();
+    beforeEach(() => {
-    setClassesSpy = spyOn(component, 'setClasses').and.callThrough();
 
-    // Selected nodes is the selected node and its header ancestors
+      component = new NavItemComponent();
-    selectedNodes = [
+      setClassesSpy = spyOn(component, 'setClasses').and.callThrough();
-      { title: 'a' },           // selected node: an item or a header
-      { title: 'parent' },      // selected node's header parent
-      { title: 'grandparent' }, // selected node's header grandparent
-    ];
-    component.selectedNodes = selectedNodes;
-  });
 
-  describe('should have expected classes when initialized', () => {
+      // Selected nodes is the selected node and its header ancestors
-    it('with selected node', () => {
+      selectedNodes = [
-      initialize(selectedNodes[0]);
+        { title: 'a' },           // selected node: an item or a header
-      expect(component.classes).toEqual(
+        { title: 'parent' },      // selected node's header parent
-        // selecting the current node has no effect on expanded state,
+        { title: 'grandparent' }, // selected node's header grandparent
-        // even if current node is a header.
+      ];
-        { 'level-1': true, collapsed: true, expanded: false, selected: true}
+      component.selectedNodes = selectedNodes;
-      );
     });
 
-    it('with selected node ancestor', () => {
+    describe('should have expected classes when initialized', () => {
-      initialize(selectedNodes[1]);
+      it('with selected node', () => {
-      expect(component.classes).toEqual(
+        initialize(selectedNodes[0]);
-        // ancestor is a header and should be expanded
+        expect(component.classes).toEqual(
-        { 'level-1': true, collapsed: false, expanded: true, selected: true}
+          // selected node should be expanded even if is a header.
-      );
+          { 'level-1': true, collapsed: false, expanded: true, selected: true }
+        );
+      });
+
+      it('with selected node ancestor', () => {
+        initialize(selectedNodes[1]);
+        expect(component.classes).toEqual(
+          // ancestor is a header and should be expanded
+          { 'level-1': true, collapsed: false, expanded: true, selected: true }
+        );
+      });
+
+      it('with other than a selected node or ancestor', () => {
+        initialize({ title: 'x' });
+        expect(component.classes).toEqual(
+          { 'level-1': true, collapsed: true, expanded: false, selected: false }
+        );
+      });
     });
 
-    it('with other than a selected node or ancestor', () => {
+    describe('when becomes a non-selected node', () => {
-      initialize({ title: 'x' });
+
-      expect(component.classes).toEqual(
+      // this node won't be the selected node when ngOnChanges() called
-        { 'level-1': true, collapsed: true, expanded: false, selected: false}
+      beforeEach(() => component.node = { title: 'x' });
-      );
+
+      it('should de-select if previously selected', () => {
+        component.isSelected = true;
+        onChanges();
+        expect(component.isSelected).toBe(false, 'becomes de-selected');
+      });
+
+      it('should collapse if previously expanded in narrow mode', () => {
+        component.isWide = false;
+        component.isExpanded = true;
+        onChanges();
+        expect(component.isExpanded).toBe(false, 'becomes collapsed');
+      });
+
+      it('should remain expanded in wide mode', () => {
+        component.isWide = true;
+        component.isExpanded = true;
+        onChanges();
+        expect(component.isExpanded).toBe(true, 'remains expanded');
+      });
+    });
+
+    describe('when becomes a selected node', () => {
+
+      // this node will be the selected node when ngOnChanges() called
+      beforeEach(() => component.node = selectedNodes[0]);
+
+      it('should select when previously not selected', () => {
+        component.isSelected = false;
+        onChanges();
+        expect(component.isSelected).toBe(true, 'becomes selected');
+      });
+
+      it('should expand the current node or keep it expanded', () => {
+        component.isExpanded = false;
+        onChanges();
+        expect(component.isExpanded).toBe(true, 'becomes true');
+
+        component.isExpanded = true;
+        onChanges();
+        expect(component.isExpanded).toBe(true, 'remains true');
+      });
+    });
+
+    describe('when becomes a selected ancestor node', () => {
+
+      // this node will be a selected node ancestor header when ngOnChanges() called
+      beforeEach(() => component.node = selectedNodes[2]);
+
+      it('should select when previously not selected', () => {
+        component.isSelected = false;
+        onChanges();
+        expect(component.isSelected).toBe(true, 'becomes selected');
+      });
+
+      it('should always expand this header', () => {
+        component.isExpanded = false;
+        onChanges();
+        expect(component.isExpanded).toBe(true, 'becomes expanded');
+
+        component.isExpanded = false;
+        onChanges();
+        expect(component.isExpanded).toBe(true, 'stays expanded');
+      });
+    });
+
+    describe('when headerClicked()', () => {
+      // current node doesn't matter in these tests.
+
+      it('should expand when headerClicked() and previously collapsed', () => {
+        component.isExpanded = false;
+        component.headerClicked();
+        expect(component.isExpanded).toBe(true, 'should be expanded');
+      });
+
+      it('should collapse when headerClicked() and previously expanded', () => {
+        component.isExpanded = true;
+        component.headerClicked();
+        expect(component.isExpanded).toBe(false, 'should be collapsed');
+      });
+
+      it('should not change isSelected when headerClicked()', () => {
+        component.isSelected = true;
+        component.headerClicked();
+        expect(component.isSelected).toBe(true, 'remains selected');
+
+        component.isSelected = false;
+        component.headerClicked();
+        expect(component.isSelected).toBe(false, 'remains not selected');
+      });
+
+      it('should set classes', () => {
+        component.headerClicked();
+        expect(setClassesSpy).toHaveBeenCalled();
+      });
     });
   });
 
-  describe('when becomes a non-selected node', () => {
+  describe('(via TestBed)', () => {
+    it('should pass the `isWide` property to all child nav-items', () => {
+      TestBed.configureTestingModule({
+        declarations: [NavItemComponent],
+        schemas: [NO_ERRORS_SCHEMA]
+      });
+      const fixture = TestBed.createComponent(NavItemComponent);
+      fixture.componentInstance.node = {
+        title: 'x',
+        children: [{ title: 'a' }, { title: 'b' }]
+      };
 
-    // this node won't be the selected node when ngOnChanges() called
+      fixture.componentInstance.isWide = true;
-    beforeEach(() => component.node = { title: 'x' });
+      fixture.detectChanges();
+      let children = fixture.debugElement.queryAll(By.directive(NavItemComponent));
+      expect(children.length).toEqual(2);
+      children.forEach(child => expect(child.componentInstance.isWide).toBe(true));
 
-    it('should collapse if previously expanded', () => {
+      fixture.componentInstance.isWide = false;
-      component.isExpanded = true;
+      fixture.detectChanges();
-      onChanges();
+      children = fixture.debugElement.queryAll(By.directive(NavItemComponent));
-      expect(component.isExpanded).toBe(false, 'becomes collapsed');
+      expect(children.length).toEqual(2);
-    });
+      children.forEach(child => expect(child.componentInstance.isWide).toBe(false));
-
-    it('should de-select if previously selected', () => {
-      component.isSelected = true;
-      onChanges();
-      expect(component.isSelected).toBe(false, 'becomes de-selected');
-    });
-  });
-
-  describe('when becomes a selected node', () => {
-
-    // this node will be the selected node when ngOnChanges() called
-    beforeEach(() => component.node = selectedNodes[0]);
-
-    it('should select when previously not selected', () => {
-      component.isSelected = false;
-      onChanges();
-      expect(component.isSelected).toBe(true, 'becomes selected');
-    });
-
-    it('should leave the expanded/collapsed state untouched', () => {
-      component.isExpanded = false;
-      onChanges();
-      expect(component.isExpanded).toBe(false, 'remains false');
-
-      component.isExpanded = true;
-      onChanges();
-      expect(component.isExpanded).toBe(true, 'remains true');
-    });
-  });
-
-  describe('when becomes a selected ancestor node', () => {
-
-    // this node will be a selected node ancestor header when ngOnChanges() called
-    beforeEach(() => component.node = selectedNodes[2]);
-
-    it('should select when previously not selected', () => {
-      component.isSelected = false;
-      onChanges();
-      expect(component.isSelected).toBe(true, 'becomes selected');
-    });
-
-    it('should always expand this header', () => {
-      component.isExpanded = false;
-      onChanges();
-      expect(component.isExpanded).toBe(true, 'becomes expanded');
-
-      component.isExpanded = false;
-      onChanges();
-      expect(component.isExpanded).toBe(true, 'stays expanded');
-    });
-  });
-
-  describe('when headerClicked()', () => {
-    // current node doesn't matter in these tests.
-
-    it('should expand when headerClicked() and previously collapsed', () => {
-      component.isExpanded = false;
-      component.headerClicked();
-      expect(component.isExpanded).toBe(true, 'should be expanded');
-    });
-
-    it('should collapse when headerClicked() and previously expanded', () => {
-      component.isExpanded = true;
-      component.headerClicked();
-      expect(component.isExpanded).toBe(false, 'should be collapsed');
-    });
-
-    it('should not change isSelected when headerClicked()', () => {
-      component.isSelected = true;
-      component.headerClicked();
-      expect(component.isSelected).toBe(true, 'remains selected');
-
-      component.isSelected = false;
-      component.headerClicked();
-      expect(component.isSelected).toBe(false, 'remains not selected');
-    });
-
-    it('should set classes', () => {
-      component.headerClicked();
-      expect(setClassesSpy).toHaveBeenCalled();
     });
   });
 });

aio/src/app/layout/nav-item/nav-item.component.ts

@@ -6,20 +6,23 @@ import { NavigationNode } from 'app/navigation/navigation.model';
   templateUrl: 'nav-item.component.html',
 })
 export class NavItemComponent implements OnChanges {
-  @Input() selectedNodes: NavigationNode[];
+  @Input() isWide = false;
-  @Input() node: NavigationNode;
   @Input() level = 1;
+  @Input() node: NavigationNode;
+  @Input() selectedNodes: NavigationNode[];
 
   isExpanded = false;
   isSelected = false;
   classes: {[index: string]: boolean };
 
   ngOnChanges(changes: SimpleChanges) {
-    if (changes['selectedNodes'] || changes['node']) {
+    if (changes['selectedNodes'] || changes['node'] || changes['isWide']) {
       if (this.selectedNodes) {
         const ix = this.selectedNodes.indexOf(this.node);
-        this.isSelected = ix !== -1;
+        this.isSelected = ix !== -1; // this node is the selected node or its ancestor
-        if (ix !== 0) { this.isExpanded = this.isSelected; }
+        this.isExpanded = this.isSelected || // expand if selected or ...
+          // preserve expanded state when display is wide; collapse in mobile.
+          (this.isWide && this.isExpanded);
       } else {
         this.isSelected = false;
       }

aio/src/app/layout/nav-menu/nav-menu.component.ts

@@ -4,11 +4,12 @@ import { CurrentNode, NavigationNode } from 'app/navigation/navigation.service';
 @Component({
   selector: 'aio-nav-menu',
   template: `
-  <aio-nav-item *ngFor="let node of filteredNodes" [node]="node" [selectedNodes]="currentNode?.nodes">
+  <aio-nav-item *ngFor="let node of filteredNodes" [node]="node" [selectedNodes]="currentNode?.nodes" [isWide]="isWide">
   </aio-nav-item>`
 })
 export class NavMenuComponent {
   @Input() currentNode: CurrentNode;
-  @Input() nodes: NavigationNode[] ;
+  @Input() isWide = false;
+  @Input() nodes: NavigationNode[];
   get filteredNodes() { return this.nodes ? this.nodes.filter(n => !n.hidden) : []; }
 }