fix(datepicker): update subscriptions of datepicker-inline and datera…

…ngepicker-inline Introduce updateSubscriptions method to unsubscribe from previous subscriptions and to create new subscriptions on new _datepickerRef.instance Reduce calls to setConfig by introducing a shouldSetConfig flag, thereby batching complex updates together Closes valor-software#5888 where EventEmitter does not fire after Input changes
JoschuaSchneider · Jul 8, 2021 · 6441d26 · 6441d26
1 parent cf29ddc
commit 6441d26
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 52 deletions.
diff --git a/src/datepicker/bs-datepicker-inline.component.ts b/src/datepicker/bs-datepicker-inline.component.ts
@@ -64,7 +64,7 @@ export class BsDatepickerInlineDirective implements OnInit, OnDestroy, OnChanges
    * Emits when datepicker value has been changed
    */
   @Output() bsValueChange: EventEmitter<Date> = new EventEmitter();
-  protected _subs: Subscription[] = [];
+  protected _subs?: Subscription;
   private readonly _datepicker: ComponentLoader<BsDatepickerInlineContainerComponent>;
   private _datepickerRef?: ComponentRef<BsDatepickerInlineContainerComponent>;
 
@@ -110,67 +110,80 @@ export class BsDatepickerInlineDirective implements OnInit, OnDestroy, OnChanges
 
   ngOnInit(): void {
     this.setConfig();
-
-    // if date changes from external source (model -> view)
-    this._subs.push(
-      this.bsValueChange.subscribe((value: Date) => {
-        if (this._datepickerRef) {
-          this._datepickerRef.instance.value = value;
-        }
-      })
-    );
-
-    // if date changes from picker (view -> model)
-    if (this._datepickerRef) {
-      this._subs.push(
-        this._datepickerRef.instance.valueChange.subscribe((value: Date) => {
-          this.bsValue = value;
-        })
-      );
-    }
+    this.updateSubscriptions();
   }
 
   ngOnChanges(changes: SimpleChanges): void {
     if (!this._datepickerRef || !this._datepickerRef.instance) {
       return;
     }
 
+    let shouldSetConfig = false;
+
     if (changes.minDate) {
       this._datepickerRef.instance.minDate = this.minDate;
-      this.setConfig();
+      shouldSetConfig = true;
     }
 
     if (changes.maxDate) {
       this._datepickerRef.instance.maxDate = this.maxDate;
-      this.setConfig();
+      shouldSetConfig = true;
     }
 
     if (changes.datesDisabled) {
       this._datepickerRef.instance.datesDisabled = this.datesDisabled;
-      this.setConfig();
+      shouldSetConfig = true;
     }
 
     if (changes.datesEnabled) {
       this._datepickerRef.instance.datesEnabled = this.datesEnabled;
       this._datepickerRef.instance.value = this._bsValue;
+      shouldSetConfig = true;
     }
 
     if (changes.isDisabled) {
       this._datepickerRef.instance.isDisabled = this.isDisabled;
-      this.setConfig();
+      shouldSetConfig = true;
     }
 
     if (changes.dateCustomClasses) {
       this._datepickerRef.instance.dateCustomClasses = this.dateCustomClasses;
-      this.setConfig();
+      shouldSetConfig = true;
     }
 
     if (changes.dateTooltipTexts) {
       this._datepickerRef.instance.dateTooltipTexts = this.dateTooltipTexts;
+      shouldSetConfig = true;
+    }
+
+    if (shouldSetConfig) {
       this.setConfig();
     }
   }
 
+  updateSubscriptions(): void {
+    this._subs?.unsubscribe();
+    this._subs = new Subscription();
+
+    // if date changes from external source (model -> view)
+    this._subs.add(
+      this.bsValueChange.subscribe((value: Date) => {
+        if (this._datepickerRef) {
+          this._datepickerRef.instance.value = value;
+        }
+      })
+    );
+
+    // if date changes from picker (view -> model)
+    if (this._datepickerRef) {
+      this._subs.add(
+        this._datepickerRef.instance.valueChange.subscribe((value: Date) => {
+          this.bsValue = value;
+        })
+      );
+    }
+  }
+
   /**
    * Set config for datepicker
    */
@@ -196,6 +209,8 @@ export class BsDatepickerInlineDirective implements OnInit, OnDestroy, OnChanges
       .attach(BsDatepickerInlineContainerComponent)
       .to(this._elementRef)
       .show();
