From 6a4ada2f1ddab9f8f6d9f2c9e5ff3eb6ad6e62f1 Mon Sep 17 00:00:00 2001
From: Marcin Majkowski <pl.marcin.m@gmail.com>
Date: Sat, 24 Apr 2021 12:56:02 +0200
Subject: [PATCH] fix(collapse): respect display input value (#6070)

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
---
 .../demos/inline-display/inline-display.html  |  6 +--
 src/collapse/collapse.directive.ts            | 42 +++++++------------
 2 files changed, 19 insertions(+), 29 deletions(-)

diff --git a/apps/ngx-bootstrap-docs/src/app/components/+collapse/demos/inline-display/inline-display.html b/apps/ngx-bootstrap-docs/src/app/components/+collapse/demos/inline-display/inline-display.html
index 2c962efe7a..e4bf6f3f49 100644
--- a/apps/ngx-bootstrap-docs/src/app/components/+collapse/demos/inline-display/inline-display.html
+++ b/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>
diff --git a/src/collapse/collapse.directive.ts b/src/collapse/collapse.directive.ts
index 00ddf7ebec..7bcb04f5aa 100644
--- a/src/collapse/collapse.directive.ts
+++ b/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,20 @@ export class CollapseDirective implements AfterViewChecked {
   // animation state
   @HostBinding('class.collapsing') isCollapsing = false;
 
-  @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 value used when collapse is visible */
+  @Input() 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 +85,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 +138,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;