From 363da1f6d6d548760952952607fa21b341ca3956 Mon Sep 17 00:00:00 2001 From: Sergey Andrievskiy Date: Tue, 7 Aug 2018 12:41:18 +0300 Subject: [PATCH 01/12] refactor(menu): remove unused field --- src/framework/theme/components/menu/menu.component.ts | 2 -- src/framework/theme/components/menu/menu.service.ts | 9 +-------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/framework/theme/components/menu/menu.component.ts b/src/framework/theme/components/menu/menu.component.ts index c124a10057..a48f2138c5 100644 --- a/src/framework/theme/components/menu/menu.component.ts +++ b/src/framework/theme/components/menu/menu.component.ts @@ -36,7 +36,6 @@ function sumSubmenuHeight(item: NbMenuItem) { } @Component({ - // tslint:disable-next-line:component-selector selector: '[nbMenuItem]', templateUrl: './menu-item.component.html', }) @@ -283,7 +282,6 @@ export class NbMenuComponent implements OnInit, AfterViewInit, OnDestroy { // TODO: this probably won't work if you pass items dynamically into items input this.menuInternalService.prepareItems(this.items); - this.items.push(...this.menuInternalService.getItems()); } ngAfterViewInit() { diff --git a/src/framework/theme/components/menu/menu.service.ts b/src/framework/theme/components/menu/menu.service.ts index 6d815cce33..ba576d1627 100644 --- a/src/framework/theme/components/menu/menu.service.ts +++ b/src/framework/theme/components/menu/menu.service.ts @@ -162,15 +162,8 @@ export class NbMenuService { @Injectable() export class NbMenuInternalService { - private items: NbMenuItem[] = []; - constructor(private location: Location) { - this.items = []; - } - - getItems(): NbMenuItem[] { - return this.items; - } + constructor(private location: Location) {} prepareItems(items: NbMenuItem[]) { const defaultItem = new NbMenuItem(); From 8d16b856a5de02b60c76e47e13e9da492a7a0e98 Mon Sep 17 00:00:00 2001 From: Sergey Andrievskiy Date: Tue, 7 Aug 2018 12:46:14 +0300 Subject: [PATCH 02/12] refactor(menu): move isParent check to menu item --- .../theme/components/menu/menu.service.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/framework/theme/components/menu/menu.service.ts b/src/framework/theme/components/menu/menu.service.ts index ba576d1627..5d6c48d1e7 100644 --- a/src/framework/theme/components/menu/menu.service.ts +++ b/src/framework/theme/components/menu/menu.service.ts @@ -95,6 +95,12 @@ export class NbMenuItem { selected?: boolean; data?: any; fragment?: string; + + isParentOf(possibleChild: NbMenuItem) { + return possibleChild.parent + ? possibleChild.parent === this || this.isParentOf(possibleChild.parent) + : false; + } } // TODO: map select events to router change events @@ -228,14 +234,8 @@ export class NbMenuInternalService { }); } - private isParent(parent, child) { - return child.parent - ? child.parent === parent || this.isParent(parent, child.parent) - : false; - } - private collapseItem(item: NbMenuItem, tag: string, except?: NbMenuItem) { - if (except && (item === except || this.isParent(item, except))) { + if (except && (item === except || item.isParentOf(except))) { return; } From 62798db048e1ffc771fdef1e16bb646e8132d433 Mon Sep 17 00:00:00 2001 From: Sergey Andrievskiy Date: Tue, 7 Aug 2018 16:58:40 +0300 Subject: [PATCH 03/12] fix(menu): emit events only for items that actually changed --- .../theme/components/menu/menu.component.ts | 14 +- .../theme/components/menu/menu.service.ts | 164 ++++++++++++------ 2 files changed, 116 insertions(+), 62 deletions(-) diff --git a/src/framework/theme/components/menu/menu.component.ts b/src/framework/theme/components/menu/menu.component.ts index a48f2138c5..f7fe2d3058 100644 --- a/src/framework/theme/components/menu/menu.component.ts +++ b/src/framework/theme/components/menu/menu.component.ts @@ -266,9 +266,7 @@ export class NbMenuComponent implements OnInit, AfterViewInit, OnDestroy { takeWhile(() => this.alive), filter((data: { tag: string }) => this.compareTag(data.tag)), ) - .subscribe((data: { tag: string }) => { - this.collapseAll(); - }); + .subscribe(() => this.collapseAll()); this.router.events .pipe( @@ -276,8 +274,7 @@ export class NbMenuComponent implements OnInit, AfterViewInit, OnDestroy { filter(event => event instanceof NavigationEnd), ) .subscribe(() => { - this.menuInternalService.resetItems(this.items); - this.menuInternalService.updateSelection(this.items, this.tag, this.autoCollapseValue) + this.menuInternalService.selectFromUrl(this.items, this.tag, this.autoCollapseValue); }); // TODO: this probably won't work if you pass items dynamically into items input @@ -285,14 +282,14 @@ export class NbMenuComponent implements OnInit, AfterViewInit, OnDestroy { } ngAfterViewInit() { - setTimeout(() => this.menuInternalService.updateSelection(this.items, this.tag)); + setTimeout(() => this.menuInternalService.selectFromUrl(this.items, this.tag)); } onAddItem(data: { tag: string; items: NbMenuItem[] }) { this.items.push(...data.items); this.menuInternalService.prepareItems(this.items); - this.menuInternalService.updateSelection(this.items, this.tag, this.autoCollapseValue); + this.menuInternalService.selectFromUrl(this.items, this.tag, this.autoCollapseValue); } onHoverItem(item: NbMenuItem) { @@ -309,8 +306,7 @@ export class NbMenuComponent implements OnInit, AfterViewInit, OnDestroy { // TODO: is not fired on page reload onSelectItem(item: NbMenuItem) { - this.menuInternalService.resetItems(this.items); - this.menuInternalService.selectItem(item, this.tag); + this.menuInternalService.selectItem(item, this.items, this.autoCollapseValue, this.tag); } onItemClick(item: NbMenuItem) { diff --git a/src/framework/theme/components/menu/menu.service.ts b/src/framework/theme/components/menu/menu.service.ts index 5d6c48d1e7..329573cc5c 100644 --- a/src/framework/theme/components/menu/menu.service.ts +++ b/src/framework/theme/components/menu/menu.service.ts @@ -10,6 +10,7 @@ import { Params } from '@angular/router'; import { Observable, BehaviorSubject, ReplaySubject } from 'rxjs'; import { share } from 'rxjs/operators'; import { isUrlPathContain, isUrlPathEqual } from './url-matching-helpers'; + export interface NbMenuBag { tag: string; item: NbMenuItem } const itemClick$ = new ReplaySubject(1); @@ -96,7 +97,21 @@ export class NbMenuItem { data?: any; fragment?: string; - isParentOf(possibleChild: NbMenuItem) { + /** + * @returns item parents in top-down order + */ + getParents(): NbMenuItem[] { + const parents = []; + + let parent; + while (parent = this.parent) { + parents.unshift(parent); + } + + return parents; + } + + isParentOf(possibleChild: NbMenuItem): boolean { return possibleChild.parent ? possibleChild.parent === this || this.isParentOf(possibleChild.parent) : false; @@ -179,19 +194,44 @@ export class NbMenuInternalService { }); } - updateSelection(items: NbMenuItem[], tag: string, collapseOther: boolean = false) { - if (collapseOther) { - this.collapseAll(items, tag); + selectFromUrl(items: NbMenuItem[], tag: string, collapseOther: boolean = false) { + const selectedItem = this.findItemByUrl(items); + if (selectedItem) { + this.selectItem(selectedItem, items, collapseOther, tag); } - items.forEach(item => this.selectItemByUrl(item, tag)); } - resetItems(items: NbMenuItem[]) { - items.forEach(i => this.resetItem(i)); + selectItem(item: NbMenuItem, items: NbMenuItem[], collapseOther: boolean = false, tag: string) { + const unselectedItems = this.resetSelection(items); + const collapsedItems = collapseOther ? this.collapseItems(items) : []; + + for (const parent of item.getParents()) { + parent.selected = true; + // emit event only for items that weren't selected before ('unselectedItems' contains items that were selected) + if (!unselectedItems.includes(parent)) { + this.itemSelect(item, tag); + } + + parent.expanded = true; + // emit event only for items that weren't expanded before ('collapsedItems' contains items that were expanded) + if (!collapsedItems.includes(parent)) { + this.submenuToggle(item, tag); + } + } + + item.selected = true; + // emit event only for items that weren't selected before ('unselectedItems' contains items that were selected) + if (!unselectedItems.includes(item)) { + this.itemSelect(item, tag); + } } collapseAll(items: NbMenuItem[], tag: string, except?: NbMenuItem) { - items.forEach(i => this.collapseItem(i, tag, except)); + const collapsedItems = this.collapseItems(items, except); + + for (const item of collapsedItems) { + this.submenuToggle(item, tag); + } } onAddItem(): Observable<{ tag: string; items: NbMenuItem[] }> { @@ -226,26 +266,53 @@ export class NbMenuInternalService { itemClick$.next({tag, item}); } - private resetItem(item: NbMenuItem) { - item.selected = false; + /** + * Unselect all given items deeply. + * @param items array of items to unselect. + * @returns items which selected value was changed. + */ + private resetSelection(items: NbMenuItem[]): NbMenuItem[] { + const unselectedItems = []; + + for (const item of items) { + if (item.selected) { + unselectedItems.push(item); + } + item.selected = false; + + if (item.children) { + unselectedItems.push(...this.resetSelection(item.children)); + } + } - item.children && item.children.forEach(child => { - this.resetItem(child); - }); + return unselectedItems; } - private collapseItem(item: NbMenuItem, tag: string, except?: NbMenuItem) { - if (except && (item === except || item.isParentOf(except))) { - return; - } - - const wasExpanded = item.expanded; - item.expanded = false; - if (wasExpanded) { - this.submenuToggle(item); + /** + * Collapse all given items deeply. + * @param items array of items to collapse. + * @param except menu item which shouldn't be collapsed, also disables collapsing for parents of this item. + * @returns items which expanded value was changed. + */ + private collapseItems(items: NbMenuItem[], except?: NbMenuItem): NbMenuItem[] { + const collapsedItems = []; + + for (const item of items) { + if (except && (item === except || item.isParentOf(except))) { + continue; + } + + if (item.expanded) { + collapsedItems.push(item) + } + item.expanded = false; + + if (item.children) { + collapsedItems.push(...this.collapseItems(item.children)); + } } - item.children && item.children.forEach(child => this.collapseItem(child, tag)); + return collapsedItems; } private applyDefaults(item, defaultItem) { @@ -253,7 +320,7 @@ export class NbMenuInternalService { Object.assign(item, defaultItem, menuItem); item.children && item.children.forEach(child => { this.applyDefaults(child, defaultItem); - }) + }); } private setParent(item: NbMenuItem) { @@ -263,38 +330,29 @@ export class NbMenuInternalService { }); } - selectItem(item: NbMenuItem, tag: string) { - item.selected = true; - this.itemSelect(item, tag); - this.selectParent(item, tag); - } - - private selectParent({ parent: item }: NbMenuItem, tag: string) { - if (!item) { - return; - } - - if (!item.expanded) { - item.expanded = true; - this.submenuToggle(item, tag); - } - - item.selected = true; - this.selectParent(item, tag); - } + /** + * Find deepest item which link matches current URL path. + * @param items array of items to search in. + * @returns found item of undefined. + */ + private findItemByUrl(items: NbMenuItem[]): NbMenuItem | undefined { + let selectedItem; + + items.some(item => { + if (item.children) { + selectedItem = this.findItemByUrl(item.children); + } + if (!selectedItem && this.isSelectedInUrl(item)) { + selectedItem = item; + } + + return selectedItem; + }); - private selectItemByUrl(item: NbMenuItem, tag: string) { - const wasSelected = item.selected; - const isSelected = this.selectedInUrl(item); - if (!wasSelected && isSelected) { - this.selectItem(item, tag); - } - if (item.children) { - this.updateSelection(item.children, tag); - } + return selectedItem; } - private selectedInUrl(item: NbMenuItem): boolean { + private isSelectedInUrl(item: NbMenuItem): boolean { const exact: boolean = item.pathMatch === 'full'; return exact ? isUrlPathEqual(this.location.path(), item.link) From 5c8ffdb2435014e0726387809ab16db85789835c Mon Sep 17 00:00:00 2001 From: Sergey Andrievskiy Date: Tue, 7 Aug 2018 18:40:14 +0300 Subject: [PATCH 04/12] test(menu): use separate menu objects --- src/playground/menu/menu-test.component.ts | 68 +++++++++++++++++++--- 1 file changed, 61 insertions(+), 7 deletions(-) diff --git a/src/playground/menu/menu-test.component.ts b/src/playground/menu/menu-test.component.ts index 401dd21d12..af7da7dee3 100644 --- a/src/playground/menu/menu-test.component.ts +++ b/src/playground/menu/menu-test.component.ts @@ -85,12 +85,12 @@ export class NbMenuItem4Component { } template: ` - + - + @@ -98,7 +98,7 @@ export class NbMenuItem4Component { } - + @@ -106,7 +106,7 @@ export class NbMenuItem4Component { } - + @@ -114,7 +114,61 @@ export class NbMenuItem4Component { } `, }) export class NbMenuTestComponent implements OnInit, OnDestroy { - menuItems = [ + sidebarMenuItems = [ + { + title: 'Menu Items', + group: true, + icon: 'nb-keypad', + }, + { + title: 'Menu #1', + link: '/menu/menu-test.component/1', + icon: 'nb-keypad', + queryParams: { param: 1 }, + fragment: '#fragment', + }, + { + title: 'Menu #2', + link: '/menu/menu-test.component/2', + icon: 'nb-keypad', + }, + { + title: 'Menu #3', + icon: 'nb-keypad', + children: [ + { + title: 'Menu #3.1', + link: '/menu/menu-test.component/3/1', + }, + { + title: 'Menu #3.2', + link: '/menu/menu-test.component/3/2', + }, + { + title: 'Menu #3.3', + children: [ + { + title: 'Menu #3.3.1', + link: '/menu/menu-test.component/3/3/1', + }, + { + title: 'Menu #3.3.2', + link: '/menu/menu-test.component/3/3/2', + queryParams: { param: 2 }, + fragment: '#fragment', + home: true, + }, + { + title: '@nebular/theme', + target: '_blank', + url: 'https://github.com/akveo/ng2-admin', + }, + ], + }, + ], + }, + ]; + firstMenuItems = [ { title: 'Menu Items', group: true, @@ -133,7 +187,7 @@ export class NbMenuTestComponent implements OnInit, OnDestroy { icon: 'nb-keypad', }, ]; - menuItems1 = [ + secondMenuItems = [ { title: 'Menu items with fragments ', group: true, @@ -156,7 +210,7 @@ export class NbMenuTestComponent implements OnInit, OnDestroy { icon: 'nb-keypad', }, ]; - menuItems2 = [ + thirdMenuItems = [ { title: 'Menu #1', }, From 4e225d3872a562eb95100b96a02b77a76f5bc47d Mon Sep 17 00:00:00 2001 From: Sergey Andrievskiy Date: Tue, 7 Aug 2018 20:07:08 +0300 Subject: [PATCH 05/12] fix(menu): make menu items methods static --- .../theme/components/menu/menu.service.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/framework/theme/components/menu/menu.service.ts b/src/framework/theme/components/menu/menu.service.ts index 329573cc5c..cbb6512953 100644 --- a/src/framework/theme/components/menu/menu.service.ts +++ b/src/framework/theme/components/menu/menu.service.ts @@ -100,20 +100,21 @@ export class NbMenuItem { /** * @returns item parents in top-down order */ - getParents(): NbMenuItem[] { + static getParents(item: NbMenuItem): NbMenuItem[] { const parents = []; - let parent; - while (parent = this.parent) { + let parent = item.parent; + while (parent) { parents.unshift(parent); + parent = parent.parent; } return parents; } - isParentOf(possibleChild: NbMenuItem): boolean { + static isParent(item, possibleChild: NbMenuItem): boolean { return possibleChild.parent - ? possibleChild.parent === this || this.isParentOf(possibleChild.parent) + ? possibleChild.parent === item || this.isParent(item, possibleChild.parent) : false; } } @@ -205,17 +206,17 @@ export class NbMenuInternalService { const unselectedItems = this.resetSelection(items); const collapsedItems = collapseOther ? this.collapseItems(items) : []; - for (const parent of item.getParents()) { + for (const parent of NbMenuItem.getParents(item)) { parent.selected = true; // emit event only for items that weren't selected before ('unselectedItems' contains items that were selected) if (!unselectedItems.includes(parent)) { - this.itemSelect(item, tag); + this.itemSelect(parent, tag); } parent.expanded = true; // emit event only for items that weren't expanded before ('collapsedItems' contains items that were expanded) if (!collapsedItems.includes(parent)) { - this.submenuToggle(item, tag); + this.submenuToggle(parent, tag); } } @@ -298,7 +299,7 @@ export class NbMenuInternalService { const collapsedItems = []; for (const item of items) { - if (except && (item === except || item.isParentOf(except))) { + if (except && (item === except || NbMenuItem.isParent(item, except))) { continue; } From 837c66e870970c298e1cb7a8f03ca3f379eede12 Mon Sep 17 00:00:00 2001 From: Sergey Andrievskiy Date: Tue, 7 Aug 2018 20:08:25 +0300 Subject: [PATCH 06/12] refactor(menu): prepare items first --- src/framework/theme/components/menu/menu.component.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/framework/theme/components/menu/menu.component.ts b/src/framework/theme/components/menu/menu.component.ts index f7fe2d3058..caa9ddb341 100644 --- a/src/framework/theme/components/menu/menu.component.ts +++ b/src/framework/theme/components/menu/menu.component.ts @@ -234,6 +234,8 @@ export class NbMenuComponent implements OnInit, AfterViewInit, OnDestroy { } ngOnInit() { + this.menuInternalService.prepareItems(this.items); + this.menuInternalService .onAddItem() .pipe( @@ -276,9 +278,6 @@ export class NbMenuComponent implements OnInit, AfterViewInit, OnDestroy { .subscribe(() => { this.menuInternalService.selectFromUrl(this.items, this.tag, this.autoCollapseValue); }); - - // TODO: this probably won't work if you pass items dynamically into items input - this.menuInternalService.prepareItems(this.items); } ngAfterViewInit() { From 2b928a3534140691e78ba9bf7ac3a9444bdead0b Mon Sep 17 00:00:00 2001 From: Sergey Andrievskiy Date: Tue, 7 Aug 2018 20:09:13 +0300 Subject: [PATCH 07/12] fix(menu): use auto collapse value on initialization --- src/framework/theme/components/menu/menu.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/framework/theme/components/menu/menu.component.ts b/src/framework/theme/components/menu/menu.component.ts index caa9ddb341..e052b4f5f5 100644 --- a/src/framework/theme/components/menu/menu.component.ts +++ b/src/framework/theme/components/menu/menu.component.ts @@ -281,7 +281,7 @@ export class NbMenuComponent implements OnInit, AfterViewInit, OnDestroy { } ngAfterViewInit() { - setTimeout(() => this.menuInternalService.selectFromUrl(this.items, this.tag)); + setTimeout(() => this.menuInternalService.selectFromUrl(this.items, this.tag, this.autoCollapseValue)); } onAddItem(data: { tag: string; items: NbMenuItem[] }) { From f440ae849a1e0a400e86432649ab28dd888e985d Mon Sep 17 00:00:00 2001 From: Sergey Andrievskiy Date: Fri, 10 Aug 2018 12:49:28 +0300 Subject: [PATCH 08/12] fix(menu): toggle using angular animations --- .../components/menu/menu-item.component.html | 4 +- .../theme/components/menu/menu.component.scss | 1 - .../theme/components/menu/menu.component.ts | 76 +++++++------------ 3 files changed, 29 insertions(+), 52 deletions(-) diff --git a/src/framework/theme/components/menu/menu-item.component.html b/src/framework/theme/components/menu/menu-item.component.html index f048dad60a..f948bf06c3 100644 --- a/src/framework/theme/components/menu/menu-item.component.html +++ b/src/framework/theme/components/menu/menu-item.component.html @@ -47,8 +47,8 @@