fix(aio): ensure all views can indicate the active node (#17194)

When more than one node matches a url, the last
node defined in the navigation.json file won. This
meant that, for instance, items in both the
TopBarNarrow and the Footer views would not
indicate that they were active.

Now, each url is associated with a map of current
nodes keyed off their view.

Closes #17022
This commit is contained in:
Pete Bacon Darwin 2017-06-06 07:36:22 +01:00 committed by Igor Minar
parent 42176a7ac4
commit 784347f61f
7 changed files with 99 additions and 73 deletions

View File

@ -19,8 +19,8 @@
<md-sidenav-container class="sidenav-container" [class.starting]="isStarting" 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]="currentNode"></aio-nav-menu>
<aio-nav-menu [nodes]="sideNavNodes" [currentNode]="currentNode" ></aio-nav-menu>
<aio-nav-menu *ngIf="!isSideBySide" [nodes]="topMenuNarrowNodes" [currentNode]="currentNodes?.TopBarNarrow"></aio-nav-menu>
<aio-nav-menu [nodes]="sideNavNodes" [currentNode]="currentNodes?.SideNav" ></aio-nav-menu>
<div class="doc-version" title="Angular docs version {{currentDocVersion?.title}}">
<select (change)="onDocVersionChange($event.target.selectedIndex)">

View File

