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

DateInput, TimeInput, and DateTimeInput are inconsistent

Open gauthierm opened this issue 1 year ago • 3 comments

What you were expecting:

These inputs should receive Date or string and publish Date.

What happened instead:

They are inconsistent:

  • DateInput receives string and returns string,
  • TimeInput receives string | Date and returns Date
  • DateTimeInput receives string | Date and returns Date

Steps to reproduce:

Make a form with a DateTimeInput, change your mind after talking with the UX team and then switch to DateInput. Now the published value switched from a Date object to a string and validation and transformation functions may unexpectedly break.

Other information:

The DateInput does actually receive a Date or a string, but the documentation says it receives a string.

The parse field for DateTimeInput and TimeInput defaults to a date parser that produces a date object. For the DateInput, the parse field is empty by default.

  • https://github.com/marmelab/react-admin/blob/master/packages/ra-ui-materialui/src/input/TimeInput.tsx#L58C1-L58C23
  • https://github.com/marmelab/react-admin/blob/master/packages/ra-ui-materialui/src/input/DateTimeInput.tsx#L35
  • https://github.com/marmelab/react-admin/blob/master/packages/ra-ui-materialui/src/input/DateInput.tsx#L45

Environment

  • React-admin version: 4.16.7
  • Last version that did not exhibit the issue (if applicable): n/a
  • React version: n/a
  • Browser: n/Z
  • Stack trace (in case of a JS error): n/a

gauthierm avatar Jan 30 '24 02:01 gauthierm

Thanks for your report.

There are various strategies to store partial dates and time, but none that is completely satisfactory. The current state of date and time inputs in react-admin is the result of a compromise between developer expectations, standards, and ease of use. Consistency between these inputs isn't a goal.

That being said, if the documentation isn't clear about the input and output, it needs fixing. So I'm marking this as a documentation issue. Would you like to open a PR to fix it?

fzaninotto avatar Jan 30 '24 08:01 fzaninotto

Right now, the documentation for DateInput clearly states that the input can either be a Date object or a string. Can this issue be closed?

image

Nela62 avatar Feb 26 '24 18:02 Nela62

@Nela62 You're referring to the MUI variant of DateInput. For the browser-specific variant, at the top of the page, the wording is still not clear enough.

Would you like to work on a PR to fix this?

fzaninotto avatar Feb 27 '24 07:02 fzaninotto