From 05be9f05c4dade845f0159b1ea361bc0b36777dd Mon Sep 17 00:00:00 2001 From: Ivan Donchev Date: Fri, 18 Nov 2022 09:50:26 +0200 Subject: [PATCH] fix(forms): form items set touched too late Signed-off-by: Ivan Donchev --- .../if-control-state.service.spec.ts | 16 +++++---- .../if-control-state.service.ts | 11 ++++-- .../common/if-control-state/if-error.spec.ts | 36 ++++++++++++------- .../angular/src/forms/tests/container.spec.ts | 9 ++--- 4 files changed, 46 insertions(+), 26 deletions(-) diff --git a/projects/angular/src/forms/common/if-control-state/if-control-state.service.spec.ts b/projects/angular/src/forms/common/if-control-state/if-control-state.service.spec.ts index 889c1c2aec..58773ee0c1 100644 --- a/projects/angular/src/forms/common/if-control-state/if-control-state.service.spec.ts +++ b/projects/angular/src/forms/common/if-control-state/if-control-state.service.spec.ts @@ -4,6 +4,7 @@ * The full license information can be found in LICENSE in the root directory of this project. */ +import { fakeAsync, tick } from '@angular/core/testing'; import { FormControl } from '@angular/forms'; import { NgControlService } from '../providers/ng-control.service'; @@ -29,16 +30,17 @@ export default function (): void { expect(() => service.triggerStatusChange()).not.toThrowError(); }); - it('provides observable for statusChanges, return valid when touched and no rules added', () => { + it('provides observable for statusChanges, return valid when touched and no rules added', fakeAsync(() => { const cb = jasmine.createSpy('cb'); const sub = service.statusChanges.subscribe((control: CONTROL_STATE) => cb(control)); ngControlService.setControl(testControl); // Change the state of the input to trigger statusChange testControl.markAsTouched(); testControl.updateValueAndValidity(); + tick(); expect(cb).toHaveBeenCalledWith(CONTROL_STATE.VALID); sub.unsubscribe(); - }); + })); it('should allow a manual trigger of status observable, return NONE', () => { const cb = jasmine.createSpy('cb'); @@ -91,7 +93,7 @@ export default function (): void { sub.unsubscribe(); }); - it('should return state INVALID', () => { + it('should return state INVALID', fakeAsync(() => { const cb = jasmine.createSpy('cb'); const sub = service.statusChanges.subscribe((control: CONTROL_STATE) => cb(control)); const fakeControl = { @@ -107,11 +109,12 @@ export default function (): void { }; ngControlService.setControl(fakeControl); service.triggerStatusChange(); + tick(); expect(cb).toHaveBeenCalledWith(CONTROL_STATE.INVALID); sub.unsubscribe(); - }); + })); - it('should return state VALID', () => { + it('should return state VALID', fakeAsync(() => { const cb = jasmine.createSpy('cb'); const sub = service.statusChanges.subscribe((control: CONTROL_STATE) => cb(control)); const fakeControl = { @@ -127,8 +130,9 @@ export default function (): void { }; ngControlService.setControl(fakeControl); service.triggerStatusChange(); + tick(); expect(cb).toHaveBeenCalledWith(CONTROL_STATE.VALID); sub.unsubscribe(); - }); + })); }); } diff --git a/projects/angular/src/forms/common/if-control-state/if-control-state.service.ts b/projects/angular/src/forms/common/if-control-state/if-control-state.service.ts index 79b29ee043..7a642b0bb4 100644 --- a/projects/angular/src/forms/common/if-control-state/if-control-state.service.ts +++ b/projects/angular/src/forms/common/if-control-state/if-control-state.service.ts @@ -51,9 +51,14 @@ export class IfControlStateService implements OnDestroy { // These status values are mutually exclusive, so a control // cannot be both valid AND invalid or invalid AND disabled. const status = CONTROL_STATE[this.control.status]; - this._statusChanges.next( - this.control.touched && ['VALID', 'INVALID'].includes(status) ? status : CONTROL_STATE.NONE - ); + // At some point, with Angular 14, we started receiving the update for the control.touched + // later than this moment in code. It depends on the order of imports of ClarityModule and + // the FormsModule. This delay makes sure the touched state is properly updated. + setTimeout(() => { + this._statusChanges.next( + this.control.touched && ['VALID', 'INVALID'].includes(status) ? status : CONTROL_STATE.NONE + ); + }); } } diff --git a/projects/angular/src/forms/common/if-control-state/if-error.spec.ts b/projects/angular/src/forms/common/if-control-state/if-error.spec.ts index c18c35105f..5c4a234c84 100644 --- a/projects/angular/src/forms/common/if-control-state/if-error.spec.ts +++ b/projects/angular/src/forms/common/if-control-state/if-error.spec.ts @@ -5,7 +5,7 @@ */ import { Component } from '@angular/core'; -import { TestBed } from '@angular/core/testing'; +import { fakeAsync, TestBed, tick } from '@angular/core/testing'; import { FormControl, FormsModule, Validators } from '@angular/forms'; import { ClrIconModule } from '../../../icon/icon.module'; @@ -73,15 +73,16 @@ export default function (): void { expect(fixture.nativeElement.innerHTML).not.toContain(errorMessage); }); - it('displays the error message after touched on general errors', () => { + it('displays the error message after touched on general errors', fakeAsync(() => { expect(fixture.nativeElement.innerHTML).not.toContain(errorMessage); const control = new FormControl('', Validators.required); control.markAsTouched(); ngControlService.setControl(control); ifControlStateService.triggerStatusChange(); fixture.detectChanges(); + tick(); expect(fixture.nativeElement.innerHTML).toContain(errorMessage); - }); + })); }); describe('specific error', () => { @@ -102,47 +103,52 @@ export default function (): void { expect(fixture.nativeElement.innerHTML).not.toContain(errorMessage); }); - it('displays the error when the specific error is defined', () => { + it('displays the error when the specific error is defined', fakeAsync(() => { expect(fixture.nativeElement.innerHTML).not.toContain(errorMessage); const control = new FormControl('', [Validators.required, Validators.minLength(5)]); control.markAsTouched(); ngControlService.setControl(control); ifControlStateService.triggerStatusChange(); + tick(); fixture.detectChanges(); expect(fixture.nativeElement.innerHTML).toContain(errorMessage); - }); + })); - it('hides the message even when it is invalid due to a different validation error', () => { + it('hides the message even when it is invalid due to a different validation error', fakeAsync(() => { expect(fixture.nativeElement.innerHTML).not.toContain(errorMessage); const control = new FormControl('abc', [Validators.required, Validators.minLength(5)]); ngControlService.setControl(control); ifControlStateService.triggerStatusChange(); + tick(); fixture.detectChanges(); expect(fixture.nativeElement.innerHTML).not.toContain(errorMessage); expect(fixture.nativeElement.innerHTML).toContain(minLengthMessage); - }); + })); - it('displays the error message with values from error object in context', () => { + it('displays the error message with values from error object in context', fakeAsync(() => { const control = new FormControl('abcdef', [Validators.maxLength(5)]); ngControlService.setControl(control); ifControlStateService.triggerStatusChange(); + tick(); fixture.detectChanges(); expect(fixture.nativeElement.innerHTML).toContain(`${maxLengthMessage}-5-6`); - }); + })); - it('updates the error message with values from error object in context', () => { + it('updates the error message with values from error object in context', fakeAsync(() => { const control = new FormControl('abcdef', [Validators.maxLength(5)]); ngControlService.setControl(control); ifControlStateService.triggerStatusChange(); + tick(); fixture.detectChanges(); expect(fixture.nativeElement.innerHTML).toContain(`${maxLengthMessage}-5-6`); control.setValue('abcdefg'); + tick(); fixture.detectChanges(); expect(fixture.nativeElement.innerHTML).toContain(`${maxLengthMessage}-5-7`); - }); + })); - it('should show error only when they are required', () => { + it('should show error only when they are required', fakeAsync(() => { const control = new FormControl(undefined, [ Validators.required, Validators.minLength(4), @@ -151,6 +157,7 @@ export default function (): void { control.markAsTouched(); ngControlService.setControl(control); ifControlStateService.triggerStatusChange(); + tick(); fixture.detectChanges(); // Required message @@ -160,6 +167,7 @@ export default function (): void { // MinLength message control.setValue('abc'); + tick(); fixture.detectChanges(); expect(fixture.nativeElement.innerHTML).not.toContain(errorMessage); expect(fixture.nativeElement.innerHTML).toContain(minLengthMessage); @@ -167,6 +175,7 @@ export default function (): void { // MaxLength message control.setValue('abcdef'); + tick(); fixture.detectChanges(); expect(fixture.nativeElement.innerHTML).not.toContain(errorMessage); expect(fixture.nativeElement.innerHTML).not.toContain(minLengthMessage); @@ -174,11 +183,12 @@ export default function (): void { // No errors control.setValue('abcde'); + tick(); fixture.detectChanges(); expect(fixture.nativeElement.innerHTML).not.toContain(errorMessage); expect(fixture.nativeElement.innerHTML).not.toContain(minLengthMessage); expect(fixture.nativeElement.innerHTML).not.toContain(maxLengthMessage); - }); + })); }); }); } diff --git a/projects/angular/src/forms/tests/container.spec.ts b/projects/angular/src/forms/tests/container.spec.ts index 96e6c067c4..3e69a4f091 100644 --- a/projects/angular/src/forms/tests/container.spec.ts +++ b/projects/angular/src/forms/tests/container.spec.ts @@ -4,7 +4,7 @@ * The full license information can be found in LICENSE in the root directory of this project. */ -import { async, TestBed } from '@angular/core/testing'; +import { fakeAsync, TestBed, tick, waitForAsync } from '@angular/core/testing'; import { FormsModule, NgControl, ReactiveFormsModule } from '@angular/forms'; import { By } from '@angular/platform-browser'; @@ -194,14 +194,15 @@ function fullSpec(description, testContainer, directives: any | any[], testCompo expect(container.controlClass()).not.toContain('clr-col-12'); }); - it('tracks the validity of the form control', () => { + it('tracks the validity of the form control', fakeAsync(() => { expect(container.showInvalid).toBeFalse(); markControlService.markAsTouched(); fixture.detectChanges(); + tick(); expect(container.showInvalid).toBeTrue(); - }); + })); - it('tracks the disabled state', async(() => { + it('tracks the disabled state', waitForAsync(() => { const test = fixture.debugElement.componentInstance; test.disabled = true; fixture.detectChanges();