Skip to content

Commit

Permalink
fix(forms): form items set touched too late
Browse files Browse the repository at this point in the history 
Signed-off-by: Ivan Donchev <idonchev@vmware.com>
  • Loading branch information
@Jinnie
Jinnie committed Nov 21, 2022
1 parent dd053e5 commit 05be9f0
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 26 deletions.
16 changes: 10 additions & 6 deletions projects/angular/src/forms/common/if-control-state/if-control-state.service.spec.ts
View file
Expand Up @@ -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';
Expand All @@ -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');
Expand Down Expand Up @@ -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 = {
Expand All @@ -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 = {
Expand All @@ -127,8 +130,9 @@ export default function (): void {
};
ngControlService.setControl(fakeControl);
service.triggerStatusChange();
tick();
expect(cb).toHaveBeenCalledWith(CONTROL_STATE.VALID);
sub.unsubscribe();
});
}));
});
}
11 changes: 8 additions & 3 deletions projects/angular/src/forms/common/if-control-state/if-control-state.service.ts
View file
Expand Up @@ -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
);
});
}
}

Expand Down
36 changes: 23 additions & 13 deletions projects/angular/src/forms/common/if-control-state/if-error.spec.ts
View file
Expand Up @@ -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';
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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),
Expand All @@ -151,6 +157,7 @@ export default function (): void {
control.markAsTouched();
ngControlService.setControl(control);
ifControlStateService.triggerStatusChange();
tick();
fixture.detectChanges();

// Required message
Expand All @@ -160,25 +167,28 @@ 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);
expect(fixture.nativeElement.innerHTML).not.toContain(maxLengthMessage);

// MaxLength message
control.setValue('abcdef');
tick();
fixture.detectChanges();
expect(fixture.nativeElement.innerHTML).not.toContain(errorMessage);
expect(fixture.nativeElement.innerHTML).not.toContain(minLengthMessage);
expect(fixture.nativeElement.innerHTML).toContain(maxLengthMessage);

// 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);
});
}));
});
});
}
9 changes: 5 additions & 4 deletions projects/angular/src/forms/tests/container.spec.ts
View file
Expand Up @@ -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';

Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 05be9f0

Please sign in to comment.