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 timepicker stepMinute should show zero-based multiples of step #2301

Closed
jamesmcglinn opened this issue Sep 19, 2021 · 8 comments · Fixed by #2451
Closed

Calendar timepicker stepMinute should show zero-based multiples of step #2301

jamesmcglinn opened this issue Sep 19, 2021 · 8 comments · Fixed by #2451
Labels
Type: Bug Issue contains a defect related to a specific component.
Milestone

Comments

@jamesmcglinn
Copy link

I'm submitting a ... (check one with "x")

[X] bug report
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://forum.primefaces.org/viewforum.php?f=57

Codesandbox Case (Bug Reports)
Please fork the codesandbox below and create a case demonstrating your bug report. Issues without a codesandbox have much less possibility to be reviewed.

https://codesandbox.io/s/primereact-test-forked-lsctl

Current behavior
When using the Calendar timepicker's stepMinute prop the default value's minutes are the exact current time, regardless of whether the current time minutes are a multiple of stepMinute.

Expected behavior
The default value should have minutes set to a multiple of the stepMinute prop.

This has already been fixed in primefaces and primevue:
primefaces/primefaces#6047
primefaces/primevue#115

I'm happy to create a PR with the same changes if you'd like.

Minimal reproduction of the problem with instructions
https://codesandbox.io/s/primereact-test-forked-lsctl

To reproduce open the codesandbox when the current time isn't on the hour, or 20/40 minutes past the hour. At all other times it will default to the current time, and the increment/decrement minute buttons will add/subtract 20 minutes from the arbitrary current time.

Please tell us about your environment:
https://codesandbox.io/s/primereact-test-forked-lsctl

  • React version:
    17.0.2

  • PrimeReact version:
    6.5.1

  • Browser:
    All

  • Language:
    All

@mertsincan mertsincan added the Resolution: Help Wanted Issue or pull request requires extra help and feedback label Nov 6, 2021
@mertsincan
Copy link
Member

mertsincan commented Nov 6, 2021

Hi @jamesmcglinn,

I'm happy to create a PR with the same changes if you'd like.

I like it ;) Could you please create a PR? Thanks a lot @jamesmcglinn ;)

@melloware
Copy link
Member

PR Submitted

@mertsincan mertsincan added Type: Bug Issue contains a defect related to a specific component. and removed Resolution: Help Wanted Issue or pull request requires extra help and feedback labels Dec 10, 2021
@mertsincan mertsincan modified the milestones: 8.0.0, 7.1.0 Dec 10, 2021
@MNazakov
Copy link

MNazakov commented Dec 9, 2022

Hey, it seems that this is still an issue with primereact (tested on 8.0.0 & 8.7.2). I have yet to test it with the new release.

For now, I handled it by manually updating the view date, but I will be happy to revisit it when I have time and submit a PR if necessary.

Here is a fork from the examples page with stepMinute set to 30 > https://codesandbox.io/s/withered-cdn-gedup5

@melloware
Copy link
Member

@MNazakov it looks like its working to me? It bounced you back and forth between 12:30 and 12:00 ? That is expected for stepMinute={30} in my opinion unless I am missing something?

@MNazakov
Copy link

Hi @melloware, thanks for the reply, as far as I understand from the OP, the issue is that the default value is wrong ( when calendar is shown )

Current behavior
When using the Calendar timepicker's stepMinute prop the default value's minutes are the exact current time, regardless of whether the current time minutes are a multiple of stepMinute.

The problem is the initial value. For example if the current time is 15:53 and stepMinute is set to 30 the initial time shown in the date picker will be 15:53 and will change to 16:00 for up button ( or 15:30 if the user has pressed the down button ) only after the interaction has happened. This means that the user has can submit the time with an invalid value depending on the use-case.

Please let me know if I misunderstood the OP's issue description, or if what I'm saying is not related to it

Thanks!

@melloware
Copy link
Member

ahh yes its not checking the initial minute. I just opened a new ticket: #3770

@MNazakov
Copy link

@melloware thanks! I will be happy to look into it and create a PR as soon as I have some spare time to get familiar with the current implementation.

Cheers!

@melloware
Copy link
Member

I fixed it! It will be in 8.7.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a defect related to a specific component.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants