Fix timepicker timeformat required #1736
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1736 +/- ##
=======================================
Coverage 99.98% 99.98%
=======================================
Files 410 410
Lines 8538 8542 +4
Branches 1252 1253 +1
=======================================
+ Hits 8537 8541 +4
Misses 1 1
Continue to review full report at Codecov.
|
| @@ -57,7 +57,7 @@ export class SkyTimepickerInputDirective implements | |||
| public skyTimepickerInput: SkyTimepickerComponent; | |||
|
|
|||
| @Input() | |||
| public timeFormat: string; | |||
| public timeFormat: string = 'hh'; | |||
There was a problem hiding this comment.
You could use the following, to avoid duplicating the default value throughout the class:
@Input()
public set timeFormat(value: string) {
this._timeFormat = value;
}
public get timeFormat(): string {
return this._timeFormat || 'hh';
}
There was a problem hiding this comment.
nice! done.
| @@ -79,6 +85,7 @@ export class SkyTimepickerInputDirective implements | |||
|
|
|||
| public ngOnChanges(changes: SimpleChanges) { | |||
| this._validatorChange(); | |||
| this.timeFormat = this.timeFormat; | |||
There was a problem hiding this comment.
@blackbaud-conorwright I don't think this line is needed? It seems redundant.
There was a problem hiding this comment.
Oh yeah haha, I think I originally had something else equal to it, but forgot to clean this up. nice catch
|
@Blackbaud-SteveBrush Can you restart this one? |
|
|
||
| @Input() | ||
| public skyTimepickerInput: SkyTimepickerComponent; | ||
|
|
||
| @Input() | ||
| public timeFormat: string; | ||
| public set timeFormat(value: string) { | ||
| this._timeFormat = value || 'hh'; |
There was a problem hiding this comment.
Would it be possible to move the default value to the get, to make it consistent with other components?
Blackbaud-SteveBrush
dismissed
their stale review
Need to a fix a bug in master first
set the expected default for the optional timeFormat property on timepicker and made it use the default if set to undefined on a change.
Resolves: #846