+
+    this.updateSubscriptions();
   }
 
   ngOnDestroy() {

diff --git a/src/datepicker/bs-daterangepicker-inline.component.ts b/src/datepicker/bs-daterangepicker-inline.component.ts
@@ -69,7 +69,7 @@ export class BsDaterangepickerInlineDirective implements OnInit, OnDestroy, OnCh
      */
     @Output() bsValueChange: EventEmitter<Date[]> = new EventEmitter();
 
-    protected _subs: Subscription[] = [];
+    protected _subs?: Subscription;
 
     private readonly _datepicker: ComponentLoader<BsDaterangepickerInlineContainerComponent>;
     private _datepickerRef?: ComponentRef<BsDaterangepickerInlineContainerComponent>;
@@ -92,43 +92,24 @@ export class BsDaterangepickerInlineDirective implements OnInit, OnDestroy, OnCh
 
     ngOnInit(): void {
         this.setConfig();
-
-        // if date changes from external source (model -> view)
-        this._subs.push(
-          this.bsValueChange.subscribe((value: Date[]) => {
-            if (this._datepickerRef) {
-              this._datepickerRef.instance.value = value;
-            }
-          })
-        );
-
-        // if date changes from picker (view -> model)
-      if (this._datepickerRef) {
-        this._subs.push(
-          this._datepickerRef.instance.valueChange
-            .pipe(
-              filter((range: Date[]) => range && range[0] && !!range[1])
-            )
-            .subscribe((value: Date[]) => {
-              this.bsValue = value;
-            })
-        );
-      }
+        this.updateSubscriptions();
     }
 
     ngOnChanges(changes: SimpleChanges): void {
         if (!this._datepickerRef || !this._datepickerRef.instance) {
           return;
         }
 
+        let shouldSetConfig = false;
+
         if (changes.minDate) {
           this._datepickerRef.instance.minDate = this.minDate;
-          this.setConfig();
+          shouldSetConfig = true;
         }
 
         if (changes.maxDate) {
           this._datepickerRef.instance.maxDate = this.maxDate;
-          this.setConfig();
+          shouldSetConfig = true;
         }
 
         if (changes.datesEnabled) {
@@ -137,25 +118,56 @@ export class BsDaterangepickerInlineDirective implements OnInit, OnDestroy, OnCh
 
         if (changes.datesDisabled) {
           this._datepickerRef.instance.datesDisabled = this.datesDisabled;
-          this.setConfig();
+          shouldSetConfig = true;
         }
 
         if (changes.daysDisabled) {
           this._datepickerRef.instance.daysDisabled = this.daysDisabled;
-          this.setConfig();
+          shouldSetConfig = true;
         }
 
         if (changes.isDisabled) {
           this._datepickerRef.instance.isDisabled = this.isDisabled;
-          this.setConfig();
+          shouldSetConfig = true;
         }
 
         if (changes.dateCustomClasses) {
           this._datepickerRef.instance.dateCustomClasses = this.dateCustomClasses;
+          shouldSetConfig = true;
+        }
+
+        if (shouldSetConfig) {
           this.setConfig();
         }
     }
 
+    updateSubscriptions(): void {
+      this._subs?.unsubscribe();
+      this._subs = new Subscription();
+
+        // if date changes from external source (model -> view)
+      this._subs.add(
+        this.bsValueChange.subscribe((value: Date[]) => {
+          if (this._datepickerRef) {
+            this._datepickerRef.instance.value = value;
+          }
+        })
+      );
+
+      // if date changes from picker (view -> model)
+      if (this._datepickerRef) {
+        this._subs.add(
+          this._datepickerRef.instance.valueChange
+            .pipe(
+              filter((range: Date[]) => range && range[0] && !!range[1])
+            )
+            .subscribe((value: Date[]) => {
+              this.bsValue = value;
+            })
+        );
+      }
+    }
+
     /**
      * Set config for datepicker
      */
@@ -182,6 +194,8 @@ export class BsDaterangepickerInlineDirective implements OnInit, OnDestroy, OnCh
         .attach(BsDaterangepickerInlineContainerComponent)
         .to(this._elementRef)
         .show();
+
+      this.updateSubscriptions();
     }
 
     ngOnDestroy() {