From 72ea569d68220e1c41abecfc8a192a92c060dc81 Mon Sep 17 00:00:00 2001
From: mkolesnikov <mishkolesnikov@gmail.com>
Date: Wed, 21 Mar 2018 16:41:59 +0300
Subject: [PATCH 1/2] fix(menu): avoid DOM elements creation for hidden menu
 items (fix#270)

---
 .../components/menu/menu-item.component.html  | 57 ++++++++++---------
 .../theme/components/menu/menu.component.ts   | 47 ++++++++-------
 2 files changed, 55 insertions(+), 49 deletions(-)

diff --git a/src/framework/theme/components/menu/menu-item.component.html b/src/framework/theme/components/menu/menu-item.component.html
index c122bff4a1..abbd6c5120 100644
--- a/src/framework/theme/components/menu/menu-item.component.html
+++ b/src/framework/theme/components/menu/menu-item.component.html
@@ -1,28 +1,28 @@
-<span *ngIf="menuItem.group && !menuItem.hidden">
+<span *ngIf="menuItem.group">
   <i class="menu-icon {{ menuItem.icon }}" *ngIf="menuItem.icon"></i>
   {{ menuItem.title }}
 </span>
-<a *ngIf="menuItem.link && !menuItem.url && !menuItem.children && !menuItem.group && !menuItem.hidden"
-        [routerLink]="menuItem.link"
-        [fragment]="menuItem.fragment"
-        [attr.target]="menuItem.target"
-        [attr.title]="menuItem.title"
-        [class.active]="menuItem.selected"
-        (mouseenter)="onHoverItem(menuItem)">
+<a *ngIf="menuItem.link && !menuItem.url && !menuItem.children && !menuItem.group"
+   [routerLink]="menuItem.link"
+   [fragment]="menuItem.fragment"
+   [attr.target]="menuItem.target"
+   [attr.title]="menuItem.title"
+   [class.active]="menuItem.selected"
+   (mouseenter)="onHoverItem(menuItem)">
   <i class="menu-icon {{ menuItem.icon }}" *ngIf="menuItem.icon"></i>
   <span class="menu-title">{{ menuItem.title }}</span>
 </a>
-<a *ngIf="menuItem.url && !menuItem.children && !menuItem.link && !menuItem.group && !menuItem.hidden"
-        [attr.href]="menuItem.url"
-        [attr.target]="menuItem.target"
-        [attr.title]="menuItem.title"
-        [class.active]="menuItem.selected"
-        (mouseenter)="onHoverItem(menuItem)"
-        (click)="onSelectItem(menuItem)">
+<a *ngIf="menuItem.url && !menuItem.children && !menuItem.link && !menuItem.group"
+   [attr.href]="menuItem.url"
+   [attr.target]="menuItem.target"
+   [attr.title]="menuItem.title"
+   [class.active]="menuItem.selected"
+   (mouseenter)="onHoverItem(menuItem)"
+   (click)="onSelectItem(menuItem)">
   <i class="menu-icon {{ menuItem.icon }}" *ngIf="menuItem.icon"></i>
   <span class="menu-title">{{ menuItem.title }}</span>
 </a>
-<a *ngIf="!menuItem.children && !menuItem.link && !menuItem.url && !menuItem.group && !menuItem.hidden"
+<a *ngIf="!menuItem.children && !menuItem.link && !menuItem.url && !menuItem.group"
    [attr.target]="menuItem.target"
    [attr.title]="menuItem.title"
    [class.active]="menuItem.selected"
@@ -31,7 +31,7 @@
   <i class="menu-icon {{ menuItem.icon }}" *ngIf="menuItem.icon"></i>
   <span class="menu-title">{{ menuItem.title }}</span>
 </a>
-<a *ngIf="menuItem.children && !menuItem.hidden"
+<a *ngIf="menuItem.children"
    (click)="$event.preventDefault(); onToggleSubMenu(menuItem);"
    [attr.target]="menuItem.target"
    [attr.title]="menuItem.title"
@@ -41,19 +41,22 @@
   <i class="menu-icon {{ menuItem.icon }}" *ngIf="menuItem.icon"></i>
   <span class="menu-title">{{ menuItem.title }}</span>
   <i class="ion chevron" [class.ion-chevron-left]="!menuItem.expanded"
-                 [class.ion-chevron-down]="menuItem.expanded"></i>
+     [class.ion-chevron-down]="menuItem.expanded"></i>
 </a>
-<ul *ngIf="menuItem.children && !menuItem.hidden"
+<ul *ngIf="menuItem.children"
     [class.collapsed]="!(menuItem.children && menuItem.expanded)"
     [class.expanded]="menuItem.expanded"
     class="menu-items"
     [style.max-height.px]="maxHeight">
-  <li nbMenuItem *ngFor="let item of menuItem.children"
-                  [menuItem]="item"
-                  [class.menu-group]="item.group"
-                  (hoverItem)="onHoverItem($event)"
-                  (toggleSubMenu)="onToggleSubMenu($event)"
-                  (selectItem)="onSelectItem($event)"
-                  (itemClick)="onItemClick($event)"
-                  class="menu-item"></li>
+  <ng-container *ngFor="let item of menuItem.children">
+    <li nbMenuItem *ngIf="!item.hidden"
+        [menuItem]="item"
+        [class.menu-group]="item.group"
+        (hoverItem)="onHoverItem($event)"
+        (toggleSubMenu)="onToggleSubMenu($event)"
+        (selectItem)="onSelectItem($event)"
+        (itemClick)="onItemClick($event)"
+        class="menu-item">
+    </li>
+  </ng-container>
 </ul>
diff --git a/src/framework/theme/components/menu/menu.component.ts b/src/framework/theme/components/menu/menu.component.ts
index 1892d22e58..c573532c73 100644
--- a/src/framework/theme/components/menu/menu.component.ts
+++ b/src/framework/theme/components/menu/menu.component.ts
@@ -5,20 +5,20 @@
  */
 
 import {
-    Component,
-    Input,
-    Output,
-    EventEmitter,
-    OnInit,
-    OnDestroy,
-    HostBinding,
-    ViewChildren,
-    QueryList,
-    ElementRef,
-    AfterViewInit,
-    PLATFORM_ID,
-    Inject,
-  } from '@angular/core';
+  Component,
+  Input,
+  Output,
+  EventEmitter,
+  OnInit,
+  OnDestroy,
+  HostBinding,
+  ViewChildren,
+  QueryList,
+  ElementRef,
+  AfterViewInit,
+  PLATFORM_ID,
+  Inject,
+} from '@angular/core';
 import { isPlatformBrowser } from '@angular/common';
 import { Router, NavigationEnd } from '@angular/router';
 import { BehaviorSubject } from 'rxjs/BehaviorSubject';
