New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(Interval Value Validators): Propagate valueRequiredValidator to child component (DSP-1193) #253
Changes from 4 commits
05d6d15
dff88e1
d5e8618
71c88f1
3c0a216
19f996d
020a5f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
import { async, ComponentFixture, TestBed } from '@angular/core/testing'; | ||
|
||
import { Interval, IntervalInputComponent } from './interval-input.component'; | ||
import { FormBuilder, FormGroup, ReactiveFormsModule } from '@angular/forms'; | ||
import { FormBuilder, FormControl, FormGroup, ReactiveFormsModule } from '@angular/forms'; | ||
import { Component, DebugElement, OnInit, ViewChild } from '@angular/core'; | ||
import { MatFormFieldModule } from '@angular/material/form-field'; | ||
import { BrowserAnimationsModule } from '@angular/platform-browser/animations'; | ||
|
@@ -37,6 +37,35 @@ class TestHostComponent implements OnInit { | |
} | ||
} | ||
|
||
/** | ||
* Test host component to simulate parent component. | ||
*/ | ||
@Component({ | ||
template: ` | ||
<div [formGroup]="form"> | ||
<mat-form-field> | ||
<dsp-interval-input #intervalInput [formControlName]="'interval'" [valueRequiredValidator]="false"></dsp-interval-input> | ||
</mat-form-field> | ||
</div>` | ||
}) | ||
class NoValueRequiredTestHostComponent implements OnInit { | ||
|
||
@ViewChild('intervalInput') intervalInputComponent: IntervalInputComponent; | ||
|
||
form: FormGroup; | ||
|
||
constructor(private _fb: FormBuilder) { | ||
} | ||
|
||
ngOnInit(): void { | ||
|
||
this.form = this._fb.group({ | ||
interval: new FormControl(null) | ||
}); | ||
|
||
} | ||
} | ||
|
||
describe('InvertalInputComponent', () => { | ||
let testHostComponent: TestHostComponent; | ||
let testHostFixture: ComponentFixture<TestHostComponent>; | ||
|
@@ -121,4 +150,45 @@ describe('InvertalInputComponent', () => { | |
|
||
}); | ||
|
||
it('should mark the form\'s validity correctly', () => { | ||
expect(testHostComponent.intervalInputComponent.valueRequiredValidator).toBe(true); | ||
expect(testHostComponent.intervalInputComponent.form.valid).toBe(true); | ||
|
||
testHostComponent.intervalInputComponent.startIntervalControl.setValue(null); | ||
|
||
testHostComponent.intervalInputComponent._handleInput(); | ||
|
||
expect(testHostComponent.intervalInputComponent.form.valid).toBe(false); | ||
}); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a test case for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done in 19f996d |
||
}); | ||
|
||
describe('InvertalInputComponent', () => { | ||
let testHostComponent: NoValueRequiredTestHostComponent; | ||
let testHostFixture: ComponentFixture<NoValueRequiredTestHostComponent>; | ||
|
||
beforeEach(async(() => { | ||
TestBed.configureTestingModule({ | ||
imports: [ReactiveFormsModule, MatFormFieldModule, MatInputModule, BrowserAnimationsModule], | ||
declarations: [IntervalInputComponent, NoValueRequiredTestHostComponent] | ||
}) | ||
.compileComponents(); | ||
})); | ||
|
||
beforeEach(() => { | ||
testHostFixture = TestBed.createComponent(NoValueRequiredTestHostComponent); | ||
testHostComponent = testHostFixture.componentInstance; | ||
testHostFixture.detectChanges(); | ||
|
||
expect(testHostComponent).toBeTruthy(); | ||
}); | ||
|
||
it('should recieve the propagated valueRequiredValidator from the parent component', () => { | ||
expect(testHostComponent.intervalInputComponent.valueRequiredValidator).toBe(false); | ||
}); | ||
|
||
it('should mark the form\'s validity correctly', () => { | ||
expect(testHostComponent.intervalInputComponent.form.valid).toBe(true); | ||
}); | ||
|
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import { Component, DoCheck, ElementRef, HostBinding, Input, OnDestroy, Optional, Self } from '@angular/core'; | ||
import { Component, DoCheck, ElementRef, HostBinding, Input, OnDestroy, OnInit, Optional, Self } from '@angular/core'; | ||
import { MatFormFieldControl } from '@angular/material/form-field'; | ||
import { ControlValueAccessor, FormBuilder, FormControl, FormGroup, FormGroupDirective, NgControl, NgForm, Validators } from '@angular/forms'; | ||
import { AbstractControl, ControlValueAccessor, FormBuilder, FormControl, FormGroup, FormGroupDirective, NgControl, NgForm, ValidatorFn, Validators } from '@angular/forms'; | ||
import { Subject } from 'rxjs'; | ||
import { FocusMonitor } from '@angular/cdk/a11y'; | ||
import { coerceBooleanProperty } from '@angular/cdk/coercion'; | ||
|
@@ -27,6 +27,20 @@ export class IntervalInputErrorStateMatcher implements ErrorStateMatcher { | |
} | ||
} | ||
|
||
/** Interval must have a start and end of the same type, either both numbers or both null */ | ||
export function startEndSameTypeValidator(otherInterval: FormControl): ValidatorFn { | ||
return (control: AbstractControl): { [key: string]: any } | null => { | ||
|
||
let invalid = true; | ||
if (typeof(control.value) === 'number' && typeof(otherInterval.value) === 'number' || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't you just check for Something like I think the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was me being overly paranoid. Refactored in 3c0a216 |
||
typeof(control.value) === 'object' && typeof(otherInterval.value) === 'object') { | ||
invalid = false; | ||
} | ||
|
||
return invalid ? { 'startEndSameTypeRequired': { value: control.value } } : null; | ||
}; | ||
} | ||
|
||
class MatInputBase { | ||
constructor(public _defaultErrorStateMatcher: ErrorStateMatcher, | ||
public _parentForm: NgForm, | ||
|
@@ -43,7 +57,7 @@ const _MatInputMixinBase: CanUpdateErrorStateCtor & typeof MatInputBase = | |
styleUrls: ['./interval-input.component.scss'], | ||
providers: [{ provide: MatFormFieldControl, useExisting: IntervalInputComponent }] | ||
}) | ||
export class IntervalInputComponent extends _MatInputMixinBase implements ControlValueAccessor, MatFormFieldControl<Interval>, DoCheck, CanUpdateErrorState, OnDestroy { | ||
export class IntervalInputComponent extends _MatInputMixinBase implements ControlValueAccessor, MatFormFieldControl<Interval>, DoCheck, CanUpdateErrorState, OnDestroy, OnInit { | ||
static nextId = 0; | ||
|
||
form: FormGroup; | ||
|
@@ -53,11 +67,16 @@ export class IntervalInputComponent extends _MatInputMixinBase implements Contro | |
errorState = false; | ||
controlType = 'dsp-interval-input'; | ||
matcher = new IntervalInputErrorStateMatcher(); | ||
onChange = (_: any) => { }; | ||
onTouched = () => { }; | ||
|
||
startIntervalControl: FormControl; | ||
endIntervalControl: FormControl; | ||
|
||
@Input() intervalStartLabel = 'start'; | ||
@Input() intervalEndLabel = 'end'; | ||
@Input() valueRequiredValidator = true; | ||
|
||
onChange = (_: any) => { }; | ||
onTouched = () => { }; | ||
|
||
get empty() { | ||
const userInput = this.form.value; | ||
|
@@ -127,6 +146,10 @@ export class IntervalInputComponent extends _MatInputMixinBase implements Contro | |
} else { | ||
this.form.setValue({ start: null, end: null }); | ||
} | ||
|
||
this.startIntervalControl.updateValueAndValidity(); | ||
this.endIntervalControl.updateValueAndValidity(); | ||
|
||
this.stateChanges.next(); | ||
} | ||
|
||
|
@@ -142,9 +165,12 @@ export class IntervalInputComponent extends _MatInputMixinBase implements Contro | |
|
||
super(_defaultErrorStateMatcher, _parentForm, _parentFormGroup, ngControl); | ||
|
||
this.startIntervalControl = new FormControl(null); | ||
this.endIntervalControl = new FormControl(null); | ||
|
||
this.form = fb.group({ | ||
start: [null, Validators.required], | ||
end: [null, Validators.required] | ||
start: this.startIntervalControl, | ||
end: this.endIntervalControl | ||
}); | ||
|
||
_fm.monitor(_elRef.nativeElement, true).subscribe(origin => { | ||
|
@@ -157,6 +183,16 @@ export class IntervalInputComponent extends _MatInputMixinBase implements Contro | |
} | ||
} | ||
|
||
ngOnInit() { | ||
if (this.valueRequiredValidator) { | ||
this.startIntervalControl.setValidators([Validators.required, startEndSameTypeValidator(this.endIntervalControl)]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to add the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely, I forgot. Added in 3c0a216 |
||
this.endIntervalControl.setValidators([Validators.required, startEndSameTypeValidator(this.startIntervalControl)]); | ||
} | ||
|
||
this.startIntervalControl.updateValueAndValidity(); | ||
this.endIntervalControl.updateValueAndValidity(); | ||
} | ||
|
||
ngDoCheck() { | ||
if (this.ngControl) { | ||
this.updateErrorState(); | ||
|
@@ -190,6 +226,8 @@ export class IntervalInputComponent extends _MatInputMixinBase implements Contro | |
} | ||
|
||
_handleInput(): void { | ||
this.startIntervalControl.updateValueAndValidity(); | ||
this.endIntervalControl.updateValueAndValidity(); | ||
this.onChange(this.value); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a need for null checks for start or end as in case of the date?
Or is this taken care of by
startEndSameTypeValidator
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is handled in
startEndSameTypeValidator