fix(forms): pr feedback

vmware-clarity · Dec 18, 2023 · c60c1d7 · c60c1d7
1 parent c9f1855
commit c60c1d7
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 27 deletions.
diff --git a/projects/angular/clarity.api.md b/projects/angular/clarity.api.md
@@ -4834,7 +4834,7 @@ export const TOGGLE_SERVICE_PROVIDER: {
 export function ToggleServiceFactory(): BehaviorSubject<boolean>;
 
 // @public (undocumented)
-export class WrappedFormControl<W extends DynamicWrapper> implements OnInit, OnDestroy, DoCheck {
+export class WrappedFormControl<W extends DynamicWrapper> implements OnInit, DoCheck, OnDestroy {
     constructor(vcr: ViewContainerRef, wrapperType: Type<W>, injector: Injector, ngControl: NgControl, renderer: Renderer2, el: ElementRef);
     // (undocumented)
     protected controlIdService: ControlIdService;

diff --git a/projects/angular/src/forms/common/wrapped-control.spec.ts b/projects/angular/src/forms/common/wrapped-control.spec.ts
@@ -213,6 +213,7 @@ class WithDynamicFormControl {
     this.form.addControl('control', new FormControl('TEST'));
   }
 }
+
 @Component({
   template: `
     <form-wrapper>
@@ -388,6 +389,7 @@ export default function (): void {
         expect(cb).toHaveBeenCalledWith(CONTROL_STATE.VALID);
         sub.unsubscribe();
       });
+
       it('with ng-control directive', function (this: TestContext) {
         setupTest(this, WithDynamicNgControl, TestControl3);
         const cb = jasmine.createSpy('cb');

diff --git a/projects/angular/src/forms/common/wrapped-control.ts b/projects/angular/src/forms/common/wrapped-control.ts
@@ -40,7 +40,7 @@ export enum CHANGE_KEYS {
 }
 
 @Directive()
-export class WrappedFormControl<W extends DynamicWrapper> implements OnInit, OnDestroy, DoCheck {
+export class WrappedFormControl<W extends DynamicWrapper> implements OnInit, DoCheck, OnDestroy {
   _id: string;
 
   protected renderer: Renderer2;
@@ -115,22 +115,6 @@ export class WrappedFormControl<W extends DynamicWrapper> implements OnInit, OnD
     }
   }
 
-  ngDoCheck() {
-    if (this.differ) {
-      const changes = this.differ.diff(this.ngControl);
-      if (changes) {
-        changes.forEachChangedItem(change => {
-          if (
-            (change.key === CHANGE_KEYS.FORM || change.key === CHANGE_KEYS.MODEL) &&
-            change.currentValue !== change.previousValue
-          ) {
-            this.triggerValidation();
-          }
-        });
-      }
-    }
-  }
-
   ngOnInit() {
     this._containerInjector = new HostWrapper(this.wrapperType, this.vcr, this.index);
     this.controlIdService = this._containerInjector.get(ControlIdService);
@@ -151,6 +135,22 @@ export class WrappedFormControl<W extends DynamicWrapper> implements OnInit, OnD
     }
   }
 
+  ngDoCheck() {
+    if (this.differ) {
+      const changes = this.differ.diff(this.ngControl);
+      if (changes) {
+        changes.forEachChangedItem(change => {
+          if (
+            (change.key === CHANGE_KEYS.FORM || change.key === CHANGE_KEYS.MODEL) &&
+            change.currentValue !== change.previousValue
+          ) {
+            this.triggerValidation();
+          }
+        });
+      }
+    }
+  }
+
   ngOnDestroy() {
     this.subscriptions.forEach(sub => sub.unsubscribe());
   }

diff --git a/projects/demo/src/app/forms/dynamic-controls/dynamic-controls.demo.html b/projects/demo/src/app/forms/dynamic-controls/dynamic-controls.demo.html
@@ -3,8 +3,8 @@ <h1>Dynamic Control Demo</h1>
 <p>
   We have a State service which have three statuses: NONE, INVALID, VALID. Success and Error messages are shown based on
   the value of this state service but if you bind formControl \ ngModel to a control that is currently not available and
-  then add it dynamically while marking it as touched and updateValueValidility status change doesn't trigger and state
-  service value is not updated.
+  then add it dynamically with initial value while marking it as touched and updateValueValidility status change doesn't
+  trigger and state service value is not updated.
 </p>
 <br />
 <label for="steps"> Steps to test:</label>

diff --git a/projects/demo/src/app/forms/dynamic-controls/dynamic-controls.demo.ts b/projects/demo/src/app/forms/dynamic-controls/dynamic-controls.demo.ts
@@ -7,7 +7,7 @@
 import { ChangeDetectorRef, Component } from '@angular/core';
 import { FormControl, FormGroup } from '@angular/forms';
 import { Subject } from 'rxjs';
-import { takeUntil } from 'rxjs/operators';
+import { filter, takeUntil, tap } from 'rxjs/operators';
 
 @Component({
   selector: 'dynamic-controls-demo',
@@ -25,18 +25,20 @@ export class DynamicControlsDemo {
   ngOnInit() {
     this.form
       .get('platform')
-      ?.valueChanges.pipe(takeUntil(this.destroyed))
-      .subscribe(res => {
-        console.log(res);
-        if (res === 'two') {
+      ?.valueChanges.pipe(
+        filter(value => value === 'two'),
+        tap(() => {
           this.form.addControl('control', new FormControl('test'));
           this.form.get('control')?.markAsTouched();
           this.form.get('control')?.updateValueAndValidity();
-        }
-      });
+        }),
+        takeUntil(this.destroyed)
+      )
+      .subscribe();
   }
 
   ngOnDestroy() {
+    this.destroyed.next();
     this.destroyed.complete();
   }
 }