@@ -153,14 +153,17 @@ export class NbMenuItemComponent implements AfterViewInit, OnDestroy {
   styleUrls: ['./menu.component.scss'],
   template: `
     <ul class="menu-items">
-      <li nbMenuItem *ngFor="let item of items"
-                      [menuItem]="item"
-                      [class.menu-group]="item.group"
-                      (hoverItem)="onHoverItem($event)"
-                      (toggleSubMenu)="onToggleSubMenu($event)"
-                      (selectItem)="onSelectItem($event)"
-                      (itemClick)="onItemClick($event)"
-                      class="menu-item"></li>
+      <ng-container *ngFor="let item of items">
+        <li nbMenuItem *ngIf="!item.hidden"
+            [menuItem]="item"
+            [class.menu-group]="item.group"
+            (hoverItem)="onHoverItem($event)"
+            (toggleSubMenu)="onToggleSubMenu($event)"
+            (selectItem)="onSelectItem($event)"
+            (itemClick)="onItemClick($event)"
+            class="menu-item">
+        </li>
+      </ng-container>
     </ul>
   `,
 })

From e0bfe5f37deb222acec5f2fd203f90cc3df6d522 Mon Sep 17 00:00:00 2001
From: mkolesnikov <mishkolesnikov@gmail.com>
Date: Mon, 26 Mar 2018 12:25:39 +0300
Subject: [PATCH 2/2] feat(menu): add tests for hidden menu item

---
 e2e/menu.e2e-spec.ts                     | 11 +++++-
 src/app/menu-test/menu-test.component.ts | 47 +++++++++++++++++++-----
 2 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/e2e/menu.e2e-spec.ts b/e2e/menu.e2e-spec.ts
index 5a644b673b..5d2e4cc3a6 100644
--- a/e2e/menu.e2e-spec.ts
+++ b/e2e/menu.e2e-spec.ts
@@ -23,7 +23,8 @@ 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 waitTime = 20 * 1000;
 
 const sidebarMenu31 = by.css('#menu-sidebar ul li:nth-child(4) ul li:nth-child(1) > a > span');
@@ -276,4 +277,12 @@ describe('nb-menu', () => {
         expect(browser.getCurrentUrl()).toContain('#/menu/1');
       });
   });
+
+  it('hidden menu item should not be present', () => {
+    expect(element(hiddenMenuItem).isPresent()).toBe(false);
+  });
+
+  it('hidden submenu item should not be present', () => {
+    expect(element(hiddenSubmenuItem).isPresent()).toBeFalsy()
+  });
 });
diff --git a/src/app/menu-test/menu-test.component.ts b/src/app/menu-test/menu-test.component.ts
index 49a5cccd09..040aca2d4f 100644
--- a/src/app/menu-test/menu-test.component.ts
+++ b/src/app/menu-test/menu-test.component.ts
@@ -96,6 +96,11 @@ export class NbMenuItem4Component { }
             <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-second" tag="secondMenu" [items]="menuItems1"></nb-menu>
+          </nb-card-body>
+        </nb-card>
       </nb-layout-column>
     </nb-layout>
   `,
@@ -119,6 +124,28 @@ export class NbMenuTestComponent implements OnInit, OnDestroy {
     },
   ];
 
+  menuItems1 = [
+    {
+      title: 'Menu #1',
+    },
+    {
+      title: 'Menu #2',
+      children: [
+        {
+          title: 'Menu #2.1',
+        },
+        {
+          title: 'Hidden Submenu Item',
+          hidden: true,
+        },
+      ],
+    },
+    {
+      title: 'Hidden Menu Item',
+      hidden: true,
+    },
+  ];
+
   private alive: boolean = true;
 
   constructor(private menuService: NbMenuService) { }
@@ -180,17 +207,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');
+}
 }