fix(Interval Value Validators): Propagate valueRequiredValidator to child component (DSP-1193) #253
Conversation
mdelez
changed the title
…test form validity
|
@tobiasschweizer same thing as #251 but for the interval value component |
| @@ -11,4 +11,10 @@ | |||
| End is <strong>required</strong>. | |||
| </mat-error> | |||
| </mat-form-field> | |||
|
|
|||
| <div class="date-form-error"> | |||
| <mat-error *ngIf="((startIntervalControl.hasError('startEndSameTypeRequired') || endIntervalControl.hasError('startEndSameTypeRequired')))"> | |||
There was a problem hiding this comment.
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.
this is handled in startEndSameTypeValidator
| 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.
Couldn't you just check for null?
Something like invalid = !(control.value === null && otherInterval.value === null || control.value !== null && otherInterval.value !== null)
I think the input only allows for numeric values.
There was a problem hiding this comment.
This was me being overly paranoid. Refactored in 3c0a216
| @@ -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.
Would it make sense to add the startEndSameTypeValidator also when the required validator is not set?
There was a problem hiding this comment.
Definitely, I forgot. Added in 3c0a216
|
|
||
| expect(testHostComponent.intervalInputComponent.form.valid).toBe(false); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Could you add a test case for the startEndSameTypeValidator? Or is this not possible because the required operator will always trigger anyway (start or end is null)?
…ols even if the value is not required & refactors startEndSameTypeValidator validity logic
…alidator functionality
|
@flavens With these changes, you can make an interval not required. However you will still see an error message if the interval is not entered correctly (i.e. only the start interval has been entered). There isn't really an intuitive way to disregard this error if the interval is not required because we want to ensure the user enters a valid interval. If the user does not want to enter an interval (both inputs are empty/null), that is allowed and the form will submit.
In this screenshot, the required validator is set to "false" however the interval only has a start so the user see's an error message. No interval will be submitted to Knora in this state. As for the Create Resource Form, I would recommend to scroll the user to this component if the user has filled it in only partially even if it is not required. The user should fill in the form correctly by either providing a start and an end or by leaving both inputs empty. The incorrect input will have the |
resolves DSP-1193