Skip to content

Commit

Permalink
fix(menu): apply default values for menuItems (#344)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: URL fragment no longer affects menu items selection.
Now we only find matches between path part of the URL and `link` property of menu-item.
  • Loading branch information
mishkolesnikov authored and nnixaa committed Apr 5, 2018
1 parent 0aa898b commit 674eef5
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 26 deletions.
44 changes: 42 additions & 2 deletions e2e/menu.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,19 @@ const menu333 = by.css('#menu-first ul li:nth-child(4) ul li:nth-child(3) ul li:
const newMenu = by.css('#menu-first ul li:nth-child(5) a');
const addButton = by.css('#addBtn');
const homeButton = by.css('#homeBtn');
const hiddenMenuItem = by.css('#menu-second ul li:nth-child(3)');
const hiddenSubmenuItem = by.css('#menu-second ul li:nth-child(2) ul li:nth-child(2)');
const hiddenMenuItem = by.css('#menu-third ul li:nth-child(3)');
const hiddenSubmenuItem = by.css('#menu-third ul li:nth-child(2) ul li:nth-child(2)');
const waitTime = 20 * 1000;

const sidebarMenu31 = by.css('#menu-sidebar ul li:nth-child(4) ul li:nth-child(1) > a > span');

const sidebarMenu1 = by.css('#menu-sidebar ul li:nth-child(2) a');
const sidebarMenu3 = by.css('#menu-sidebar ul li:nth-child(4) a');

const secondMenu1 = by.css('#menu-second ul li:nth-child(2) a');
const secondMenu2 = by.css('#menu-second ul li:nth-child(3) a');
const secondMenu4 = by.css('#menu-second ul li:nth-child(4) a');

describe('nb-menu', () => {

beforeEach((done) => {
Expand Down Expand Up @@ -299,4 +304,39 @@ describe('nb-menu', () => {
expect(browser.getCurrentUrl()).toContain('param=2');
})
});

it('pathMatch: "full" should be applied by default', () => {
browser.get('#menu/3/3').then(() => {
expect(hasClass(element.all(secondMenu4).first(), 'active')).toBeFalsy();
})
});

it('pathMatch: "partial" should work by url segments', () => {
const menu1Element = element.all(secondMenu1).first();
const menu2Element = element.all(secondMenu2).first();
menu1Element.click()
.then(() => {
expect(hasClass(menu1Element, 'active')).toBeTruthy();
});
menu2Element.click()
.then(() => {
expect(hasClass(menu2Element, 'active')).toBeTruthy();
expect(hasClass(menu1Element, 'active')).toBeFalsy();
})
});

it('should add fragment to url', () => {
element.all(menu1).first().click()
.then(() => {
expect(browser.getCurrentUrl()).toContain('#fragment');
})
});

it('should add fragment to url (navigate home)', () => {
element(homeButton).click()
.then(() => {
expect(browser.getCurrentUrl()).toContain('#fragment');
})
});

});
4 changes: 4 additions & 0 deletions src/app/app.routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ export const routes: Routes = [
path: '2',
component: NbMenuItem2Component,
},
{
path: '12',
component: NbMenuItem1Component,
},
{
path: '3',
component: NbMenuItem3Component,
Expand Down
56 changes: 44 additions & 12 deletions src/app/menu-test/menu-test.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,15 @@ export class NbMenuItem4Component { }
</nb-card>
<nb-card size="xxlarge">
<nb-card-body>
<nb-menu id="menu-second" tag="secondMenu" [items]="menuItems1"></nb-menu>
<nb-menu id="menu-second" tag="SecondMenu" [items]="menuItems1"></nb-menu>
<router-outlet></router-outlet>
<button class="btn btn-primary" id="addBtn" (click)="addMenuItem()">Add</button>
<button class="btn btn-primary" id="homeBtn" (click)="navigateHome()">Home</button>
</nb-card-body>
</nb-card>
<nb-card size="xxlarge">
<nb-card-body>
<nb-menu id="menu-third" tag="thirdMenu" [items]="menuItems2"></nb-menu>
</nb-card-body>
</nb-card>
</nb-layout-column>
Expand All @@ -117,15 +125,38 @@ export class NbMenuTestComponent implements OnInit, OnDestroy {
link: '/menu/1',
icon: 'nb-keypad',
queryParams: { param: 1 },
fragment: '#fragment',
},
{
title: 'Menu #2',
link: '/menu/2',
icon: 'nb-keypad',
},
];

menuItems1 = [
{
title: 'Menu items with fragments ',
group: true,
},
{
title: 'Menu #1',
link: '/menu/1',
icon: 'nb-keypad',
pathMatch: 'partial',
},
{
title: 'Menu #12 + fragment',
link: '/menu/12',
fragment: 'fragment',
icon: 'nb-keypad',
},
{
title: 'Menu #3',
link: '/menu/3',
icon: 'nb-keypad',
},
];
menuItems2 = [
{
title: 'Menu #1',
},
Expand Down Expand Up @@ -195,6 +226,7 @@ export class NbMenuTestComponent implements OnInit, OnDestroy {
title: 'Menu #3.3.2',
link: '/menu/3/3/2',
queryParams: { param: 2 },
fragment: '#fragment',
home: true,
},
{
Expand All @@ -209,17 +241,17 @@ export class NbMenuTestComponent implements OnInit, OnDestroy {
],
'firstMenu',
);
}
}

ngOnDestroy() {
this.alive = false;
}
ngOnDestroy() {
this.alive = false;
}

addMenuItem() {
this.menuService.addItems([{ title: 'New Menu Item' }], 'firstMenu');
}
addMenuItem() {
this.menuService.addItems([{ title: 'New Menu Item' }], 'firstMenu');
}

navigateHome() {
this.menuService.navigateHome('firstMenu');
}
navigateHome() {
this.menuService.navigateHome('firstMenu');
}
}
2 changes: 1 addition & 1 deletion src/framework/theme/components/menu/menu.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ export class NbMenuComponent implements OnInit, AfterViewInit, OnDestroy {

if (homeItem) {
if (homeItem.link) {
this.router.navigate([homeItem.link], { queryParams: homeItem.queryParams });
this.router.navigate([homeItem.link], { queryParams: homeItem.queryParams, fragment: homeItem.fragment });
}

if (homeItem.url) {
Expand Down
30 changes: 19 additions & 11 deletions src/framework/theme/components/menu/menu.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { BehaviorSubject } from 'rxjs/BehaviorSubject';
import { ReplaySubject } from 'rxjs/ReplaySubject';
import { share } from 'rxjs/operators/share';
import { Params } from '@angular/router';

import { isUrlPathContain, isUrlPathEqual } from './url-matching-helpers';
export interface NbMenuBag { tag: string; item: NbMenuItem }

const itemClick$ = new ReplaySubject<NbMenuBag>(1);
Expand All @@ -27,7 +27,7 @@ const submenuToggle$ = new ReplaySubject<NbMenuBag>(1);
/**
* Menu Item options
*/
export abstract class NbMenuItem {
export class NbMenuItem {
/**
* Item Title
* @type {string}
Expand Down Expand Up @@ -77,7 +77,7 @@ export abstract class NbMenuItem {
* Item is selected when partly or fully equal to the current url
* @type {string}
*/
pathMatch?: string = 'full'; // TODO: is not set if item is initialized by default, should be set anyway
pathMatch?: string = 'full';
/**
* Where this is a home item
* @type {boolean}
Expand Down Expand Up @@ -166,7 +166,11 @@ export class NbMenuInternalService {
}

prepareItems(items: NbMenuItem[]) {
items.forEach(i => this.setParent(i));
const defaultItem = new NbMenuItem();
items.forEach(i => {
this.applyDefaults(i, defaultItem);
this.setParent(i);
});
}

updateSelection(items: NbMenuItem[], tag: string, collapseOther: boolean = false) {
Expand Down Expand Up @@ -240,6 +244,14 @@ export class NbMenuInternalService {
item.children && item.children.forEach(child => this.collapseItem(child, tag));
}

private applyDefaults(item, defaultItem) {
const menuItem = {...item};
Object.assign(item, defaultItem, menuItem);
item.children && item.children.forEach(child => {
this.applyDefaults(child, defaultItem);
})
}

private setParent(item: NbMenuItem) {
item.children && item.children.forEach(child => {
child.parent = item;
Expand Down Expand Up @@ -280,12 +292,8 @@ export class NbMenuInternalService {

private selectedInUrl(item: NbMenuItem): boolean {
const exact: boolean = item.pathMatch === 'full';
const location: string = this.location.path();

return (
(exact && location === item.link) ||
(!exact && location.includes(item.link)) ||
(exact && item.fragment && location.substr(location.indexOf('#') + 1).includes(item.fragment))
);
return exact
? isUrlPathEqual(this.location.path(), item.link)
: isUrlPathContain(this.location.path(), item.link);
}
}
39 changes: 39 additions & 0 deletions src/framework/theme/components/menu/menu.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* @license
* Copyright Akveo. All Rights Reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*/

import { isUrlPathContain, isUrlPathEqual } from './url-matching-helpers';


describe('menu URL helpers', () => {
it('isUrlPathContain should work by url segments', () => {
expect(isUrlPathContain('/a/ba', '/a/b')).toBeFalsy();
expect(isUrlPathContain('/a/b/c', '/a/b')).toBeTruthy();
});

it('isUrlPathContain should work for url with fragments', () => {
expect(isUrlPathContain('/a/b#fragment', '/a/b')).toBeTruthy();
});

it('isUrlPathContain should work for url with query strings', () => {
expect(isUrlPathContain('/a/b?a=1;b=2&c=3', '/a/b')).toBeTruthy();
});

it('isUrlPathEqual should work for identical paths', () => {
expect(isUrlPathEqual('/a/b/c', '/a/b')).toBeFalsy();
expect(isUrlPathEqual('/a/b/c', '/a/b/c')).toBeTruthy();
});

it('isUrlPathEqual should work for url with fragments', () => {
expect(isUrlPathEqual('/a/b/c#fragment', '/a/b/c')).toBeTruthy();
});

it('isUrlPathEqual should work for url with query strings', () => {
expect(isUrlPathEqual('/a/b/c?a=1;b=2&c=3', '/a/b/c')).toBeTruthy();
});

})


21 changes: 21 additions & 0 deletions src/framework/theme/components/menu/url-matching-helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* @license
* Copyright Akveo. All Rights Reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*/

export function isUrlPathEqual(path, link) {
const locationPath = getPathPartOfUrl(path);
return link === locationPath;
}

export function isUrlPathContain(path, link) {
const locationPath = getPathPartOfUrl(path);
const endOfUrlSegmentRegExp = /\/|^$/;
return locationPath.startsWith(link) &&
locationPath.slice(link.length).charAt(0).search(endOfUrlSegmentRegExp) !== -1;
}

function getPathPartOfUrl(url): string {
return url.match(/.*?(?=[?#]|$)/)[0];
}

0 comments on commit 674eef5

Please sign in to comment.