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

CMS-595: Parse date input on blur #79

Merged
merged 2 commits into from
Jan 16, 2025
Merged

Conversation

duncan-oxd
Copy link
Collaborator

Jira Ticket

CMS-595

Description

The default behavior of the react-datepicker parses dates and handles interaction events pretty well, but it wasn't working because the page was re-rendering every time a date changed. This adds a "localDateRanges" value for each date range, and only updates the data object when you hit enter or blur the input.

I removed the onKeyDown handler because the component more or less does this by itself now.

Along with that, some other changes to make that possible:

  • Tweaks to allow "null" dates if you clear the input
  • normalizeToLocalDate - the opposite of normalizeToUTCDate
  • Swapped the positions of the parameters for onChange to be consistent with the new onDateChange handler

CMS-595: Use local state for date ranges

CMS-595: Update handler functions for consistency

CMS-595: Allow nulls

CMS-595: Remove unused date-fns import

CMS-595: Allow null to pass through

CMS-595: Fix open dates

CMS-595: Remove unused import

CMS-595: Always normalize parsed input

CMS-595: Always store as UTC, parse as local
@duncan-oxd duncan-oxd requested a review from diego-oxd January 9, 2025 17:50
@duncan-oxd duncan-oxd self-assigned this Jan 9, 2025
Copy link
Collaborator

@diego-oxd diego-oxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use the normalizeToLocalDate, there could be inconsistencies because all the date objects are saved in the DB as UTC. It also shows the previous date on some instances (see video)

The idea was to always rely on UTC and disregard the time and timezone since we only care about the date.

We'd only use local timezone for timestamps.

Screen.Recording.2025-01-14.at.2.21.23.PM.mov

@duncan-oxd
Copy link
Collaborator Author

The date picker needs a local date object to work, but a date (ex. Jan 14, 2025) is stored in the DB as Jan 14 UTC time, which would be 5pm on Jan 13th if we parsed it straight up. JS doesn't have a "date" without the time part like Postgres. Ideally we'd have "2025-01-13" instead of "2025-01-15T00:00:00.000-7:00" or "2025-01-15T00:00:00.000Z"

I added the function to "normalize to local time" which effectively just chops off the timezone part (or the whole time part, really) so Jan 13th UTC turns back into Jan 13th local time, still at midnight so nothing gets messed up. I think it should be fine if we use the correct normalize function going in and coming out of the DB.

That YYYY-MM-DD issue you noticed is an annoying quirk in JS I wasn't aware of until I worked on this ticket. The datepicker just parses the input string with new Date(<whatever you type here>) and it uses local time for almost everything, but YYYY-MM-DD it assumes is UTC time, and so it's off by 7 hours.
This really stinks:
image

But this is fine??
image

image

So that bug will always happen with react-datepicker, even in their examples. I made a big ticket on their Github here: Hacker0x01/react-datepicker#5289 , asking for the ability to use a custom date parser so we can specifically parse everything as local time.

I found some other datepickers that don't do that kind of native JS date parsing, but for now I think we should just tell users not to type in that date format. Any other common format should work.

😖 🫠 !

@duncan-oxd duncan-oxd requested a review from diego-oxd January 15, 2025 17:28
@diego-oxd
Copy link
Collaborator

but for now I think we should just tell users not to type in that date format. Any other common format should work.

If Amanda is ok with this, then it's good with me.

@duncan-oxd duncan-oxd merged commit d9e744c into alpha Jan 16, 2025
@duncan-oxd duncan-oxd deleted the CMS-595-parse-on-blur branch January 16, 2025 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants