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

[pickers] DateTimePicker value is of type ChangeEvent<HTMLInputElement> #8923

Closed
2 tasks done
RK21898 opened this issue May 8, 2023 · 7 comments
Closed
2 tasks done
Labels
component: pickers This is the name of the generic UI component, not the React module!

Comments

@RK21898
Copy link

RK21898 commented May 8, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

What you see is what you get so I cannot really provide any reproduction. The closest to reproduction steps that I can offer would be:

  1. npm install @mui/x-date-pickers
  2. npm install moment
  3. Try to assign a Moment to the DateTimePicker value, minTime or maxTime property

Current behavior 😯

Currently when I try to pass a value to any of the DateTimePicker components, whether it is standard, mobile or etc, I receive the following error. This happens on both the value and minTime property as shown below, and I am therefore unable to create a controlled DateTimePicker.

image

Expected behavior 🤔

What I expect that would happen is that it accepts either a Moment object or possibly a string

Context 🔦

I am trying to create a dialog that creates a new object to pass through a data grid and a blob storage respectively, this is however out of scope for the problem at hand, as the issue is solely with the DateTimePicker component.

With the DateTimePicker component I am trying to provide a custom object with two date values via change events.

Your environment 🌎

npx @mui/envinfo
   System:
    OS: Windows 10 10.0.19044
  Binaries:
    Node: 16.14.0 - D:\Program Files\nodejs\node.EXE
    Yarn: Not Found
    npm: 8.3.1 - D:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: Not Found
    Edge: Spartan (44.19041.1266.0), Chromium (113.0.1774.35)
  npmPackages:
    @emotion/react: ^11.10.6 => 11.10.6
    @emotion/styled: ^11.10.6 => 11.10.6
    @mui/base:  5.0.0-alpha.127
    @mui/core-downloads-tracker:  5.12.2
    @mui/icons-material: ^5.11.0 => 5.11.0
    @mui/material: ^5.12.2 => 5.12.2
    @mui/private-theming:  5.12.0
    @mui/styled-engine:  5.12.0
    @mui/system:  5.12.1
    @mui/types:  7.2.4
    @mui/utils:  5.12.0
    @mui/x-data-grid: ^6.3.0 => 6.3.0
    @mui/x-date-pickers: ^6.3.0 => 6.3.0
    @types/react:  18.0.24
    react: ^18.2.0 => 18.2.0
    react-dom: ^18.2.0 => 18.2.0
    typescript:  4.8.4

It says that Chrome is not found, I am however definitely working with Chrome.

Order ID or Support key 💳 (optional)

No response

@RK21898 RK21898 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 8, 2023
@LukasTy
Copy link
Member

LukasTy commented May 8, 2023

Hello, from what I can see given the screenshot, it seems that the culprit might be the incorrect minTime prop. moment.now() returns a number of milliseconds, whereas the component expects a Moment object.
Changing it to moment() seems to avoid TS issues. 🤔

If you are still having issues, could you provide a minimal working reproduction example of your issue?

@LukasTy LukasTy added status: waiting for author Issue with insufficient information component: pickers This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels May 8, 2023
@LukasTy LukasTy changed the title [question] DateTimePicker value is of type ChangeEvent<HTMLInputElement> [pickers] DateTimePicker value is of type ChangeEvent<HTMLInputElement> May 8, 2023
@RK21898
Copy link
Author

RK21898 commented May 8, 2023

@LukasTy thank you for the response however this is not the case. You can see what I mean for yourself in this sandbox. Honestly, I am quite stumbled because it seems like the value of the underlying input is taking the return type of the onChangeHandler function.

@github-actions github-actions bot removed the status: waiting for author Issue with insufficient information label May 8, 2023
@LukasTy
Copy link
Member

LukasTy commented May 9, 2023

What was the reason that you were expecting the onChange callback to have the parameters that you defined in your function?
The callback declaration is detailed on the API page.
I've updated your codesandbox with the mentioned change as well as more correct highlightObject definition and less boilerplate (i.e. there is no need to build a new moment object given already existing moment value object).

@RK21898
Copy link
Author

RK21898 commented May 9, 2023

Thank you for your answer. The reason I expected it to have those parameters is because most of MUI's fields are based on the basic input, so I was pretty confused that it was not applicable here.

I'm not sure how to apply this to my code exactly however as I excluded some properties from the object here as it was for demonstration purposes only. I require the two dates to be strings because they are stored in json in a blob storage.

image

Would a correct solution be to create a custom type and convert it to the necessary json format later on?

@LukasTy
Copy link
Member

LukasTy commented May 9, 2023

Thank you for your answer. The reason I expected it to have those parameters is because most of MUI's fields are based on the basic input, so I was pretty confused that it was not applicable here.

That is a fair assumption. We already had similar considerations to include the event in the onChange callback, but that is a major breaking change and needs to be seriously thought through. 😉

I'm not sure how to apply this to my code exactly however as I excluded some properties from the object here as it was for demonstration purposes only. I require the two dates to be strings because they are stored in json in a blob storage.

Would a correct solution be to create a custom type and convert it to the necessary json format later on?

Firstly, I'd suggest having a bit more complete type for the highlight than any, which is the same as nothing. 🙈
Secondly, consider defaulting to null when there is no date, simply because it is a bit more correct, especially, if you were to pass an empty string to any of the date library constructors - they might behave differently...
IMHO, only a small portion of your issue is the typing, the main change that you need to do is the update the change handler.
After that is done, you need to ensure that the picker is working only with the Moment objects or null values.
Here is a minimal and a bit more elaborate example: https://codesandbox.io/s/moment-usage-parsing-g2e3pb?file=/src/App.tsx

Basically, if you are receiving the object with dates as strings, you need to decide on a place where to transform it into Moment | null type. 😉

@RK21898
Copy link
Author

RK21898 commented May 10, 2023

I should be able to find a solution with this information. I may not be able to implement it immediately however, so I will keep you posted when the time comes and close this issue when I've produced a solution.

@LukasTy
Copy link
Member

LukasTy commented Dec 15, 2023

Hello @RK21898. The issue has been open for half a year without any activity. 🙈
We have a separate issue: #4729 for the introduction of an event in the onChange handler.
If you find it relevant, please feel free to upvote or interact with it in any way.
You can look into the proposed non-breaking solution and if that would be useful. 😉
I'm closing this issue.
Please feel free to re-open it or create a new one if you are still experiencing problems. 👍

@LukasTy LukasTy closed this as not planned Won't fix, can't repro, duplicate, stale Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

2 participants