Feature: job id advanced parameter type #1383
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1383 +/- ##
=========================================
+ Coverage 53.7% 53.9% +0.19%
=========================================
Files 1025 1027 +2
Lines 24333 24387 +54
Branches 2800 2804 +4
=========================================
+ Hits 13069 13145 +76
+ Misses 11264 11242 -22
Continue to review full report at Codecov.
|
timotheeguerin
changed the title
| @@ -5,9 +5,6 @@ bl-cloud-file-picker { | |||
| bl-form-field { | |||
| flex: 1; | |||
|
|
|||
| .warning { | |||
There was a problem hiding this comment.
why did you remove this one?
| import { StorageAccountPickerComponent } from "./storage-account-picker"; | ||
|
|
||
| const components = [ | ||
| FileGroupPickerComponent, FileGroupSasComponent, FileGroupsPickerComponent, |
There was a problem hiding this comment.
can you make those 1 per line instead
|
|
||
| import "./job-id.scss"; | ||
|
|
||
| // tslint:disable:no-forward-ref |
There was a problem hiding this comment.
you don't need this one anymore
| { provide: NG_VALUE_ACCESSOR, useExisting: forwardRef(() => JobIdComponent), multi: true }, | ||
| { provide: NG_VALIDATORS, useExisting: forwardRef(() => JobIdComponent), multi: true }, | ||
| ], | ||
| }) |
There was a problem hiding this comment.
can you add changeDetection OnPush, we should be doing this for all the new components we write(And when we edit one try to update it to have onPush)
There was a problem hiding this comment.
Cool, will make sure to add this whenever i am pottering about in the code.
|
|
||
| constructor(private jobService: JobService) { | ||
| this._subscriptions.push(this.value.valueChanges.debounceTime(400).distinctUntilChanged().subscribe((value) => { | ||
| this._checkValid(value); |
There was a problem hiding this comment.
that should probably be a validator so it prevent the form from being submitted
There was a problem hiding this comment.
Yeah i noticed this just before i left last night. Seems that none of the advanced components use validators, so file-in-file-group also passes validation if you add a rubbish filename.
| private _checkValid(value: string) { | ||
| this.jobService.get(value).subscribe({ | ||
| next: (job: any) => { | ||
| this.warning = true; |
There was a problem hiding this comment.
you might want to add changeDetector.markForCheck() here now that it is OnPush or it might not get detected
There was a problem hiding this comment.
Yeah, cheers. This will probably change somewhat when i look to use the validator.
|
I think the issue is that the parameter-input.component.validate() doesnt know how to deal with async validation. it's expecting a normal sync validator. might need some refactoring in here as there are at least 3 advances types that could do with async validation. |
| } else { | ||
| return Observable.of(processErrors(control.errors)); | ||
| } | ||
| } | ||
| } |
Fix: #1330