fix(tabs): fixed tabs duplication issue (#1941)

fixed #1629
* fixed duplicate tab issue

* removed automatic reselection of tabs if tab is destroyed

* added tests for remove tab

* added viewvchild to be able to call method

* added detect changes call to fix unittests

* fix(tabs): remove tab-content node if tab is destroyed

* fix(tabs): fix ngIf error and change docs

demo/src/app/components/+tabs/demos/basic/basic.html

@@ -1,20 +1,8 @@
 <div>
-  <p>Access to static tab from component</p>
-  <p>
-    <button type="button" class="btn btn-primary btn-sm" (click)="selectTab(1)">Select second tab</button>
-    <button type="button" class="btn btn-primary btn-sm" (click)="selectTab(2)">Select third tab</button>
-  </p>
-  <p>
-    <button type="button" class="btn btn-primary btn-sm" (click)="disableEnable()">
-      Enable / Disable third tab
-    </button>
-  </p>
-  <hr/>  
-  <tabset #staticTabs>
+  <tabset>
     <tab heading="Static title">Static content</tab>
     <tab heading="Static Title 1">Static content 1</tab>
     <tab heading="Static Title 2">Static content 2</tab>
-    <tab heading="Static Title 3" removable="true">Static content 3</tab>
     <tab (select)="alertMe()">
       <template tabHeading>
         <i class="glyphicon glyphicon-bell"></i> Alert!

demo/src/app/components/+tabs/demos/basic/basic.ts

@@ -7,19 +7,9 @@ import { TabsetComponent } from 'ngx-bootstrap';
 })
 export class DemoTabsBasicComponent {
 
-  @ViewChild('staticTabs') staticTabs: TabsetComponent;
-
   public alertMe(): void {
     setTimeout(function (): void {
       alert('You\'ve selected the alert tab!');
     });
   }
-
-  selectTab(tab_id: number) {
-      this.staticTabs.tabs[tab_id].active = true;
-  }
-
-  disableEnable() {
-    this.staticTabs.tabs[2].disabled = ! this.staticTabs.tabs[2].disabled
-  }
 }

demo/src/app/components/+tabs/demos/dynamic/dynamic.html

@@ -1,14 +1,11 @@
 <div (click)="$event.preventDefault()">
-  <p>Select a tab by setting active binding to true:</p>
-  <p>
-    <button type="button" class="btn btn-primary btn-sm" (click)="tabs[0].active = true">Select second tab</button>
-    <button type="button" class="btn btn-primary btn-sm" (click)="tabs[1].active = true">Select third tab</button>
-  </p>
-  <p>
-    <button type="button" class="btn btn-primary btn-sm" (click)="tabs[1].disabled = ! tabs[1].disabled">
-      Enable / Disable third tab
+    <p>Add / remove new tabs by manipulating tabs array:</p>
+    <button type="button" class="btn btn-primary btn-sm" (click)="addNewTab()">
+      Add new tab
+    </button>
+    <button type="button" class="btn btn-primary btn-sm" (click)="tabs = []"  *ngIf="tabs.length">
+      Remove all tabs
     </button>
-  </p>
   <hr/>
   <tabset>
     <tab heading="Static title">Static content</tab>

demo/src/app/components/+tabs/demos/dynamic/dynamic.ts

@@ -12,11 +12,18 @@ export class DemoTabsDynamicComponent {
     {title: 'Dynamic Title 3', content: 'Dynamic content 3', removable: true}
   ];
 
