Time picker: add hour/minute only mode #4939
Conversation
SAndreeva
added
❌ status: awaiting-test
SAndreeva
added
🛠️ status: in-development
SAndreeva
added
❌ status: awaiting-test
sboykova
added
💥 status: in-test
|
Please, @SAndreeva add this change to the changelog.md |
| let formattedMinute; | ||
| let formattedHour; | ||
| let amPM; | ||
| const amPM = (hour > 11) ? 'PM' : 'AM'; |
There was a problem hiding this comment.
Better replace am/pm with enum values
projects/igniteui-angular/src/lib/time-picker/time-picker.component.ts
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| if (listName.indexOf('minuteList') !== -1 && this.timePicker.ampmList) { | ||
| this.timePicker.ampmList.nativeElement.focus(); |
There was a problem hiding this comment.
duplicate code with the line above (ampmList.nativeElement.focus()) - merge if possible
| if (this.timePicker.minuteList) { | ||
| this.timePicker.minuteList.nativeElement.focus(); | ||
| } else if (this.timePicker.hourList) { | ||
| this.timePicker.hourList.nativeElement.focus(); |
There was a problem hiding this comment.
duplicate code with the line above (...hourList.nativeElement.focus()) - merge if possible
| date.setSeconds(0); | ||
| if (this.selectedAmPm === 'PM' && this.selectedHour !== '12') { | ||
| if (this.showHoursList && this.selectedAmPm === 'PM' && this.selectedHour !== '12') { |
There was a problem hiding this comment.
merge conditions with the below line if possible to avoid duplicating code (date.setHours(date.getHours() + 12))
There was a problem hiding this comment.
I'd rather not, this logic is very fragile.
There was a problem hiding this comment.
You could merge those with || and the result will be the same.
| date.setHours(date.getHours() + 12); | ||
| } | ||
|
|
||
| if (!this.showHoursList && amPM === 'PM' && date.getHours().toString() <= '11') { |
There was a problem hiding this comment.
merge with the above condition to avoid duplicating code (date.setHours(date.getHours() + 12))
There was a problem hiding this comment.
I'd rather not, this logic is very fragile.
There was a problem hiding this comment.
You could merge those with || and the result will be the same.
| date.setSeconds(0); | ||
| if (this.selectedAmPm === 'PM' && this.selectedHour !== '12') { | ||
| if (this.showHoursList && this.selectedAmPm === 'PM' && this.selectedHour !== '12') { |
There was a problem hiding this comment.
You could merge those with || and the result will be the same.
sboykova
added
✅ status: verified
Closes #4679
Additional information (check all that apply):
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes