-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[pickers] Rename prop date into parsedValue when it can contain a range
#4736
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
Conversation
|
These are the results for the performance tests:
|
For me it makes sense to have two namings, one for values which can either be a range of date or single date. And another naming for when we know what type it is |
|
There is a small nuance, the two criteria is "Components in which the value can be a range OR called with a prop spread from a hook (often usePickerState) on which the value can be a range". And this nuance is ... not great 😆 |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
alexfauquette
left a 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.
It took me some time to understand the nuance. The new naming does not cover the minimal set of variables because of the hook, but at least it covers all the needed ones:
- I see
parsedValue, maybe it can be date or range, maybe it is only a date. If I have a doubt I take care of handling both - I see
dateI know it is only a date, I don't need to take care
Goal
Improve the consistency of the naming.
A lot of our inner components received a
dateprop even when using theDateRangePickerThings like
date: DateRange<TDate>(or more suble,date: TValuewhich can be a range).Why
parsedValue?We already have a
props.valuewhich is drilled done even if the typing does not say somui-x/packages/x-date-pickers/src/MobileDatePicker/MobileDatePicker.tsx
Lines 46 to 47 in 8459a54
This trade-off could be re-evaluated at some point
Work
pickerProps.dateofusePickerStateintopickerProps.parsedValueparsedValuein all the components that are being given thepickerProps(CalendarOrClockPickerandDateRangePickerView)Questions
Should we rename
dateintoparsedValuein deeper views ? LikeCalendarPickerorClockPickerWhen reaching those components, we know that it is not a range so the
datenaming is not incoherent. With that being said, maybe we should aim for consistency.What's next ?
In
DayPickerI kept thedatename even if it can be given a range.That's because I want to do deeper changes to this component to fix #4703.