fix(nav): allow having falsy values like 0 and 'false' for nav ids (#…

…3571)

Fixes #3569

demo/src/app/components/nav/overview/nav-overview.component.html

@@ -67,7 +67,9 @@ <h4>Selection / active nav</h4>
   <p>
     To select your navs programmatically, you have to give your nav items unique ids, so you could know and set
     which one is currently active either using the <code>[(activeId)]="..."</code> binding or the imperative API
-    with<code>.select(id)</code>. If you don't provide any ids, they will be generated automatically.
+    with<code>.select(id)</code>. If you don't provide any ids, they will be generated automatically. The only
+    limitation is that you can't have the <code>''</code> (empty string) as id, because <code>ngbNavItem</code>,
+    <code>ngbNavItem=""</code> and <code>[ngbNavItem]="''"</code> are indistinguishable.
   </p>
 
   <ngbd-code [snippet]="SELECTION"></ngbd-code>

src/nav/nav.spec.ts

@@ -333,6 +333,53 @@ describe('nav', () => {
     expectContents(fixture, ['content 2']);
   });
 
+  it(`should work navs with boundary [ngbNavItem] values`, () => {
+    const fixture = createTestComponent(`
+      <ul ngbNav #n="ngbNav" [activeId]="activeId" class="nav-tabs">
+        <li [ngbNavItem]="0">
+            <a ngbNavLink>link 1</a>
+            <ng-template ngbNavContent>content 1</ng-template>
+        </li>
+        <li [ngbNavItem]="true">
+            <a ngbNavLink>link 2</a>
+            <ng-template ngbNavContent>content 2</ng-template>
+        </li>
+        <li [ngbNavItem]="false">
+            <a ngbNavLink>link 3</a>
+            <ng-template ngbNavContent>content 3</ng-template>
+        </li>
+        <li [ngbNavItem]="''">
+            <a ngbNavLink>link 4</a>
+            <ng-template ngbNavContent>content 4</ng-template>
+        </li>
+      </ul>
+      <div [ngbNavOutlet]="n"></div>
+    `);
+
+    expectLinks(fixture, [true, false, false, false]);
+    expectContents(fixture, ['content 1']);
+
+    fixture.componentInstance.activeId = true;
+    fixture.detectChanges();
+    expectLinks(fixture, [false, true, false, false]);
+    expectContents(fixture, ['content 2']);
+
+    fixture.componentInstance.activeId = false;
+    fixture.detectChanges();
+    expectLinks(fixture, [false, false, true, false]);
+    expectContents(fixture, ['content 3']);
+
+    fixture.componentInstance.activeId = 0;
+    fixture.detectChanges();
+    expectLinks(fixture, [true, false, false, false]);
+    expectContents(fixture, ['content 1']);
+
+    fixture.componentInstance.activeId = '';
+    fixture.detectChanges();
+    expectLinks(fixture, [false, false, false, false]);
+    expectContents(fixture, []);
+  });
+
   it(`should allow overriding ids used in DOM with [domId]`, () => {
     const fixture = createTestComponent(`
       <ul ngbNav #n="ngbNav" class="nav-tabs">

src/nav/nav.ts

@@ -18,6 +18,8 @@ import {
 import {isDefined} from '../util/util';
 import {NgbNavConfig} from './nav-config';
 
+const isValidNavId = (id: any) => isDefined(id) && id !== '';
+
 let navCounter = 0;
 
 /**
@@ -79,6 +81,9 @@ export class NgbNavItem implements AfterContentChecked, OnInit {
   /**
    * The id used as a model for active nav.
    * It can be anything, but must be unique inside one `ngbNav`.
+   *
+   * The only limitation is that it is not possible to have the `''` (empty string) as id,
+   * because ` ngbNavItem `, `ngbNavItem=''` and `[ngbNavItem]="''"` are indistinguishable
    */
   @Input('ngbNavItem') _id: any;
 
@@ -107,7 +112,7 @@ export class NgbNavItem implements AfterContentChecked, OnInit {
 
   get active() { return this._nav.activeId === this.id; }
 
-  get id() { return this._id || this.domId; }
+  get id() { return isValidNavId(this._id) ? this._id : this.domId; }
 
   get panelDomId() { return `${this.domId}-panel`; }
 
@@ -200,7 +205,7 @@ export class NgbNav implements AfterContentInit {
   ngAfterContentInit() {
     if (!isDefined(this.activeId)) {
       const nextId = this.items.first ? this.items.first.id : null;
-      if (nextId) {
+      if (isValidNavId(nextId)) {
         this._updateActiveId(nextId, false);
         this._cd.detectChanges();
       }