From fa86c79e3c2d9111e0d9dbc4b110e36b5778ed4a Mon Sep 17 00:00:00 2001 From: Ward Bell Date: Sat, 29 Apr 2017 18:08:25 -0700 Subject: [PATCH] fix(aio): header click should always toggle expand/collapse Also adds class tests for this NavItemComponent. Not (yet) testing effects on the templated HTML --- .../layout/nav-item/nav-item.component.html | 2 +- .../nav-item/nav-item.component.spec.ts | 159 ++++++++++++++++++ .../app/layout/nav-item/nav-item.component.ts | 12 +- .../nav-menu/nav-menu.component.spec.ts | 2 +- 4 files changed, 165 insertions(+), 10 deletions(-) create mode 100644 aio/src/app/layout/nav-item/nav-item.component.spec.ts diff --git a/aio/src/app/layout/nav-item/nav-item.component.html b/aio/src/app/layout/nav-item/nav-item.component.html index 3894100686cbb..5f53f58dd5c8c 100644 --- a/aio/src/app/layout/nav-item/nav-item.component.html +++ b/aio/src/app/layout/nav-item/nav-item.component.html @@ -1,6 +1,6 @@
+ class="vertical-menu-item"> {{node.title}}
diff --git a/aio/src/app/layout/nav-item/nav-item.component.spec.ts b/aio/src/app/layout/nav-item/nav-item.component.spec.ts new file mode 100644 index 0000000000000..22379c238885e --- /dev/null +++ b/aio/src/app/layout/nav-item/nav-item.component.spec.ts @@ -0,0 +1,159 @@ + +import { SimpleChange, SimpleChanges } 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 +// No dependencies. Just new it and test :) +// Let e2e tests verify how it displays. +describe('NavItemComponent (class-only)', () => { + + let component: NavItemComponent; + + let selectedNodes: NavigationNode[]; + let setClassesSpy: jasmine.Spy; + + function initialize(nd: NavigationNode) { + component.node = nd; + onChanges(); // Angular calls when initializing the component + } + + // Enough to triggers component's ngOnChange method + function onChanges() { + component.ngOnChanges({node: 'anything' }); + } + + beforeEach(() => { + + component = new NavItemComponent(); + setClassesSpy = spyOn(component, 'setClasses').and.callThrough(); + + // Selected nodes is the selected node and its header ancestors + selectedNodes = [ + { 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', () => { + it('with selected node', () => { + initialize(selectedNodes[0]); + expect(component.classes).toEqual( + // selecting the current node has no effect on expanded state, + // even if current node is a header. + { 'level-1': true, collapsed: true, expanded: false, 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} + ); + }); + }); + + describe('when becomes a non-selected node', () => { + + // this node won't be the selected node when ngOnChanges() called + beforeEach(() => component.node = { title: 'x' }); + + it('should collapse if previously expanded', () => { + component.isExpanded = true; + onChanges(); + expect(component.isExpanded).toBe(false, 'becomes collapsed'); + }); + + 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(); + }); + }); +}); diff --git a/aio/src/app/layout/nav-item/nav-item.component.ts b/aio/src/app/layout/nav-item/nav-item.component.ts index dbea4784db5f1..da393b0069d73 100644 --- a/aio/src/app/layout/nav-item/nav-item.component.ts +++ b/aio/src/app/layout/nav-item/nav-item.component.ts @@ -1,5 +1,5 @@ import { Component, Input, OnChanges, SimpleChanges } from '@angular/core'; -import { NavigationNode } from 'app/navigation/navigation.service'; +import { NavigationNode } from 'app/navigation/navigation.model'; @Component({ selector: 'aio-nav-item', @@ -16,8 +16,9 @@ export class NavItemComponent implements OnChanges { ngOnChanges(changes: SimpleChanges) { if (changes['selectedNodes'] || changes['node']) { - this.isSelected = this.selectedNodes.indexOf(this.node) !== -1; - this.isExpanded = this.isSelected; + const ix = this.selectedNodes.indexOf(this.node); + this.isSelected = ix !== -1; + if (ix !== 0) { this.isExpanded = this.isSelected; } } this.setClasses(); } @@ -31,11 +32,6 @@ export class NavItemComponent implements OnChanges { }; } - itemClicked() { - this.isExpanded = true; - this.isSelected = !!this.node; - } - headerClicked() { this.isExpanded = !this.isExpanded; this.setClasses(); diff --git a/aio/src/app/layout/nav-menu/nav-menu.component.spec.ts b/aio/src/app/layout/nav-menu/nav-menu.component.spec.ts index 7b8bfe5280b54..2daffc816ae16 100644 --- a/aio/src/app/layout/nav-menu/nav-menu.component.spec.ts +++ b/aio/src/app/layout/nav-menu/nav-menu.component.spec.ts @@ -4,7 +4,7 @@ import { NavigationNode } from 'app/navigation/navigation.service'; // Testing the component class behaviors, independent of its template // No dependencies, no life-cycle hooks. Just new it and test :) // Let e2e tests verify how it displays. -describe('NavMenuComponent', () => { +describe('NavMenuComponent (class-only)', () => { it('should filter out hidden nodes', () => { const component = new NavMenuComponent(); const nodes: NavigationNode[] =