Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calendar problem when using minDate and timeOnly=true #15609

Open
juanguerra97 opened this issue May 17, 2024 · 7 comments
Open

Calendar problem when using minDate and timeOnly=true #15609

juanguerra97 opened this issue May 17, 2024 · 7 comments
Labels
Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible

Comments

@juanguerra97
Copy link

Describe the bug

Hello, I am currently using the calendar component for selecting only the time so I set the property timeOnly=true and hourFormat=24 for 24 hour format. Also I am setting minDate with the current date and time: 7:00 AM (Fri May 17 2024 07:00:00 GMT-0600).

The problem is that when I decrement the hour, after 7:00 the time is reset to 11:00 even though my maxDate is 9 AM(Fri May 17 2024 09:00:00 GMT-0600). The 11 hour is only shown in the panel and not the calendar's input. The correct behavior must be to stay at 7 which is my minDate.

I also tested setting my minDate to 13:00 and the behavior is correct becase when I try to decrement after 13 the hour stays at 13. The problem is when my minDate is 12 or less.

I looked at the code and it seems that the problem is in file /src/app/components/calendar/calendar.ts in the function contrainTime you have the following code:

case isMinDate && minHoursExceeds12 && this.minDate.getHours() === 12 && this.minDate.getHours() > convertedHour: returnTimeTriple[0] = 11; case isMinDate && this.minDate.getHours() === convertedHour && this.minDate.getMinutes() > minute: returnTimeTriple[1] = this.minDate.getMinutes(); case isMinDate && this.minDate.getHours() === convertedHour && this.minDate.getMinutes() === minute && this.minDate.getSeconds() > second: returnTimeTriple[2] = this.minDate.getSeconds(); break; case isMinDate && !minHoursExceeds12 && this.minDate.getHours() - 1 === convertedHour && this.minDate.getHours() > convertedHour: returnTimeTriple[0] = 11; this.pm = true;

Thats the problem because you are setting the hour to 11 instead of this.minDate.getHours() which I think would be correct.

I think thats the solution but I have problems running the code to send a pr.

Environment

I am using angular 17.3.7 and primeng 17.16.1

Reproducer

No response

Angular version

17.3.7

PrimeNG version

17.16.1

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

20.10.0

Browser(s)

Firefox, Chrome, Edge

Steps to reproduce the behavior

I am using the component like this:

<p-calendar [(ngModel)]="solicitud.horaInicio" (onSelect)="cambioEntrada($event)" [minDate]="minimoEntrada" [maxDate]="maximoEntrada" [stepHour]="1" [stepMinute]="15" [readonlyInput]="true" [showIcon]="true" [timeOnly]="true" name="horaEntrada1" hourFormat="24" inputId="horaEntrada1" [style]="{ width: '100%' }" [inputStyle]="{ width: '100%' }"></p-calendar>

The min and max date parameters have the values:

minimoEntrada = Fri May 17 2024 07:00:00 GMT-0600
maximoEntrada = Fri May 17 2024 09:00:00 GMT-0600

Expected behavior

When I set the timeOnly=true and my minDate is 7:00 AM the hour must not decrement less than 7 AM.

@juanguerra97 juanguerra97 added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label May 17, 2024
@RogueTea
Copy link
Contributor

I think another possible solution could be to check the hour format. So the logic could include if hourformat is 12 or 24. I'll have a look into this.

@RogueTea
Copy link
Contributor

Upon reflection I think it also doesn't consider the possibility of a max date/time being set as well.

@RogueTea
Copy link
Contributor

So I think the solution should be something similar to this. I still think it should loop through the hours so id min hour was 7 and user clicks decrement it should go to 9. I think this will help with user accessibility, instead of two clicks of increment to reach 9, it is one click of decrement instead.

// For the case where min date is the same as max date. Should loop through respective hours and minutes case isMinMaxSameDay && this.minDate.getHours() > convertedHour: returnTimeTriple[0] = this.maxDate.getHours(); break; case isMinMaxSameDay && this.maxDate.getHours() < convertedHour: returnTimeTriple[0] = this.minDate.getHours(); break;

I should have a PR up for this later today.

@juanguerra97
Copy link
Author

So I think the solution should be something similar to this. I still think it should loop through the hours so id min hour was 7 and user clicks decrement it should go to 9. I think this will help with user accessibility, instead of two clicks of increment to reach 9, it is one click of decrement instead.

// For the case where min date is the same as max date. Should loop through respective hours and minutes case isMinMaxSameDay && this.minDate.getHours() > convertedHour: returnTimeTriple[0] = this.maxDate.getHours(); break; case isMinMaxSameDay && this.maxDate.getHours() < convertedHour: returnTimeTriple[0] = this.minDate.getHours(); break;

I should have a PR up for this later today.

Hello @RogueTea , thanks for the reply.

I'm not sure if the best way is to reset it to the maxDate because what if the user wants to get to the minimum possible value, if it resets to 9 in this case, then the user would have to click the decrement two times againt to reach 7. Also I am upgrading an application from version 6 of angular and primeng and in older versions it just stays at the minDate so maybe the users are used to the hour not restarting at the maxDate when it reaches the minDate.

@RogueTea
Copy link
Contributor

It would still work the same way but different way around. If you were at 9 you'd Increment 1 and reset to 7. I'd be happy to remove this.

@juanguerra97
Copy link
Author

Yeah I just said it for consistency with older versions. I don't know in which version this was broken but that is how it used to work.

@RogueTea
Copy link
Contributor

It might be worth checking with @cetincakiroglu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible
Projects
None yet
Development

No branches or pull requests

2 participants