feat(popover): conditional popover

If title and content are empty, the popover won't open Closes #2188 Closes #2217 BREAKING CHANGE: Popovers with an empty title and content are considered deisabled and won't open by default.
ng-bootstrap · Apr 12, 2018 · 9de58e6 · 9de58e6
1 parent 267c668
commit 9de58e6
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 5 deletions.
diff --git a/src/popover/popover.spec.ts b/src/popover/popover.spec.ts
@@ -147,6 +147,17 @@ describe('ngb-popover', () => {
       expect(spyService.called).toBeTruthy();
     });
 
+    it('should not open a popover if content and title are empty', () => {
+      const fixture = createTestComponent(`<div [ngbPopover]="" [popoverTitle]=""></div>`);
+      const directive = fixture.debugElement.query(By.directive(NgbPopover));
+
+      directive.triggerEventHandler('click', {});
+      fixture.detectChanges();
+      const windowEl = getWindow(fixture.nativeElement);
+
+      expect(windowEl).toBeNull();
+    });
+
     it('should not open a popover if [disablePopover] flag', () => {
       const fixture = createTestComponent(`<div [ngbPopover]="Disabled!" [disablePopover]="true"></div>`);
       const directive = fixture.debugElement.query(By.directive(NgbPopover));
@@ -158,6 +169,44 @@ describe('ngb-popover', () => {
       expect(windowEl).toBeNull();
     });
 
+    it('should close the popover if content and title become empty', () => {
+      const fixture = createTestComponent(`<div [ngbPopover]="name" [popoverTitle]="title"></div>`);
+      const directive = fixture.debugElement.query(By.directive(NgbPopover));
+
+      directive.triggerEventHandler('click', {});
+      fixture.detectChanges();
+      expect(getWindow(fixture.nativeElement)).not.toBeNull();
+
+      fixture.componentInstance.name = '';
+      fixture.componentInstance.title = '';
+      fixture.detectChanges();
+      expect(getWindow(fixture.nativeElement)).toBeNull();
+    });
+
+    it('should open the popover if content is empty but title has value', () => {
+      const fixture = createTestComponent(`<div [ngbPopover]="" popoverTitle="title"></div>`);
+      const directive = fixture.debugElement.query(By.directive(NgbPopover));
+
+      directive.triggerEventHandler('click', {});
+      fixture.detectChanges();
+      const windowEl = getWindow(fixture.nativeElement);
+
+      expect(windowEl).not.toBeNull();
+    });
+
+    it('should not close the popover if content becomes empty but title has value', () => {
+      const fixture = createTestComponent(`<div [ngbPopover]="name" popoverTitle="title"></div>`);
+      const directive = fixture.debugElement.query(By.directive(NgbPopover));
+
+      directive.triggerEventHandler('click', {});
+      fixture.detectChanges();
+      expect(getWindow(fixture.nativeElement)).not.toBeNull();
+
+      fixture.componentInstance.name = '';
+      fixture.detectChanges();
+      expect(getWindow(fixture.nativeElement)).not.toBeNull();
+    });
+
     it('should allow re-opening previously closed popovers', () => {
       const fixture = createTestComponent(`<div ngbPopover="Great tip!" popoverTitle="Title"></div>`);
       const directive = fixture.debugElement.query(By.directive(NgbPopover));

diff --git a/src/popover/popover.ts b/src/popover/popover.ts
@@ -7,14 +7,16 @@ import {
   ChangeDetectionStrategy,
   OnInit,
   OnDestroy,
+  OnChanges,
   Injector,
   Renderer2,
   ComponentRef,
   ElementRef,
   TemplateRef,
   ViewContainerRef,
   ComponentFactoryResolver,
-  NgZone
+  NgZone,
+  SimpleChanges
 } from '@angular/core';
 
 import {listenToTriggers} from '../util/triggers';
@@ -90,13 +92,13 @@ export class NgbPopoverWindow {
  * A lightweight, extensible directive for fancy popover creation.
  */
 @Directive({selector: '[ngbPopover]', exportAs: 'ngbPopover'})
-export class NgbPopover implements OnInit, OnDestroy {
+export class NgbPopover implements OnInit, OnDestroy, OnChanges {
   /**
-   * Content to be displayed as popover.
+   * Content to be displayed as popover. If title and content are empty, the popover won't open.
    */
   @Input() ngbPopover: string | TemplateRef<any>;
   /**
-   * Title of a popover.
+   * Title of a popover. If title and content are empty, the popover won't open.
    */
   @Input() popoverTitle: string;
   /**
@@ -135,6 +137,15 @@ export class NgbPopover implements OnInit, OnDestroy {
   private _windowRef: ComponentRef<NgbPopoverWindow>;
   private _unregisterListenersFn;
   private _zoneSubscription: any;
+  private _isDisabled(): boolean {
+    if (this.disablePopover) {
+      return true;
+    }
+    if (!this.ngbPopover && !this.popoverTitle) {
+      return true;
+    }
+    return false;
+  }
 
   constructor(
       private _elementRef: ElementRef, private _renderer: Renderer2, injector: Injector,
@@ -162,7 +173,7 @@ export class NgbPopover implements OnInit, OnDestroy {
    * The context is an optional value to be injected into the popover template when it is created.
    */
   open(context?: any) {
-    if (!this._windowRef && !this.disablePopover) {
+    if (!this._windowRef && !this._isDisabled()) {
       this._windowRef = this._popupService.open(this.ngbPopover, context);
       this._windowRef.instance.title = this.popoverTitle;
       this._windowRef.instance.id = this._ngbPopoverWindowId;
@@ -221,6 +232,13 @@ export class NgbPopover implements OnInit, OnDestroy {
         this.toggle.bind(this));
   }
 
+  ngOnChanges(changes: SimpleChanges) {
+    // close popover if title and content become empty, or disablePopover set to true
+    if ((changes['ngbPopover'] || changes['popoverTitle'] || changes['disablePopover']) && this._isDisabled()) {
+      this.close();
+    }
+  }
+
   ngOnDestroy() {
     this.close();
     this._unregisterListenersFn();