-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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] Remove TDate
generics in favor of PickerValidDate
direct usage
#15001
base: master
Are you sure you want to change the base?
Conversation
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
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.
This is a massive improvement on the codebase. 💙 😍
Thank you for working on it! 🙏
However, I'm really not fond of the ugly solutions we have to take for docs
. 🙈
Have you tried exploring a solution of localized PickerValidDate
redefining? 🤔
@@ -18,7 +19,7 @@ export default function ReferenceDateExplicitDateTimePicker() { | |||
referenceDate={dayjs('2022-04-17T15:30')} | |||
/> | |||
<Typography> | |||
Stored value: {value == null ? 'null' : value.format()} | |||
Stored value: {value == null ? 'null' : (value as Dayjs).format()} |
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.
Oh yes, this isn't lovely. 🙈
I think that if we are going forward with this approach, we need a way to avoid the need for casting. 🙈
At least in the generated "preview".
Maybe we could by default enforce 'Dayjs' type and keep the option to override the type in those few instances where another adapter is used? 🤔
docs/data/date-pickers/custom-field/BrowserV7SingleInputRangeField.tsx
Outdated
Show resolved
Hide resolved
docs/data/date-pickers/validation/DateValidationShouldDisableDate.tsx
Outdated
Show resolved
Hide resolved
packages/x-date-pickers-pro/src/StaticDateRangePicker/StaticDateRangePicker.test.tsx
Show resolved
Hide resolved
@@ -0,0 +1,6 @@ | |||
import { PickerValidDate } from './pickers'; | |||
|
|||
export type SimpleValue = PickerValidDate | null; |
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.
SimpleValue sounds somewhat strange. 🤷
Have you considered PickerValue
?
export type SimpleValue = PickerValidDate | null; | |
export type PickerValue = PickerValidDate | null; |
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 would like to rename DateRange
in a follow up to have both interfaces being named coherently.
I was going for SimpleValue
/ RangeValue
, but we could go for PickerValue
/ PickerRangeValue
if you prefer.
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.
My vote goes for the PickerValue
and PickerRangeValue
. 👍
WDYT @arthurbalduini?
@@ -113,23 +113,23 @@ export const testTextFieldKeyboardRangeValidation: DescribeRangeValidationTestSu | |||
} | |||
|
|||
act(() => { | |||
setValue(adapterToUse.date(past)); | |||
setValue(past); |
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.
👏 🙈 😆
How would you do that? |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Co-authored-by: Lukas Tyla <[email protected]> Signed-off-by: Flavien DELANGLE <[email protected]>
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
@LukasTy one option would be to add the following code in one of the doc files: declare module '@mui/x-date-pickers/models' {
interface PickerValidDateLookup {
fakeDocAdapter: any;
}
} It would not impact Codesandbox / Stackblitz. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Closes #14796
Side note: all
PickerValidDate | null
passed as a generic will be replaced with atrue
orfalse
onceTValue
is replaced byTIsRange
(phrase 2 of #14823).I will do the migration guide after the phase 2 of #14823 to have a finalized list of the breaking changes on the generics.