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

fix: fix property type 'date' when backend returns an iso string #1700

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

dziraf
Copy link
Contributor

@dziraf dziraf commented Aug 13, 2024

Fixes issue related to #1691

@AshotN this fixes a bug where date values were rendered as NaN-NaN-NaN if backend returned an ISO string instead of a simple date string while having the property's type set to date instead of datetime.

@AshotN
Copy link
Contributor

AshotN commented Aug 13, 2024

Makes sense, I haven't been able to test on my actual project yet. But looking at the changes it looks good to me. Sorry for introducing that issue

@dziraf
Copy link
Contributor Author

dziraf commented Aug 14, 2024

I'll have to give this more thought. If the database/backend stores datetimes using UTC timezone and the user's timezone is i.e. GMT+4, this will convert backend's 2024/08/13 11:00PM to 2024-08-13, while that should be 2024-08-14 in the user's timezone.

@AshotN
Copy link
Contributor

AshotN commented Aug 14, 2024

My thought process was for date's with no time. We should display exactly what the DB is storing, so 2024-01-01 no matter where you are should display 2024-01-01. For editing I can see it either way, if your local timezone should be converted to UTC, or just write it as you input it. The latter is more intuitive at face value. But I'm not sure what people would expect more

@phprelated
Copy link

Hi there,
Upgrading from 7.8.1 to 7.8.12 gave me the NaN-NaN-NaN issue.
Waiting for this fix to be deployed.
Thanks

@dziraf
Copy link
Contributor Author

dziraf commented Sep 18, 2024

I'll merge this for now. We should consider using https://www.npmjs.com/package/@formkit/tempo for dates in the meantime

@dziraf dziraf merged commit de86636 into master Sep 18, 2024
8 checks passed
Copy link

🎉 This PR is included in version 7.8.13 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants