-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[DateRangePicker] Allow same date selection #23701
[DateRangePicker] Allow same date selection #23701
Conversation
@material-ui/core: parsed: +Infinity% , gzip: +Infinity% |
1f444e0
to
bb59e1d
Compare
<DesktopDateRangePicker | ||
renderInput={defaultRangeRenderInput} | ||
onChange={() => {}} | ||
value={[ | ||
adapterToUse.date('2018-01-01T00:00:00.000'), | ||
adapterToUse.date('2018-01-01T00:00:00.000'), | ||
]} | ||
/>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was sceptic that we should allow this by default but then realized it is behind a flag (allowSameDateSelection
prop).
However, the test makes it seem like either it is default behavior or allowSameDateSelection
is not working.
If you expect a range as an input that you usually don't expect a single date. I've worked on some internal tools for car-rentals in the past and they had the notion of "minimum range" which would make more sense here. Though it looks like DateRangePicker doesn't implement such a feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe:
allowSameDateSelection
is leaking from another type, it shouldn't end up in the public API- we miss the capability to customize the valid ranges, we started to discuss this in Add ability to select same date in date range picker material-ui-pickers#1759 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be possible with existing API. I haven't looked so far. Still scratching the surface and ironing out kinks from the forceful integration.
@hmaddisb Thanks for spending time on it |
The test is the first one I have written for MUI. The test checks that a range of one day is accepted.
I hope I introduced the new test according to your conventions. Feedback is of course appreciated!
Fixes #23603
Fixes mui/material-ui-pickers#1759