-
Notifications
You must be signed in to change notification settings - Fork 498
fix(core): date time input fixes #11233
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
🧪 E2E Preview environment🔑 Environment Variables for Local TestingThis is the preview URL for the E2E tests: https://e2e-studio-2zvnf95vz.sanity.dev To run the E2E tests locally, you can use the following environment variables, then run 💬 Remember to build the project first with |
📊 Playwright Test ReportThis report contains test results, including videos of failing tests. |
⚡️ Editor Performance ReportUpdated Mon, 24 Nov 2025 15:06:15 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
f39ad54 to
179ca67
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is so good. Especially finally solving the issue that prevents the selected value being persisted on blur. Thanks, Pedro!
I don't predict any issues in switching away from the lazy input myself.
Description
This PR fixes 4 issues found in the date time input.
SAPP-3184 - inconsistent time in time input
If a date is selected on any day in the future, the timestamp shows 00:000Z for the seconds. It looks like this:
"datetime": "2025-10-22T14:10:00.000Z"If a later time is selected on the current day, it appears like this:
"datetime": "2025-10-09T09:20:52.718Z"I found this to be happening because of how we set the initial values for the time input, we use
new Date()which will default to use the current seconds and miliseconds, once set, there is no way to change if it is not exposed as part of the date time format. This change, prevents this from happening and doesn't set seconds and miliseconds in the time input for the initial valuetimeStepwas not respectedfixes sanity-io/ui#2137
fixes #11003
Pass the
timeStepproperty to the time input, we are multiplying it by 60 because our docs indicates that this time is set in minutes, not seconds like the one in the time input. To support existing behaviors we are multiplying that value by 60.This doesn't update the UI that is rendered when the user clicks in the input, but it is respected natively by the browsers when using arrow up and down, and it also validates the value and sets the closes value to the time selected, so, if you have a timeStep of 10 minutes and select
11:14it will set the time to11:10.Example with 15 minutes time step
Screen.Recording.2025-11-24.at.15.08.07.mov
Delayed on change
We were using for the time input a lazy text input, this has the downside that the change is triggered only on blur of the input, which could cause users to lose the values they have updated if they close the calendar before triggering an onBlur action, which is not ideal and produces an unexpected behavior
This PR updates this input to use a normal text input, I think there is no real need to use here a lazy because the changes will not happen frequently, given the user needs to select the time in the input.
Now, each change in the hours - minute selector is propagated immediately
Screen.Recording.2025-11-24.at.15.11.10.mov
Invalid date time value was propagated from the time selector
When selecting a time which was invalid and blurring the input, that value was propagated up and inside releases it made the releases tool crash, this is now fixed by checking for the existence of the value before propagating it up.
Issue example:
Screen.Recording.2025-11-24.at.15.14.15.mov
What to review
Is the change from
LazyTextInputtoTextInputbad?Is there any gotcha we can consider for the miliseconds update? Should we also make the same change in the
set to current timebutton?Testing
Visit the studio in https://test-studio-eoojv5grb.sanity.dev/test/structure/input-standard;datetimeTest;9bc2cd38-496b-4014-befc-c0c9bebbce9b and try the changes in the time inputs components.
Notes for release
Fixes inconsistent second and milliseconds in the time input initial value.
Restore support for time step in the time input
Fixes an issue in where in some cases the releases tool could crash when doing changes in the time input