fix(Time Value Validators): Propagate valueRequiredValidator to child component (DSP-1218) #257
Conversation
mdelez
changed the title
|
@tobiasschweizer FYI I formatted the input-value spec file so that's why there are 495 changes in that file. The relevant part is at the bottom. |
| @@ -211,6 +246,8 @@ export class TimeInputComponent extends _MatInputMixinBase implements ControlVal | |||
| } | |||
|
|
|||
| _handleInput(): void { | |||
| this.dateFormControl.updateValueAndValidity(); | |||
There was a problem hiding this comment.
Why is this needed here?
There was a problem hiding this comment.
If we don't, dateTimeValidator is not triggered and the error message from it remains.
There was a problem hiding this comment.
Is this also the case in the other components like date and interval? Or is there something special about time?
There was a problem hiding this comment.
Nothing special about time (this time), we do it in the Date and Interval value components as well.
| let invalid = !(control.value === null && otherControl.value === null || control.value !== null && otherControl.value !== null); | ||
|
|
||
| // ensure the time value does not contain an empty string | ||
| if (control.value === "" || otherControl.value === "") { |
There was a problem hiding this comment.
Couldn't this be written all in one expression?
| <mat-datepicker #picker></mat-datepicker> | ||
| </dsp-jdn-datepicker> | ||
| </mat-form-field> | ||
| <dsp-jdn-datepicker [activeCalendar]="'Gregorian'"> |
There was a problem hiding this comment.
Is this change of the wrapping (datepicker directive, mat form field) intentional?
There was a problem hiding this comment.
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 🤷
| @@ -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.hasError('validDateTimeRequired') || timeFormControl.hasError('validDateTimeRequired')) && | |||
There was a problem hiding this comment.
Maybe it would be easier to understand this if the null checks come first.
| let testHostFixture: ComponentFixture<NoValueRequiredTestHostComponent>; | ||
|
|
||
| beforeEach(async(() => { | ||
| TestBed.configureTestingModule({ |
There was a problem hiding this comment.
Do you need this setup again?
There was a problem hiding this comment.
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.
…o a misspelled word
mdelez
deleted the
wip/DSP-1218-time-value-required-validator-error-messages
branch
resolves DSP-1218