@ -2,7 +2,7 @@ import { Component, ElementRef, HostBinding, HostListener, OnInit,
QueryList, ViewChild, ViewChildren } from '@angular/core';
import { MdSidenav } from '@angular/material';
import { CurrentNode, NavigationService, NavigationViews, NavigationNode, VersionInfo } from 'app/navigation/navigation.service';
import { CurrentNodes, NavigationService, NavigationViews, NavigationNode, VersionInfo } from 'app/navigation/navigation.service';
import { DocumentService, DocumentContents } from 'app/documents/document.service';
import { DocViewerComponent } from 'app/layout/doc-viewer/doc-viewer.component';
import { LocationService } from 'app/shared/location.service';
@ -25,7 +25,7 @@ export class AppComponent implements OnInit {
currentDocument: DocumentContents;
currentDocVersion: NavigationNode;
currentNode: CurrentNode;
currentNodes: CurrentNodes;
currentPath: string;
docVersions: NavigationNode[];
dtOn = false;
@ -59,7 +59,6 @@ export class AppComponent implements OnInit {
isSideBySide = false;
private isFetchingTimeout: any;
private isSideNavDoc = false;
private previousNavView: string;
private sideBySideWidth = 992;
sideNavNodes: NavigationNode[];
@ -139,17 +138,17 @@ export class AppComponent implements OnInit {
}
});
this.navigationService.currentNode.subscribe(currentNode => {
this.currentNode = currentNode;
this.navigationService.currentNodes.subscribe(currentNodes => {
this.currentNodes = currentNodes;
// Preserve current sidenav open state by default
let openSideNav = this.sidenav.opened;
const isSideNavDoc = !!currentNodes[sideNavView];
if (this.previousNavView !== currentNode.view) {
this.previousNavView = currentNode.view;
if (this.isSideNavDoc !== isSideNavDoc) {
// View type changed. Is it now a sidenav view (e.g, guide or tutorial)?
// Open if changed to a sidenav doc; close if changed to a marketing doc.
openSideNav = this.isSideNavDoc = currentNode.view === sideNavView;
openSideNav = this.isSideNavDoc = isSideNavDoc;
}
// May be open or closed when wide; always closed when narrow
this.sideNavToggle(this.isSideBySide ? openSideNav : false);
@ -253,9 +252,9 @@ export class AppComponent implements OnInit {
const sideNavOpen = `sidenav-${this.sidenav.opened ? 'open' : 'closed'}`;
const pageClass = `page-${this.pageId}`;
const folderClass = `folder-${this.folderId}`;
const viewClass = `view-${this.currentNode && this.currentNode.view}`;
const viewClasses = Object.keys(this.currentNodes || {}).map(view => `view-${view}`).join(' ');
this.hostClasses = `${sideNavOpen} ${pageClass} ${folderClass} ${viewClass}`;
this.hostClasses = `${sideNavOpen} ${pageClass} ${folderClass} ${viewClasses}`;
}
// Dynamically change height of table of contents container

View File

@ -16,9 +16,13 @@ export class NavItemComponent implements OnChanges {
ngOnChanges(changes: SimpleChanges) {
if (changes['selectedNodes'] || changes['node']) {
const ix = this.selectedNodes.indexOf(this.node);
this.isSelected = ix !== -1;
if (ix !== 0) { this.isExpanded = this.isSelected; }
if (this.selectedNodes) {
const ix = this.selectedNodes.indexOf(this.node);
this.isSelected = ix !== -1;
if (ix !== 0) { this.isExpanded = this.isSelected; }
} else {
this.isSelected = false;
}
}
this.setClasses();
}

View File

@ -4,7 +4,7 @@ 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">
</aio-nav-item>`
})
export class NavMenuComponent {

View File

@ -19,7 +19,7 @@ export interface NavigationViews {
/**
* Navigation information about a node at specific URL
* url: the current URL
* view: 'SideNav' | 'TopBar' | 'Footer'
* view: 'SideNav' | 'TopBar' | 'Footer' | etc
* nodes: the current node and its ancestor nodes within that view
*/
export interface CurrentNode {
@ -28,6 +28,15 @@ export interface CurrentNode {
nodes: NavigationNode[];
}
/**
* A map of current nodes by view.
* This is needed because some urls map to nodes in more than one view.
* If a view does not contain a node that matches the current url then the value will be undefined.
*/
export interface CurrentNodes {
[view: string]: CurrentNode;
}
export interface VersionInfo {
raw: string;
major: number;

View File

@ -1,7 +1,7 @@
import { ReflectiveInjector } from '@angular/core';
import { Http, ConnectionBackend, RequestOptions, BaseRequestOptions, Response, ResponseOptions } from '@angular/http';
import { MockBackend } from '@angular/http/testing';
import { CurrentNode, NavigationService, NavigationViews, NavigationNode, VersionInfo } from 'app/navigation/navigation.service';
import { CurrentNodes, NavigationService, NavigationViews, NavigationNode, VersionInfo } from 'app/navigation/navigation.service';
import { LocationService } from 'app/shared/location.service';
import { MockLocationService } from 'testing/location.service';
import { Logger } from 'app/shared/logger.service';
@ -113,7 +113,7 @@ describe('NavigationService', () => {
});
describe('currentNode', () => {
let currentNode: CurrentNode;
let currentNodes: CurrentNodes;
let locationService: MockLocationService;
const topBarNodes: NavigationNode[] = [
@ -138,80 +138,92 @@ describe('NavigationService', () => {
beforeEach(() => {
locationService = injector.get(LocationService);
navService.currentNode.subscribe(selected => currentNode = selected);
navService.currentNodes.subscribe(selected => currentNodes = selected);
backend.connectionsArray[0].mockRespond(createResponse(navJson));
});
it('should list the side navigation node that matches the current location, and all its ancestors', () => {
locationService.go('b');
expect(currentNode).toEqual({
url: 'b',
view: 'SideNav',
nodes: [
sideNavNodes[0].children[0],
sideNavNodes[0]
]
expect(currentNodes).toEqual({
SideNav: {
url: 'b',
view: 'SideNav',
nodes: [
sideNavNodes[0].children[0],
sideNavNodes[0]
]
}
});
locationService.go('d');
expect(currentNode).toEqual({
url: 'd',
view: 'SideNav',
nodes: [
sideNavNodes[0].children[0].children[1],
sideNavNodes[0].children[0],
sideNavNodes[0]
]
expect(currentNodes).toEqual({
SideNav: {
url: 'd',
view: 'SideNav',
nodes: [
sideNavNodes[0].children[0].children[1],
sideNavNodes[0].children[0],
sideNavNodes[0]
]
}
});
locationService.go('f');
expect(currentNode).toEqual({
url: 'f',
view: 'SideNav',
nodes: [ sideNavNodes[1] ]
expect(currentNodes).toEqual({
SideNav: {
url: 'f',
view: 'SideNav',
nodes: [ sideNavNodes[1] ]
}
});
});
it('should be a TopBar selected node if the current location is a top menu node', () => {
locationService.go('features');
expect(currentNode).toEqual({
url: 'features',
view: 'TopBar',
nodes: [ topBarNodes[0] ]
expect(currentNodes).toEqual({
TopBar: {
url: 'features',
view: 'TopBar',
nodes: [ topBarNodes[0] ]
}
});
});
it('should be a plain object if no side navigation node matches the current location', () => {
it('should be a plain object if no navigation node matches the current location', () => {
locationService.go('g?search=moo#anchor-1');
expect(currentNode).toEqual({
url: 'g',
view: '',
nodes: []
expect(currentNodes).toEqual({
'': {
url: 'g',
view: '',
nodes: []
}
});
});
it('should ignore trailing slashes, hashes, and search params on URLs in the navmap', () => {
const cnode: CurrentNode = {
url: 'c',
view: 'SideNav',
nodes: [
sideNavNodes[0].children[0].children[0],
sideNavNodes[0].children[0],
sideNavNodes[0]
]
const cnode: CurrentNodes = {
SideNav: {
url: 'c',
view: 'SideNav',
nodes: [
sideNavNodes[0].children[0].children[0],
sideNavNodes[0].children[0],
sideNavNodes[0]
]
}
};
locationService.go('c');
expect(currentNode).toEqual(cnode, 'location: c');
expect(currentNodes).toEqual(cnode, 'location: c');
locationService.go('c#foo');
expect(currentNode).toEqual(cnode, 'location: c#foo');
expect(currentNodes).toEqual(cnode, 'location: c#foo');
locationService.go('c?foo=1');
expect(currentNode).toEqual(cnode, 'location: c?foo=1');
expect(currentNodes).toEqual(cnode, 'location: c?foo=1');
locationService.go('c#foo?bar=1&baz=2');
expect(currentNode).toEqual(cnode, 'location: c#foo?bar=1&baz=2');
expect(currentNodes).toEqual(cnode, 'location: c#foo?bar=1&baz=2');
});
});

View File

@ -13,8 +13,8 @@ import { LocationService } from 'app/shared/location.service';
import { CONTENT_URL_PREFIX } from 'app/documents/document.service';
// Import and re-export the Navigation model types
import { CurrentNode, NavigationNode, NavigationResponse, NavigationViews, VersionInfo } from './navigation.model';
export { CurrentNode, NavigationNode, NavigationResponse, NavigationViews, VersionInfo } from './navigation.model';
import { CurrentNodes, NavigationNode, NavigationResponse, NavigationViews, VersionInfo } from './navigation.model';
export { CurrentNodes, CurrentNode, NavigationNode, NavigationResponse, NavigationViews, VersionInfo } from './navigation.model';
const navigationPath = CONTENT_URL_PREFIX + 'navigation.json';
@ -35,13 +35,13 @@ export class NavigationService {
* node (if any) that matches the current URL location
* including its navigation view and its ancestor nodes in that view
*/
currentNode: Observable<CurrentNode>;
currentNodes: Observable<CurrentNodes>;
constructor(private http: Http, private location: LocationService, private logger: Logger) {
const navigationInfo = this.fetchNavigationInfo();
this.navigationViews = this.getNavigationViews(navigationInfo);
this.currentNode = this.getCurrentNode(this.navigationViews);
this.currentNodes = this.getCurrentNodes(this.navigationViews);
// The version information is packaged inside the navigation response to save us an extra request.
this.versionInfo = this.getVersionInfo(navigationInfo);
}
@ -88,23 +88,23 @@ export class NavigationService {
}
/**
* Get an observable of the current node (the one that matches the current URL)
* Get an observable of the current nodes (the ones that match the current URL)
* We use `publishReplay(1)` because otherwise subscribers will have to wait until the next
* URL change before they receive an emission.
* See above for discussion of using `connect`.
*/
private getCurrentNode(navigationViews: Observable<NavigationViews>): Observable<CurrentNode> {
const currentNode = combineLatest(
private getCurrentNodes(navigationViews: Observable<NavigationViews>): Observable<CurrentNodes> {
const currentNodes = combineLatest(
navigationViews.map(views => this.computeUrlToNavNodesMap(views)),
this.location.currentPath,
(navMap, url) => {
const urlKey = url.startsWith('api/') ? 'api' : url;
return navMap[urlKey] || { view: '', url: urlKey, nodes: [] };
return navMap[urlKey] || { '' : { view: '', url: urlKey, nodes: [] }};
})
.publishReplay(1);
currentNode.connect();
return currentNode;
currentNodes.connect();
return currentNodes;
}
/**
@ -114,7 +114,7 @@ export class NavigationService {
* @param navigation - A collection of navigation nodes that are to be mapped
*/
private computeUrlToNavNodesMap(navigation: NavigationViews) {
const navMap = new Map<string, CurrentNode>();
const navMap = new Map<string, CurrentNodes>();
Object.keys(navigation)
.forEach(view => navigation[view]
.forEach(node => this.walkNodes(view, navMap, node)));
@ -138,7 +138,7 @@ export class NavigationService {
* patching them and computing their ancestor nodes
*/
private walkNodes(
view: string, navMap: Map<string, CurrentNode>,
view: string, navMap: Map<string, CurrentNodes>,
node: NavigationNode, ancestors: NavigationNode[] = []) {
const nodes = [node, ...ancestors];
const url = node.url;
@ -147,7 +147,9 @@ export class NavigationService {
// only map to this node if it has a url
if (url) {
// Strip off trailing slashes from nodes in the navMap - they are not relevant to matching
navMap[url.replace(/\/$/, '')] = { url, view, nodes };
const cleanedUrl = url.replace(/\/$/, '');
const navMapItem = navMap[cleanedUrl] = navMap[cleanedUrl] || {};
navMapItem[view] = { url, view, nodes };
}
if (node.children) {