-  public setActiveTab(index: number): void {
-    this.tabs[index].active = true;
+  public addNewTab(): void {
+    const newTabIndex = this.tabs.length + 1;
+    this.tabs.push({
+      title: `Dynamic Title ${newTabIndex}`,
+      content: `Dynamic content ${newTabIndex}`,
+      disabled:false,
+      removable:true
+    });
   }
 
-  public removeTabHandler(/*tab:any*/): void {
+  public removeTabHandler(tab:any): void {
+    this.tabs.splice(this.tabs.indexOf(tab), 1);
     console.log('Remove Tab handler');
   }
 }

demo/src/app/components/+tabs/demos/index.ts

@@ -5,9 +5,10 @@ import { DemoTabsPillsComponent } from './pills/pills';
 import { DemoTabsVerticalPillsComponent } from './vertical-pills/vertical-pills';
 import { DemoTabsJustifiedComponent } from './justified/justified';
 import { DemoTabsConfigComponent } from './config/config';
+import { DemoTabsManualComponent } from './manual/manual';
 
 export const DEMO_COMPONENTS = [
-  DemoTabsBasicComponent, DemoTabsDynamicComponent, DemoTabsStylingComponent, DemoTabsPillsComponent,
+  DemoTabsBasicComponent, DemoTabsManualComponent, DemoTabsDynamicComponent, DemoTabsStylingComponent, DemoTabsPillsComponent,
   DemoTabsVerticalPillsComponent, DemoTabsJustifiedComponent, DemoTabsConfigComponent
 ];
 
@@ -16,6 +17,10 @@ export const DEMOS = {
     component: require('!!raw-loader?lang=typescript!./basic/basic'),
     html: require('!!raw-loader?lang=markup!./basic/basic.html')
   },
+  manual: {
+    component: require('!!raw-loader?lang=typescript!./manual/manual'),
+    html: require('!!raw-loader?lang=markup!./manual/manual.html')
+  },
   dynamic: {
     component: require('!!raw-loader?lang=typescript!./dynamic/dynamic'),
     html: require('!!raw-loader?lang=markup!./dynamic/dynamic.html')

demo/src/app/components/+tabs/demos/manual/manual.html

@@ -0,0 +1,20 @@
+<div>
+  <p>Access to static tab from component</p>
+  <p>
+    <button type="button" class="btn btn-primary btn-sm" (click)="selectTab(1)">Select second tab</button>
+    <button type="button" class="btn btn-primary btn-sm" (click)="selectTab(2)">Select third tab</button>
+  </p>
+  <p>
+    <button type="button" class="btn btn-primary btn-sm" (click)="disableEnable()">
+      Enable / Disable third tab
+    </button>
+  </p>
+  <hr/>
+  <tabset #staticTabs>
+    <tab heading="Static title">Static content</tab>
+    <tab heading="Static Title 1">Static content 1</tab>
+    <tab heading="Static Title 2">Static content 2</tab>
+    <tab heading="Static Title 3" removable="true">Static content 3</tab>
+  </tabset>
+</div>
+

demo/src/app/components/+tabs/demos/manual/manual.ts

@@ -0,0 +1,19 @@
+import { Component, ViewChild } from '@angular/core';
+import { TabsetComponent } from 'ngx-bootstrap';
+
+@Component({
+  selector: 'demo-tabs-manual',
+  templateUrl: './manual.html'
+})
+export class DemoTabsManualComponent {
+
+  @ViewChild('staticTabs') staticTabs: TabsetComponent;
+
+  selectTab(tab_id: number) {
+    this.staticTabs.tabs[tab_id].active = true;
+  }
+
+  disableEnable() {
+    this.staticTabs.tabs[2].disabled = ! this.staticTabs.tabs[2].disabled
+  }
+}

demo/src/app/components/+tabs/tabs-section.component.ts

@@ -15,6 +15,7 @@ let titleDoc = require('html-loader!markdown-loader!./docs/title.md');
     <li><a routerLink="." fragment="examples">Examples</a>
       <ul>
         <li><a routerLink="." fragment="static">Static tabs</a></li>
+        <li><a routerLink="." fragment="manual">Manual selection</a></li>
         <li><a routerLink="." fragment="dynamic">Dynamic tabs</a></li>
         <li><a routerLink="." fragment="pills">Pills</a></li>
         <li><a routerLink="." fragment="vertical-pills">Vertical Pills</a></li>
@@ -43,7 +44,12 @@ let titleDoc = require('html-loader!markdown-loader!./docs/title.md');
   <ng-sample-box [ts]="demos.basic.component" [html]="demos.basic.html">
       <demo-tabs-basic></demo-tabs-basic>
   </ng-sample-box>
-       
+
+  <h2 routerLink="." fragment="manual" id="manual">Manual selection</h2>
+  <ng-sample-box [ts]="demos.manual.component" [html]="demos.manual.html">
+    <demo-tabs-manual></demo-tabs-manual>
+  </ng-sample-box>
+  
   <h2 routerLink="." fragment="dynamic" id="dynamic">Dynamic tabs</h2>    
   <ng-sample-box [ts]="demos.dynamic.component" [html]="demos.dynamic.html">
       <demo-tabs-dynamic></demo-tabs-dynamic>

demo/src/ng-api-doc.ts

@@ -1669,7 +1669,7 @@ export const ngdoc: any = {
       },
       {
         "name": "removed",
-        "description": "<p>fired before tab will be removed </p>\n"
+        "description": "<p>fired before tab will be removed, $event:Tab equals to instance of removed tab. <br> It's <strong>strongly recommended</strong> to remove a tab object from component when using dynamic tabs, otherwise removed tab can be displayed again after change detection</p>\n"
       },
       {
         "name": "select",

src/spec/tabset.component.spec.ts

@@ -1,11 +1,12 @@
-import { Component } from '@angular/core';
+import { Component, ViewChild } from '@angular/core';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { TabsetConfig } from '../tabs/tabset.config';
 
 import { TabsModule } from '../tabs/tabs.module';
+import { TabsetComponent } from '../tabs/tabset.component';
 
 const html = `
-  <tabset [justified]="isJustified"
+  <tabset #tabset [justified]="isJustified"
           [vertical]="isVertical">
     <tab heading="tab0">tab0 content</tab>
     <tab *ngFor="let tab of tabs"
@@ -35,7 +36,6 @@ function getTabContent(nativeEl: HTMLElement): NodeListOf<Element> {
 function expectActiveTabs(nativeEl: HTMLElement, active: boolean[]): void {
   const tabItems = getTabItems(nativeEl);
   const tabContent = getTabContent(nativeEl);
-
   expect(tabItems.length).toBe(active.length);
   expect(tabContent.length).toBe(active.length);
 
@@ -126,6 +126,20 @@ describe('Component: Tabs', () => {
     expect(tabTitlesAfter.length).toEqual(3);
   });
 
+  it('should select another tab if the active tab is removed', () => {
+    context.tabset.tabs[0].active = true;
+    context.tabset.removeTab(context.tabset.tabs[0]);
+    fixture.detectChanges();
+    expectActiveTabs(element, [true, false, false]);
+  });
+
+  it('should not select another tab if the active tab is removed and reselect is set to false', () => {
+    context.tabset.tabs[0].active = true;
+    context.tabset.removeTab(context.tabset.tabs[0], {reselect: false, emit: false});
+    fixture.detectChanges();
+    expectActiveTabs(element, [false, false, false]);
+  });
+
   it('should set tab as active on click and disable another active', () => {
     const tabTitles = getTabTitles(element);
 
@@ -197,7 +211,7 @@ describe('Component: Tabs', () => {
     fixture.detectChanges();
     expect(tabItems[1].classList).toContain('testCustomClass');
     expectActiveTabs(element, [false, true, false, false]);
-    
+
     context.tabs[0].customClass = 'otherCustomClass';
     fixture.detectChanges();
 
@@ -220,6 +234,7 @@ class TestTabsetComponent {
     {title: 'tab2', content: 'tab2 content', disabled: true},
     {title: 'tab3', content: 'tab3 content', removable: true}
   ];
+  @ViewChild('tabset') tabset: TabsetComponent;
 
   public constructor(config: TabsetConfig) {
     Object.assign(this, config);

src/tabs/tab.directive.ts

@@ -1,8 +1,9 @@
-import { Directive, EventEmitter, HostBinding, Input, Output, TemplateRef, OnInit } from '@angular/core';
+import {
+  Directive, EventEmitter, HostBinding, Input, Output, TemplateRef, OnInit, OnDestroy, ElementRef } from '@angular/core';
 import { TabsetComponent } from './tabset.component';
 
 @Directive({selector: 'tab, [tab]'})
-export class TabDirective implements OnInit {
+export class TabDirective implements OnInit, OnDestroy {
   /** tab header text */
   @Input() public heading: string;
   /** tab id */
@@ -20,7 +21,7 @@ export class TabDirective implements OnInit {
   public get active(): boolean {
     return this._active;
   }
-
+  
   public set active(active: boolean) {
     if (this.disabled && active || !active) {
       if (!active) {
@@ -44,7 +45,7 @@ export class TabDirective implements OnInit {
   @Output() public select: EventEmitter<TabDirective> = new EventEmitter();
   /** fired when tab became inactive, $event:Tab equals to deselected instance of Tab component */
   @Output() public deselect: EventEmitter<TabDirective> = new EventEmitter();
-  /** fired before tab will be removed */
+  /** fired before tab will be removed, $event:Tab equals to instance of removed tab */
   @Output() public removed: EventEmitter<TabDirective> = new EventEmitter();
 
   @HostBinding('class.tab-pane') public addClass: boolean = true;
@@ -53,12 +54,16 @@ export class TabDirective implements OnInit {
   public tabset: TabsetComponent;
   protected _active: boolean;
 
-  public constructor(tabset: TabsetComponent) {
+  public constructor(tabset: TabsetComponent, public elementRef: ElementRef) {
     this.tabset = tabset;
     this.tabset.addTab(this);
   }
 
   public ngOnInit(): void {
     this.removable = this.removable;
   }
+
+  public ngOnDestroy(): void {
+    this.tabset.removeTab(this, {reselect: false, emit: false});
+  }
 }

src/tabs/tabset.component.ts

@@ -1,4 +1,4 @@
-import { Component, HostBinding, Input, OnDestroy } from '@angular/core';
+import { Component, HostBinding, Input, OnDestroy } from '@angular/core';
 
 import { TabDirective } from './tab.directive';
 import { TabsetConfig } from './tabset.config';
@@ -79,19 +79,23 @@ export class TabsetComponent implements OnDestroy {
     tab.active = this.tabs.length === 1 && tab.active !== false;
   }
 
-  public removeTab(tab:TabDirective):void {
+  public removeTab(tab: TabDirective, options = {reselect: true, emit: true}):void {
     let index = this.tabs.indexOf(tab);
     if (index === -1 || this.isDestroyed) {
       return;
     }
     // Select a new tab if the tab to be removed is selected and not destroyed
-    if (tab.active && this.hasAvailableTabs(index)) {
+    if (options.reselect && tab.active && this.hasAvailableTabs(index)) {
       let newActiveIndex = this.getClosestTabIndex(index);
       this.tabs[newActiveIndex].active = true;
     }
-
-    tab.removed.emit(tab);
+    if(options.emit) {
+      tab.removed.emit(tab);
+    }
     this.tabs.splice(index, 1);
+    if(tab.elementRef.nativeElement && tab.elementRef.nativeElement.remove) {
+      tab.elementRef.nativeElement.remove();
+    }
   }
 
   protected getClosestTabIndex(index:number):number {