[docs] Create the doc of the new date pickers #6902

flaviendelangle · 2022-11-17T16:59:40Z

Copy the current state of docs/date/date-pickers/date-picker in docs/date/date-pickers/legacy-date-picker as make this new page accessible
Adapt the examples of docs/date/date-pickers/date-picker to use the new pickers
Create a new Controlled vs uncontrolled value demo for the new date picker (to match the approach of the other input-like component both on X and on the Core)
Remove useless props in the demos to keep them as focused as possible (do not control when not needed, don't set custom views if it has nothing to do with the feature demonstrated etc...)

I will do a follow up with all the other pickers.
You can ignore all the /legacy-pickers files, it's just a copy paste.
The /pickers files are interesting to see the actual diff between the old and the new pickers.

mui-bot · 2022-11-17T17:05:05Z

	Messages
📖	Netlify deploy preview: https://deploy-preview-6902--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	684.2	1,036.1	735.4	839.04	125.543
Sort 100k rows	ms	689.1	1,275.2	689.1	954.06	210.362
Select 100k rows	ms	163.7	343.1	282.3	270.08	58.671
Deselect 100k rows	ms	146.2	298.2	196.5	218.38	54.697

Generated by 🚫 dangerJS against 44e45b0

flaviendelangle · 2022-11-17T17:03:10Z

docs/data/date-pickers/date-picker/HelperText.tsx

-          setValue(newValue);
+        componentsProps={{
+          input: {
+            helperText: 'MM / DD / YYYY',


Here we have a small problem

The format is determined inside useDateField
But useDateField is called with the props resolved from the slots.

The only solution I see to keep this feature possible is to duplicate the format logic (extract https://github.com/mui/mui-x/blob/next/packages/x-date-pickers/src/DateField/useDateField.ts#L25 inside a tiny hook) and call it on DateField as well)
Same for the other fields

If you have another solution, I am interested.

The original example sounds more like a hack than a feature.

Another solution could be to let dev compute the format helper themself:

adapter.getFormatHelperText(adapter.formats.keyboardDate)

What about having both ways of overriding slot props as we have for styleOverrides?

// static input: { helperText: 'MM / DD / YYYY', } // function input: (inputProps) => ({ helperText: inputProps?.placeholder, })

We already callbacks on the slot (when they are built using useSlotProps).
But in the params of your function, you don't have the format yet, because the function is called before useDateField which computes the format.

Thinking about this case a bit more—I don't think that we have any sort of problem here.
Static props can be used to pass static props, but in case something more advanced is needed with access to real/current props—the custom component can be passed down. 👍

I feel like being able to put the format in the helper test is something quite a lot of people want.
And not being able to easily do it (without loosing the automatic format of the pickers / field) is not great.

But we can see if people complain about it, we have solutions if needed

olution could be to let dev compute the format helper themself:

adapter.getFormatHelperText(adapter.formats.keyboardDate)

Sorry @alexfauquette I missed your answer
Sure the way it was done was hacky, and if we want to support it, I'm in favor of passing the format in a format property, not a placeholder property.

The only issue with your proposal is that the user need to know the format.
He would loose the ability to have the picker decide the format based on the locale / the views / the meridiem etc...

Maybe that's not that big of a deal, we can wait for feedbacks.

Maybe that's not that big of a deal, we can wait for feedbacks.

I'd say that having the option to access current localized format in one way or another for different use cases might be the expectation by users—especially ones used to using previously available placeholder prop.
Based on this assumption—it would be worthwhile to think of the easiest way to expose it.
Maybe it could be initialized in the components format props? 🤔 Now it seems to be empty if user did not provide any format...

Maybe it could be initialized in the components format props?

Right now the format is initialized in useDateField (or the equivalent for other fields)
Which is called after the resolution of the slots because it needs the result from this resolution
But we could either

Make the format param mandatory in useDateField and apply its default value inside the component instead of the hook

Apply the default value both in the component and in the hook (more duplication but people using the hook directly could still benefit from it)

flaviendelangle · 2022-11-17T17:05:06Z

docs/data/date-pickers/date-picker/CustomInput.tsx

+  ownerState?: any;
+};
+
+const BrowserInput = (props: BrowserInputProps) => {


I'm in favor of removing this demo in the future, in favor of detailed sections in the Custom sub-components page.

LukasTy

Looks great overall. 👍
I've left a few minor comments.

One question stopping from approving it: How are we planning to approach API pages?
To me it would seem that we should basically do the same thing—keep adding new api pages with every component swap PR and possibly try to adopt similar naming strategy to them 🤔

docs/data/date-pickers/date-picker/CustomDay.tsx

docs/data/date-pickers/date-picker/CustomField.tsx.preview

LukasTy · 2022-11-21T09:24:39Z

docs/data/date-pickers/date-picker/DatePickerValue.tsx

+        <NextDatePicker
+          label="Controlled picker"
+          value={value}
+          onChange={(newValue) => setValue(newValue)}


Unrelated question: Have we considered shortening such simple onChange handlers?

-onChange={(newValue) => setValue(newValue)} +onChange={setValue}

To be honest, we very often recreate a method (same on the grid model setters) and I don't know why
Maybe we are just doing it because we always did it 😆

No problem using the shorter version for me

Maybe for pedagogical reasons. The long version explicitly says to newbies that the onChange is expecting a function which has the new value in argument. Moreover not sure every one is super familiar with setter accepting a function

Moreover not sure every one is super familiar with setter accepting a function

But we are not using this feature here right ?

I do agree that longer form of creating an intermediate arrow function is a bit more explicit.
I was thinking of maybe using this way in case the function has only one argument.
Anyways, maybe we are not the ones who should promote nit-pick optimisations. It's not those arrow function are hard to re-create on every render. 🙈

LukasTy · 2022-11-21T09:36:14Z

docs/data/date-pickers/date-picker/HelperText.tsx

-          setValue(newValue);
+        componentsProps={{
+          input: {
+            helperText: 'MM / DD / YYYY',


What about having both ways of overriding slot props as we have for styleOverrides?

// static input: { helperText: 'MM / DD / YYYY', } // function input: (inputProps) => ({ helperText: inputProps?.placeholder, })

docs/data/date-pickers/date-picker/ResponsiveDatePickers.tsx

docs/data/date-pickers/date-picker/StaticDatePickerDemo.tsx

LukasTy · 2022-11-21T11:27:29Z

docs/data/date-pickers/date-picker/SubComponentsCalendars.tsx

-            maxDate={maxDate}
-            onChange={(newValue) => setValue(newValue)}
-          />
+          <YearCalendar defaultValue={dayjs('2022-04-07')} />


Hm... This one is interesting...
By going the simplicity route we not only loose that sort of nice behaviour of any changes being reflected in the other calendars, but also not having the min and max dates on this calendar causes the calendar to render A LOT of years and also shows a bug—the view is not scrolled into the selected year. 🤔

but also not having the min and max dates on this calendar causes the calendar to render A LOT of years and also shows a bug—the view is not scrolled into the selected year

For me the minDate / maxDate were defined here because in the past they were mandatory (see #4814)
But if we think that they are useful in the demo, I can of course add them back.

For the bug, I thought @alexfauquette fixed it some time ago, but maybe not in this situation.

Yes, it sounds like a bug in the computation of the scroll, because the scroll is done (scrollbar is not at the top) but a bit bellow the expected time (2022) Need to investigate in another PR

docs/data/date-pickers/date-picker/date-picker.md

packages/x-date-pickers/src/internals/hooks/usePicker/usePickerValue.ts

flaviendelangle · 2022-11-21T13:03:08Z

@LukasTy

One question stopping from approving it: How are we planning to approach API pages?

We just have to turn the includeUnstableComponents flag to true on getTypescriptProject
I created it to have the proptypes working but I wait for the doc to be ready before generating those pages 👍

flaviendelangle · 2022-11-21T13:10:45Z

To me it would seem that we should basically do the same thing—keep adding new api pages with every component swap PR and possibly try to adopt similar naming strategy to them thinking

Just, those pages being generated, they will always have the name of their component.
So it will be NextDatePicker etc... for the new and DatePicker etc... for the legacy.
And then when we rename the components they will change.

But we should make sure that the demos (see #6909) are sent to the right page.

LukasTy · 2022-11-21T13:12:54Z

So it will be NextDatePicker etc... for the new and DatePicker etc... for the legacy.
And then when we rename the components they will change.

Yeah, I think that in the alpha/beta phase such naming inconsistency shouldn't be too much of a trouble. 👌

docs/data/date-pickers/date-picker/date-picker.md

LukasTy · 2022-11-21T16:36:35Z

docs/data/date-pickers/date-picker/HelperText.tsx

-          setValue(newValue);
+        componentsProps={{
+          input: {
+            helperText: 'MM / DD / YYYY',


Thinking about this case a bit more—I don't think that we have any sort of problem here.
Static props can be used to pass static props, but in case something more advanced is needed with access to real/current props—the custom component can be passed down. 👍

alexfauquette

That's a solid starting point 👍

alexfauquette · 2022-11-18T15:12:46Z

docs/data/date-pickers/date-picker/HelperText.tsx

-          setValue(newValue);
+        componentsProps={{
+          input: {
+            helperText: 'MM / DD / YYYY',


The original example sounds more like a hack than a feature.

Another solution could be to let dev compute the format helper themself:

adapter.getFormatHelperText(adapter.formats.keyboardDate)

alexfauquette · 2022-11-18T15:14:06Z

docs/data/date-pickers/date-picker/JalaliDatePicker.tsx

-        renderInput={(params) => <TextField {...params} />}
-      />
+    <LocalizationProvider dateAdapter={AdapterDateFnsJalali}>
+      <NextDatePicker defaultValue={new Date(2022, 3, 7)} />


Maybe it will make sens to make this demo RTL when the support will be merged

I would be in favor of removing it once we merge #6869 (and improve the demos their with RTL of course)

alexfauquette · 2022-11-22T08:28:09Z

docs/data/date-pickers/date-picker/DatePickerValue.tsx

+        <NextDatePicker
+          label="Controlled picker"
+          value={value}
+          onChange={(newValue) => setValue(newValue)}


Maybe for pedagogical reasons. The long version explicitly says to newbies that the onChange is expecting a function which has the new value in argument. Moreover not sure every one is super familiar with setter accepting a function

alexfauquette · 2022-11-22T08:35:12Z

docs/data/date-pickers/date-picker/SubComponentsCalendars.tsx

-            maxDate={maxDate}
-            onChange={(newValue) => setValue(newValue)}
-          />
+          <YearCalendar defaultValue={dayjs('2022-04-07')} />


Yes, it sounds like a bug in the computation of the scroll, because the scroll is done (scrollbar is not at the top) but a bit bellow the expected time (2022) Need to investigate in another PR

[docs] Create the doc of the new pickers

5b3371b

flaviendelangle marked this pull request as draft November 17, 2022 16:59

flaviendelangle added the docs Improvements or additions to the documentation label Nov 17, 2022

flaviendelangle self-assigned this Nov 17, 2022

Fix

6011985

flaviendelangle commented Nov 17, 2022

View reviewed changes

flaviendelangle added 2 commits November 18, 2022 07:52

Merge branch 'next' into doc-next-pickers

06ae27b

Fix

db5f1da

flaviendelangle marked this pull request as ready for review November 18, 2022 12:01

flaviendelangle changed the title ~~[docs] Create the doc of the new pickers~~ [docs] Create the doc of the new date pickers Nov 18, 2022

flaviendelangle requested review from LukasTy and alexfauquette November 18, 2022 12:02

flaviendelangle added the component: pickers This is the name of the generic UI component, not the React module! label Nov 18, 2022

LukasTy reviewed Nov 21, 2022

View reviewed changes

Code review: Lukas

a5f03b0

flaviendelangle requested a review from LukasTy November 21, 2022 15:04

LukasTy approved these changes Nov 21, 2022

View reviewed changes

Code review: Lukas

44e45b0

alexfauquette approved these changes Nov 22, 2022

View reviewed changes

flaviendelangle merged commit 9d6bd23 into mui:next Nov 22, 2022

flaviendelangle deleted the doc-next-pickers branch November 22, 2022 09:27

flaviendelangle mentioned this pull request Nov 22, 2022

[docs] Create the doc of the new time, date time and date range pickers #6958

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[docs] Create the doc of the new date pickers #6902

[docs] Create the doc of the new date pickers #6902

flaviendelangle commented Nov 17, 2022 •

edited

mui-bot commented Nov 17, 2022 •

edited

flaviendelangle Nov 17, 2022

alexfauquette Nov 18, 2022

LukasTy Nov 21, 2022

flaviendelangle Nov 21, 2022

LukasTy Nov 21, 2022

flaviendelangle Nov 22, 2022

flaviendelangle Nov 22, 2022 •

edited

LukasTy Nov 22, 2022

flaviendelangle Nov 22, 2022

flaviendelangle Nov 17, 2022

LukasTy left a comment

LukasTy Nov 21, 2022

flaviendelangle Nov 21, 2022

alexfauquette Nov 22, 2022

flaviendelangle Nov 22, 2022

LukasTy Nov 22, 2022 •

edited

LukasTy Nov 21, 2022

LukasTy Nov 21, 2022

flaviendelangle Nov 21, 2022

alexfauquette Nov 22, 2022

flaviendelangle commented Nov 21, 2022

flaviendelangle commented Nov 21, 2022

LukasTy commented Nov 21, 2022

LukasTy Nov 21, 2022

alexfauquette left a comment

alexfauquette Nov 18, 2022

alexfauquette Nov 18, 2022

flaviendelangle Nov 22, 2022

alexfauquette Nov 22, 2022

alexfauquette Nov 22, 2022

[docs] Create the doc of the new date pickers #6902

[docs] Create the doc of the new date pickers #6902

Conversation

flaviendelangle commented Nov 17, 2022 • edited

mui-bot commented Nov 17, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flaviendelangle Nov 22, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukasTy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukasTy Nov 22, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flaviendelangle commented Nov 21, 2022

flaviendelangle commented Nov 21, 2022

LukasTy commented Nov 21, 2022

Choose a reason for hiding this comment

alexfauquette left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flaviendelangle commented Nov 17, 2022 •

edited

mui-bot commented Nov 17, 2022 •

edited

flaviendelangle Nov 22, 2022 •

edited

LukasTy Nov 22, 2022 •

edited