react-datepicker icon indicating copy to clipboard operation
react-datepicker copied to clipboard

parts object so we can work around the off by one day bug

Open dgreene1 opened this issue 2 years ago • 4 comments

Is your feature request related to a problem? Please describe. Yes, this would give users a high quality mechanism to get around the 1 day off bug: https://github.com/Hacker0x01/react-datepicker/issues/1018

Describe the solution you'd like

We we would add a third parameter to onChange which would enable users to produce a LocalDate (or PlainDate as the Temporal spec calls it) without having to do workarounds such as those described in https://github.com/Hacker0x01/react-datepicker/issues/1018

export interface IDateParts{
    year: number;
    month: number;
    dayOfMonth: number;
    hour: number;
    minutes: number;
    seconds: number;
    /** a string so that you can capture negative numbers */
    offset: string
}

onChange(
        date: WithRange extends false | undefined ? Date | null : [Date | null, Date | null],
        event: React.SyntheticEvent<any> | undefined,
        parts: IDateParts 
    ): void;

For example, you could then use that parts object to do a high quality translation to LocalDate since you'd know that you can disregard hours and minutes:

import { LocalDate } from  "@js-joda/core"

const handleOnChange = (,,parts) => {
  if(parts.hours === null || parts.hours === undefined){
    // We now know that the selection was a day not a date/time
    const myLocalDate= LocalDate.of(parts.year, parts.month, parts.dayOfMonth));
    setDate(myLocalDate);
  }
}

And of course you can still handle moments in time types too:

import { ZonedDateTime } from  "@js-joda/core"

const handleOnChange = (,,parts) => {
  var myZDT = ZonedDateTime.of(
    LocalDate.of(parts.year, parts.month, parts.dayOfMonth),
    LocalTime.of(parts.hour, parts.minutes),
    ZoneId.SYSTEM
  );
  setDate(myZDT);
}

Describe alternatives you've considered There are a great many workarounds listed in https://github.com/Hacker0x01/react-datepicker/issues/1018 and none of them are considered to be universally successful or more superior to another. My approach described in https://dev.to/dgreene1/making-your-datepicker-easier-to-work-with-21n4 gets closer by using domain driven types (like the Temporal spec) to give more clarity. But that approach is also hamstrung by having to work on formatting the ISO string. So the approach above removes the JS Date object from the mix entirely and allows true representation of LocalDate (i.e. a date without time).

Additional context

  • The numbers and strings in the parts object would have to have the same index (i.e. 0 or 1) as the JS date numbers and JsJoda's so we would create strong interop and future proof for Temporal.
  • While I would classify this as a workaround at the moment due to the desire to make a non-breaking change, ideally in the future the onChange method would not return a Date object at all since that is inherently the problem.
  • I'm not sure that offset is something we need to support. Any thoughts? If this was approved I would only add offset if we were able to make sure it was the offset that the picker was using.
  • I wish that we didn't have to support seconds since it’s not available in the calendar area (unless https://github.com/Hacker0x01/react-datepicker/pull/3214 were to be reopened and merged); however, due to the ability to pass in a custom format, the input allows people to type fractions of seconds:

image

dgreene1 avatar Aug 17 '23 20:08 dgreene1

Looks clean. Also keep the ability for developers to use the date library of their choice (luxon, datejs, Day.js etc)

RHeynsZa avatar Aug 23 '23 08:08 RHeynsZa

@martijnrusschen does the proposal look good to you?

dgreene1 avatar Sep 27 '23 15:09 dgreene1

Sorry for the delay. Makes sense to me.

martijnrusschen avatar Oct 09 '23 08:10 martijnrusschen

Thank you for approving the solution. Unfortunately our team has moved into another focus and will be unable to implement this. However, I do feel that this is winning solution to the associate issue which is multiple years old. So I hope that someone will choose to implement it.

dgreene1 avatar Oct 09 '23 12:10 dgreene1

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions[bot] avatar May 30 '24 01:05 github-actions[bot]

I’m requesting that this stay open (ie remove the stale label) since the proposed solution has the potential to close the multi year issue that is plaguing many business.

dgreene1 avatar May 30 '24 02:05 dgreene1