fix(collapse): respect display input value

This change removes race conditions caused by TypeScript setters being called by Angular in non-deterministic order. Setters side effects (reading values set in other setters) are moved to ngOnChanges lifecycle hook. The consequence is that those side effects are no longer called when those inputs are set other than in the template. This is a breaking change but methods to call instead of setting those properties are already provided (show/hide) so migration path is straightforward. Setting display to 'none' no longer hides the collapse, setting collapse input to true or calling hide method is the way to go.

BREAKING CHANGE: setting display or collapse property on CollapseDirective no longer expands/collapses the collapse - use show/hide methods instead or set collapse input in template

apps/ngx-bootstrap-docs/src/app/components/+collapse/demos/inline-display/inline-display.html

@@ -1,10 +1,10 @@
-<button type="button" class="btn btn-success" (click)="collapse.display='inline-block'"
+<button type="button" class="btn btn-success" (click)="isCollapsed = false"
         aria-controls="collapseBasic">Inline-block
 </button>
-<button type="button" class="btn btn-primary" (click)="collapse.display='none'"
+<button type="button" class="btn btn-primary" (click)="isCollapsed = true"
         aria-controls="collapseBasic">None
 </button>
 <hr>
-<div id="collapseBasic" [collapse]="!isCollapsed" #collapse="bs-collapse">
+<div id="collapseBasic" [collapse]="isCollapsed" [display]="'inline-block'">
   <div class="well well-lg card card-block card-header">Some content</div>
 </div>

src/collapse/collapse.directive.ts

@@ -11,8 +11,10 @@ import {
   EventEmitter,
   HostBinding,
   Input,
+  OnChanges,
   Output,
-  Renderer2
+  Renderer2,
+  SimpleChanges
 } from '@angular/core';
 
 import {
@@ -28,7 +30,7 @@ import {
     '[class.collapse]': 'true'
   }
 })
-export class CollapseDirective implements AfterViewChecked {
+export class CollapseDirective implements OnChanges, AfterViewChecked {
   /** This event fires as soon as content collapses */
   @Output() collapsed: EventEmitter<CollapseDirective> = new EventEmitter();
   /** This event fires when collapsing is started */
@@ -50,41 +52,21 @@ export class CollapseDirective implements AfterViewChecked {
   // animation state
   @HostBinding('class.collapsing') isCollapsing = false;
 
+  /** display value used when collapse is visible */
   @Input()
-  set display(value: string) {
-    if (!this.isAnimated) {
-      this._renderer.setStyle(this._el.nativeElement, 'display', value);
-
-      return;
-    }
-
-    this._display = value;
-
-    if (value === 'none') {
-      this.hide();
-
-      return;
-    }
-
-    this.show();
-  }
+  display = 'block'
   /** turn on/off animation */
   @Input() isAnimated = false;
   /** A flag indicating visibility of content (shown or hidden) */
   @Input()
   set collapse(value: boolean) {
     this.collapseNewValue = value;
-    if (!this._player || this._isAnimationDone) {
-      this.isExpanded = value;
-      this.toggle();
-    }
   }
 
   get collapse(): boolean {
     return this.isExpanded;
   }
 
-  private _display = 'block';
   private _isAnimationDone?: boolean;
   private _player?: AnimationPlayer;
   private _stylesLoaded = false;
@@ -104,6 +86,15 @@ export class CollapseDirective implements AfterViewChecked {
     this._factoryExpandAnimation = _builder.build(expandAnimation);
   }
 
+  ngOnChanges(changes: SimpleChanges): void {
+    if ('collapse' in changes) {
+      if (!this._player || this._isAnimationDone) {
+        this.isExpanded = this.collapseNewValue;
+        this.toggle();
+      }
+    }
+  }
+
   ngAfterViewChecked(): void {
     this._stylesLoaded = true;
 
@@ -148,7 +139,7 @@ export class CollapseDirective implements AfterViewChecked {
   }
   /** allows to manually show collapsed content */
   show(): void {
-    this._renderer.setStyle(this._el.nativeElement, 'display', this._display);
+    this._renderer.setStyle(this._el.nativeElement, 'display', this.display);
 
     this.isCollapsing = true;
     this.isExpanded = true;