mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.05k stars 1.25k forks source link

[pickers] DateTimeRangePicker `disableFuture` disables Time in the Past #12048

Closed MarcusKuehn closed 6 months ago

MarcusKuehn commented 7 months ago

Steps to reproduce

Link to live example: https://codesandbox.io/p/sandbox/quizzical-mclaren-2jd2gp

Steps:

  1. Select date in the past.
  2. Try to Select Time in the future.

Current behavior

Not being able to select Time.

Expected behavior

Being able to select all times for days that are in the past.

Context

No response

Your environment

npx @mui/envinfo ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Search keywords: DateTimeRangePicker, disableFuture

Order ID: 85157

LukasTy commented 7 months ago

Hello @MarcusKuehn, thank you for trying out the beta and testing a new component. 🙏 From what I can see, the disablePast seems to be working correctly. The problem is that while no value is selected, the validation assumes the start of the day and even when selecting a day, the value is set to the start of the day. If you select PM when it's past lunch, you'll get the expected validation. You can also check that the behavior is correct by specifying ampm={false}. 👍

You can try setting the referenceDate prop to a specific value (e.g. current time) after lunch to ensure that at least some time options are enabled. You can check the reference date documentation for more information.

Could you please confirm if that clarifies the behavior and resolves your issue? If this is no longer a problem, please feel free to close the issue. 😉

MarcusKuehn commented 7 months ago

Hello @LukasTy, unfortunately that does not solve my issue. Iam unsure whether you could access my CodeSandBox, because maybe it was private. Maybe I need to clarify my Problem with an example.

Assume that today is 15.2.2024 14:00, now i want to select the 1.1.2024 13:00 as the start date. That is possible. But if I want to select the 1.1.2024 16:00, its not possible because the times after 14:00 are disabled. This also applies to the second date selection field.

LukasTy commented 7 months ago

Yes, I wasn't able to view the codesandbox initially. Thank you for confirming the problem and sorry for not spotting it initially. 🙈 🙏 The issue is clear and needs to be fixed. 👌

shaharyar-shamshi commented 6 months ago

@LukasTy I was looking into this issue and I found probable reason for this bug.

https://github.com/mui/mui-x/blob/0cd3b4f63afa8f4e3d4afd016a3e76d5b11d58b5/packages/x-date-pickers/src/DigitalClock/DigitalClock.tsx#L233

over here we are matching the only time of past date with time of current date which obviously will disable the time of the date which is after current time.

proper logic should be

 if (
          disableFuture &&
          utils.getDate(valueToCheck) === utils.getDate(now) &&
          isAfter(valueToCheck, now)
        ) {
          return false;
        }

I am looking forward to solve this issue also this logic needs to implemented some other place as well also we need to consider this for the disablePast prop as well.

if you can assign this to me.

Although There is also alternative to achieve the same effect by using disableIgnoringDatePartForTimeValidation props.

 <DateTimeRangePicker
          ampm={false}
          disableFuture
          views={['day', 'hours']}
          disableIgnoringDatePartForTimeValidation={true}
          viewRenderers={{ hours: renderDigitalClockTimeView }}
          timeSteps={{ minutes: 10 }}
          slots={{ field: SingleInputDateTimeRangeField }}
        />
shaharyar-shamshi commented 6 months ago

@LukasTy if you can kindly check the PR also confirm whether we should proceed to fix this or use disableIgnoringDatePartForTimeValidation prop

LukasTy commented 6 months ago

@shaharyar-shamshi Thank you for being interested in solving this issue! 🙏 Sorry for taking so long to reply, I was busy trying to finalize something else before the release. 🙈

After doing some exploration and discovering https://github.com/mui/mui-x/issues/8520, it does not seem that your proposed solution is the correct one. 🤔 If you don't mind, I'll take care of this one. 😉 At the same time, it feels like we need to add the tests that were omitted with the initial DateTImeRangePicker implementation. 🙈

shaharyar-shamshi commented 6 months ago

@shaharyar-shamshi Thank you for being interested in solving this issue! 🙏 Sorry for taking so long to reply, I was busy trying to finalize something else before the release. 🙈

After doing some exploration and discovering #8520, it does not seem that your proposed solution is the correct one. 🤔 If you don't mind, I'll take care of this one. 😉 At the same time, it feels like we need to add the tests that were omitted with the initial DateTImeRangePicker implementation. 🙈

Ohh no worries I was also assuming that the proposed solution might not cover all cases.Do let me know if you need any help with the test cases I can help in writing the test cases and providing maximum coverage

github-actions[bot] commented 6 months ago

:warning: This issue has been closed. If you have a similar problem, please open a new issue and provide details about your specific problem. If you can provide additional information related to this topic that could help future readers, please feel free to leave a comment.

How did we do @MarcusKuehn? Your experience with our support team matters to us. If you have a moment, please share your thoughts through our brief survey.