Skip to content
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(Time Value Validators): Propagate valueRequiredValidator to child component (DSP-1218) #257

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -331,7 +331,7 @@ describe('NoValueRequiredTestHostComponent', () => {
expect(testHostComponent).toBeTruthy();
});

it('should recieve the propagated valueRequiredValidator from the parent component', () => {
it('should receive the propagated valueRequiredValidator from the parent component', () => {
expect(testHostComponent.dateInputComponent.valueRequiredValidator).toBe(false);
});

Expand Down
Expand Up @@ -183,7 +183,7 @@ describe('InvertalInputComponent', () => {
expect(testHostComponent).toBeTruthy();
});

it('should recieve the propagated valueRequiredValidator from the parent component', () => {
it('should receive the propagated valueRequiredValidator from the parent component', () => {
expect(testHostComponent.intervalInputComponent.valueRequiredValidator).toBe(false);
});

Expand Down
@@ -1,11 +1,11 @@
<div [formGroup]="form">
<mat-form-field class="child-input-component">
<dsp-jdn-datepicker [activeCalendar]="'Gregorian'">
<input matInput class="time-input-element date" [matDatepicker]="picker" [formControlName]="'date'" (dateChange)="_handleInput()" aria-label="Date" [placeholder]="dateLabel" readonly>
<mat-datepicker-toggle matSuffix [for]="picker"></mat-datepicker-toggle>
<mat-datepicker #picker></mat-datepicker>
</dsp-jdn-datepicker>
</mat-form-field>
<dsp-jdn-datepicker [activeCalendar]="'Gregorian'">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change of the wrapping (datepicker directive, mat form field) intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's intentional so that the css of the calendar icon works correctly. It fixes this https://dasch.myjetbrains.com/youtrack/issue/DSP-647. Two birds, one stone 🤷

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, great!

<mat-form-field class="child-input-component">
<input matInput class="time-input-element date" [matDatepicker]="picker" [formControlName]="'date'" (dateChange)="_handleInput()" aria-label="Date" [placeholder]="dateLabel" readonly>
<mat-datepicker-toggle matSuffix [for]="picker"></mat-datepicker-toggle>
<mat-datepicker #picker></mat-datepicker>
</mat-form-field>
</dsp-jdn-datepicker>
<mat-form-field class="child-input-component">
<input matInput class="time-input-element time" [formControlName]="'time'" type="time" aria-label="Time" (input)="_handleInput()" [placeholder]="timeLabel" [errorStateMatcher]="matcher">
<mat-error *ngIf="form.controls.time.hasError('required')">
Expand All @@ -15,4 +15,10 @@
<span class="custom-error-message">Time should be given in precision HH:MM.</span>
</mat-error>
</mat-form-field>
<div class="date-form-error">
<mat-error *ngIf="((dateFormControl.value !== null || timeFormControl.value !== null)) &&
(dateFormControl.hasError('validDateTimeRequired') || timeFormControl.hasError('validDateTimeRequired'))">
<span class="custom-error-message">A time value must have a <strong>date</strong> and <strong>time</strong>.</span>
</mat-error>
</div>
</div>
Expand Up @@ -2,7 +2,7 @@ import { async, ComponentFixture, TestBed } from '@angular/core/testing';

import { TimeInputComponent, DateTime } from './time-input.component';
import { Component, OnInit, ViewChild, DebugElement } from '@angular/core';
import { FormGroup, FormBuilder, ReactiveFormsModule } from '@angular/forms';
import { FormGroup, FormBuilder, ReactiveFormsModule, FormControl } from '@angular/forms';
import { MatFormFieldModule } from '@angular/material/form-field';
import { MatInputModule } from '@angular/material/input';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
Expand Down Expand Up @@ -42,6 +42,35 @@ class TestHostComponent implements OnInit {
}
}

/**
* Test host component to simulate parent component.
*/
@Component({
template: `
<div [formGroup]="form">
<mat-form-field>
<dsp-time-input #timeInput [formControlName]="'time'" [valueRequiredValidator]="false"></dsp-time-input>
</mat-form-field>
</div>`
})
class NoValueRequiredTestHostComponent implements OnInit {

@ViewChild('timeInput') timeInputComponent: TimeInputComponent;

form: FormGroup;

constructor(private _fb: FormBuilder) {
}

ngOnInit(): void {

this.form = this._fb.group({
time: new FormControl(null)
});

}
}

describe('TimeInputComponent', () => {
let testHostComponent: TestHostComponent;
let testHostFixture: ComponentFixture<TestHostComponent>;
Expand Down Expand Up @@ -132,4 +161,62 @@ describe('TimeInputComponent', () => {
expect(dateTime.time).toEqual('12:00');

});

it('should mark the form\'s validity correctly', () => {
expect(testHostComponent.timeInputComponent.valueRequiredValidator).toBe(true);
expect(testHostComponent.timeInputComponent.form.valid).toBe(true);

testHostComponent.timeInputComponent.timeFormControl.setValue(null);

testHostComponent.timeInputComponent._handleInput();

expect(testHostComponent.timeInputComponent.form.valid).toBe(false);

testHostComponent.timeInputComponent.timeFormControl.setValue("");

testHostComponent.timeInputComponent._handleInput();

expect(testHostComponent.timeInputComponent.form.valid).toBe(false);
});
});

describe('TimeInputComponent no value required', () => {
let testHostComponent: NoValueRequiredTestHostComponent;
let testHostFixture: ComponentFixture<NoValueRequiredTestHostComponent>;

beforeEach(async(() => {
TestBed.configureTestingModule({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need this setup again?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how it's done in the other complex value spec files. It could possibly be refactored at a later point but I don't think it's necessary.

imports: [
ReactiveFormsModule,
MatFormFieldModule,
MatInputModule,
MatDatepickerModule,
MatJDNConvertibleCalendarDateAdapterModule,
BrowserAnimationsModule],
declarations: [TimeInputComponent, NoValueRequiredTestHostComponent, JDNDatepickerDirective]
})
.compileComponents();
}));

beforeEach(() => {
testHostFixture = TestBed.createComponent(NoValueRequiredTestHostComponent);
testHostComponent = testHostFixture.componentInstance;
testHostFixture.detectChanges();

expect(testHostComponent).toBeTruthy();
});

it('should receive the propagated valueRequiredValidator from the parent component', () => {
expect(testHostComponent.timeInputComponent.valueRequiredValidator).toBe(false);
});

it('should mark the form\'s validity correctly', () => {
expect(testHostComponent.timeInputComponent.form.valid).toBe(true);

testHostComponent.timeInputComponent.timeFormControl.setValue('2019-08-06T12:00:00Z');

testHostComponent.timeInputComponent._handleInput();

expect(testHostComponent.timeInputComponent.form.valid).toBe(false);
});
});
Expand Up @@ -2,15 +2,17 @@ import { FocusMonitor } from '@angular/cdk/a11y';
import { coerceBooleanProperty } from '@angular/cdk/coercion';
import { DatePipe } from '@angular/common';
import { ValueErrorStateMatcher } from '../../value-error-state-matcher';
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 {
AbstractControl,
ControlValueAccessor,
FormBuilder,
FormControl,
FormGroup,
FormGroupDirective,
NgControl,
NgForm,
ValidatorFn,
Validators
} from '@angular/forms';
import { CanUpdateErrorState, CanUpdateErrorStateCtor, ErrorStateMatcher, mixinErrorState } from '@angular/material/core';
Expand All @@ -19,6 +21,17 @@ import { CalendarDate, CalendarPeriod, GregorianCalendarDate } from 'jdnconverti
import { Subject } from 'rxjs';
import { CustomRegex } from '../../custom-regex';

/** A valid time value must have both a date and a time, or both inputs must be null */
export function dateTimeValidator(otherControl: FormControl): ValidatorFn {
return (control: AbstractControl): { [key: string]: any } | null => {

// valid if both date and time are null or have values, excluding empty strings
const invalid = !(control.value === null && otherControl.value === null ||
((control.value !== null && control.value !== '') && (otherControl.value !== null && otherControl.value !== '')));

return invalid ? { 'validDateTimeRequired': { value: control.value } } : null;
};
}

class MatInputBase {
constructor(public _defaultErrorStateMatcher: ErrorStateMatcher,
Expand All @@ -43,7 +56,7 @@ export class DateTime {
styleUrls: ['./time-input.component.scss'],
providers: [{ provide: MatFormFieldControl, useExisting: TimeInputComponent }]
})
export class TimeInputComponent extends _MatInputMixinBase implements ControlValueAccessor, MatFormFieldControl<string>, DoCheck, CanUpdateErrorState, OnDestroy {
export class TimeInputComponent extends _MatInputMixinBase implements ControlValueAccessor, MatFormFieldControl<string>, DoCheck, CanUpdateErrorState, OnDestroy, OnInit {

static nextId = 0;

Expand All @@ -59,6 +72,7 @@ export class TimeInputComponent extends _MatInputMixinBase implements ControlVal

@Input() dateLabel = 'Date';
@Input() timeLabel = 'Time';
@Input() valueRequiredValidator = true;

dateFormControl: FormControl;
timeFormControl: FormControl;
Expand Down Expand Up @@ -144,6 +158,10 @@ export class TimeInputComponent extends _MatInputMixinBase implements ControlVal
} else {
this.form.setValue({ date: null, time: null });
}

this.dateFormControl.updateValueAndValidity();
this.timeFormControl.updateValueAndValidity();

this.stateChanges.next();
}

Expand All @@ -159,9 +177,9 @@ export class TimeInputComponent extends _MatInputMixinBase implements ControlVal

super(_defaultErrorStateMatcher, _parentForm, _parentFormGroup, ngControl);

this.dateFormControl = new FormControl(null, [Validators.required]);
this.dateFormControl = new FormControl(null);

this.timeFormControl = new FormControl(null, [Validators.required, Validators.pattern(CustomRegex.TIME_REGEX)]);
this.timeFormControl = new FormControl(null);

this.form = fb.group({
date: this.dateFormControl,
Expand All @@ -178,6 +196,19 @@ export class TimeInputComponent extends _MatInputMixinBase implements ControlVal
}
}

ngOnInit() {
if (this.valueRequiredValidator) {
this.dateFormControl.setValidators([Validators.required, dateTimeValidator(this.timeFormControl)]);
this.timeFormControl.setValidators([Validators.required, dateTimeValidator(this.dateFormControl), Validators.pattern(CustomRegex.TIME_REGEX)]);
} else {
this.dateFormControl.setValidators(dateTimeValidator(this.timeFormControl));
this.timeFormControl.setValidators([dateTimeValidator(this.dateFormControl), Validators.pattern(CustomRegex.TIME_REGEX)]);
}

this.dateFormControl.updateValueAndValidity();
this.timeFormControl.updateValueAndValidity();
}

ngDoCheck() {
if (this.ngControl) {
this.updateErrorState();
Expand Down Expand Up @@ -211,6 +242,8 @@ export class TimeInputComponent extends _MatInputMixinBase implements ControlVal
}

_handleInput(): void {
this.dateFormControl.updateValueAndValidity();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't, dateTimeValidator is not triggered and the error message from it remains.
Screen Shot 2021-01-14 at 10 42 33

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this also the case in the other components like date and interval? Or is there something special about time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing special about time (this time), we do it in the Date and Interval value components as well.

this.timeFormControl.updateValueAndValidity();
this.onChange(this.value);
}

Expand Down
Expand Up @@ -6,7 +6,7 @@
<ng-template #showForm>
<span [formGroup]="form" class="parent-component-wrapper">
<mat-form-field class="large-field child-value-component" floatLabel="never">
<dsp-time-input #timeInput [formControlName]="'value'" class="value" [errorStateMatcher]="matcher"></dsp-time-input>
<dsp-time-input #timeInput [formControlName]="'value'" class="value" [errorStateMatcher]="matcher" [valueRequiredValidator]="valueRequiredValidator"></dsp-time-input>
<mat-error *ngIf="valueFormControl.hasError('valueNotChanged') &&
(valueFormControl.touched || valueFormControl.dirty)">
<span class="custom-error-message">New value must be different than the current value.</span>
Expand Down