-
-
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] Rename prop date
into parsedValue
when it can contain a range
#4736
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. |
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
date
I 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
date
prop even when using theDateRangePicker
Things like
date: DateRange<TDate>
(or more suble,date: TValue
which can be a range).Why
parsedValue
?We already have a
props.value
which 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.date
ofusePickerState
intopickerProps.parsedValue
parsedValue
in all the components that are being given thepickerProps
(CalendarOrClockPicker
andDateRangePickerView
)Questions
Should we rename
date
intoparsedValue
in deeper views ? LikeCalendarPicker
orClockPicker
When reaching those components, we know that it is not a range so the
date
naming is not incoherent. With that being said, maybe we should aim for consistency.What's next ?
In
DayPicker
I kept thedate
name even if it can be given a range.That's because I want to do deeper changes to this component to fix #4703.