[pickers] Add `DigitalClock` to `DesktopDateTimePicker` #8946

LukasTy · 2023-05-10T14:08:12Z

Fixes #8819

The design update syncing is pending and will be done after #8851 is merged.

Create dateTimeViewRenderer with two column layout
Use dateTimeViewRenderer for all views in DesktopDateTimePicker in case there are no custom viewRenderers provided
- if there are any custom renderers, fallback to the original renderers, where only date views have entries to benefit from legacy layout
Use updated DateTimePickerToolbar styling in case the new dateTimeViewRenderers are used
Update doc and migration guide
Update tests to work after the changes
Extend existing tests with coverage for multiple views instead of testing only one preferred view
Manual tests
- Test custom viewRenderers behaves as expected
- Test new DesktopDateTimePicker props & slot behavior

mui-bot · 2023-05-10T14:13:41Z

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-8946--material-ui-x.netlify.app/

Updated pages

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	680.1	1,365.7	680.1	974.98	270.683
Sort 100k rows	ms	835.3	1,447.6	1,447.6	1,138.6	214.503
Select 100k rows	ms	227	391.1	312.6	301.34	58.468
Deselect 100k rows	ms	153.5	440.4	196.1	241.78	102.356

Generated by 🚫 dangerJS against 142add2

ArtHell · 2023-05-12T21:01:00Z

Will it be possible to hide digital clock? So that our users will be able to pick date from calendar and put time manually? As it is right now for DesktopDateTimePicker

LukasTy · 2023-05-15T06:09:48Z

Will it be possible to hide digital clock? So that our users will be able to pick date from calendar and put time manually? As it is right now for DesktopDateTimePicker

Yes, you'd get the current behavior back by reverting to the current configuration:

viewRenderers: {
  hours: null,
  minutes: null,
  seconds: null,
  meridiem: null // we'll see if it's possible to avoid the need of specifying this one
}

…derers` for time views

…te-time-picker

github-actions · 2023-05-19T10:46:19Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

LukasTy · 2023-05-19T15:25:30Z

packages/x-date-pickers/src/tests/describeValidation/testMinutesViewValidation.tsx

-            componentsProps: { toolbar: { hidden: true } },
-          }
-        : { slotProps: { toolbar: { hidden: true } } }),
+      slotProps: { toolbar: { hidden: true } },


Just cleaned some old 🗑️

packages/x-date-pickers/src/tests/describeValue/testControlledUnControlled.tsx

flaviendelangle

That's a huge piece of works, congrats ! 🥳

flaviendelangle · 2023-05-22T06:15:02Z

packages/x-date-pickers/src/DesktopDateTimePicker/DesktopDateTimePicker.tsx

+
+  const viewRenderers: PickerViewRendererLookup<TDate | null, DateOrTimeViewWithMeridiem, any, {}> =
+    // we can only ensure the expected two-column layout if none of the renderers are overridden
+    shouldUseNewRenderer


We need a way to at least drop this condition on v7
That's probably part of #8989, how can a renderer (here renderDesktopDateTimeView when used on a date view) be aware of which other views are using it.
In other words, how could renderDesktopDateTimeView adapt when used only for date views, if someone wants to use the TimeClock.

Or do we just say that in v7 people will have to also override the date views to set the renderDateView renderer.

packages/x-date-pickers/src/DesktopDateTimePicker/DesktopDateTimePicker.tsx

flaviendelangle · 2023-05-22T06:17:24Z

packages/x-date-pickers/src/dateTimeViewRenderers/dateTimeViewRenderers.tsx

+  skipDisabled,
+  timeViewsCount,
+}: DateTimeViewRendererProps<TDate>) => {
+  const isActionBarVisible = !!resolveComponentProps(


OK that's super not sexy...
But not sure how to do better

Agreed. 🙈
But given layout, I didn't see other options of how to cleanly avoid having the unwanted divider/border when no actions are present. 🤷

joserodolfofreitas

Thank you for the hard work integrating the digital clock on the pickers, @LukasTy!

A couple of notes:

I noticed when you select the last items on the list, they won't scroll to the top. Is this a technical limitation particular to the DateTime? (It's working on the TimePicker)

Auto-close on Meridiem:
Selecting a Meridiem should not always close the view.
I wonder what we can do here to get a proper auto-closing strategy that considers the selection of the other sections of the date.
Discrepancy between field and view
I noticed that if you add "seconds" to the views property, it displays correctly, but perhaps we should consider reflecting this automatically on the field as well; unless the user provides a specific format.

flaviendelangle · 2023-05-22T12:35:23Z

I noticed that if you add "seconds" to the views property, it displays correctly, but perhaps we should consider reflecting this automatically on the field as well; unless the user provides a specific format.

Right now, the "smart format" is only something applied on the DatePicker
I do agree that the TimePicker and DateTimePicker could also benefit from it.

But note, that if people do strange formats, the UI will not follow the order of the sections present on the format.
To do so, we would need to use the actual parsed format from the fields to build the UI, but I doubt the complexity is worth it.

LukasTy · 2023-05-24T15:30:24Z

Thank you for the feedback @joserodolfofreitas

I noticed when you select the last items on the list, they won't scroll to the top. Is this a technical limitation particular to the DateTime? (It's working on the TimePicker)

Indeed, that is an issue of a naive implementation on the MultiSectionDigitalClock side, which worked fine with a known height but needs to be adjusted to account for the fact that it will be taller in this case. 👌

Auto-close on Meridiem:
Selecting a Meridiem should not always close the view.
I wonder what we can do here to get a proper auto-closing strategy that considers the selection of the other sections of the date.

Well, it's the same logic that we have for the TimePicker, in this case, it's probably a little bit more obvious.
We do close the picker if it's the last view that has been chosen.
How would you change it? Close it only in case when a previous view was not a date view—at least one time section has already been chosen or at least focused/active.
But that would raise another problem—what would happen if you chose meridiem in such a case? Where would the focus jump? To the first time view? 🤔

Discrepancy between field and view
I noticed that if you add "seconds" to the views property, it displays correctly, but perhaps we should consider reflecting this automatically on the field as well; unless the user provides a specific format.

I believe that we've cleared this point during a meeting and confirmed that it is a separate issue that will be tacked via a separate PR. 👌

github-actions · 2023-05-25T15:07:34Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

…te-time-picker

LukasTy · 2023-05-26T12:26:00Z

I noticed when you select the last items on the list, they won't scroll to the top. Is this a technical limitation particular to the DateTime? (It's working on the TimePicker)

Indeed, that is an issue of a naive implementation on the MultiSectionDigitalClock side, which worked fine with a known height but needs to be adjusted to account for the fact that it will be taller in this case. 👌

@joserodolfofreitas I've made some styling adjustments to properly account for the possibility of different container height of the MultiSectionDigitalClock. The scrolling should work as expected in both DatePicker and DateTimePicker cases.
Could you double-check if this issue has been resolved?

I'd be interested in releasing this change without the #8851 because the styling updates are taking longer than expected to correctly implement and finalize.

…te-time-picker

joserodolfofreitas

Considering the scope of integrating the Digital clock on the DesktopDateTimePicker, I believe we're good to merge this one.
We'll need follow-ups to polish:

the auto-closing strategy on both TimePicker and DateTimePicker
the discrepancy between field and picker UI
but those can be easily broken down on different PRs.

Great work bringing the digital clock to the DateTimePicker!

LukasTy added 5 commits May 10, 2023 14:51

Add a combined dateTimeVIewRenderer and adapt typing

89759fb

Avoid type casting

003ce58

Use util

4d5d90a

Avoid passing incorrect focusedView

77d5ad4

Avoid setting view twice and re-setting it in case of DateTimePicker

f3a73a6

LukasTy added component: pickers This is the name of the generic UI component, not the React module! component: DateTimePicker The React component. labels May 10, 2023

LukasTy self-assigned this May 10, 2023

LukasTy added 3 commits May 11, 2023 14:23

Improve typing

914f451

Avoid adding a divider when there are no actions

f34b6d2

Add custom DesktopDateTimePickerToolbar styling and layout

89a0cda

LukasTy added 13 commits May 15, 2023 14:06

Support desktop toolbar in landscape mode

c5f05c0

Specify desktop wrapperVariant

141e619

Keep focus and view in sync.

b872592

Fix border

9be2828

Avoid rendering MultiSectionDigitalClock when there are no `viewRen…

72691df

…derers` for time views

Hiding focus state is evil

db355ec

Fix tests on DesktopDateTimePicker

d3d4b2d

Fix time related tests and use more views in them

36ab70d

proptypes

73f378f

Remove no longer relevant condition

76430b8

Update docs

0a5b99e

docs:api

21e5131

Merge remote-tracking branch 'origin/master' into digital-clock-in-da…

06bcdd8

…te-time-picker

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label May 19, 2023

Use new renderer only if no custom renderers are provided

b308f7d

LukasTy commented May 19, 2023

View reviewed changes

packages/x-date-pickers/src/tests/describeValue/testControlledUnControlled.tsx Show resolved Hide resolved

LukasTy marked this pull request as ready for review May 19, 2023 15:29

LukasTy added the new feature New feature or request label May 19, 2023

LukasTy requested review from joserodolfofreitas, flaviendelangle and alexfauquette May 19, 2023 15:30

flaviendelangle reviewed May 22, 2023

View reviewed changes

Code review: Flavien

632d14d

joserodolfofreitas reviewed May 22, 2023

View reviewed changes

flaviendelangle mentioned this pull request May 23, 2023

[DateTimePicker] "closeOnSelect" can't prevent to close DateTimePicker #9081

Closed

2 tasks

LukasTy mentioned this pull request May 23, 2023

[pickers] Update toolbar spacing and design #9090

Open

Adjust "desktop" toolbar left padding

ddf3736

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label May 25, 2023

LukasTy added 3 commits May 26, 2023 15:06

Calculate the scroll offset container height based on the parent height

b33ca89

Merge remote-tracking branch 'origin/master' into digital-clock-in-da…

139146f

…te-time-picker

Fix proptypes

c7894f2

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label May 26, 2023

LukasTy requested review from joserodolfofreitas and flaviendelangle May 26, 2023 12:26

Merge remote-tracking branch 'origin/master' into digital-clock-in-da…

142add2

…te-time-picker

joserodolfofreitas approved these changes May 26, 2023

View reviewed changes

LukasTy merged commit 030a8d0 into mui:master May 26, 2023
4 checks passed

LukasTy deleted the digital-clock-in-date-time-picker branch May 26, 2023 13:03

LukasTy mentioned this pull request May 30, 2023

[pickers] Replace clock on desktop from TimePicker #4483

Closed

SlowBurner mentioned this pull request Jun 3, 2023

[DateTimePicker] Add Single Column Time Picker to DateTimePicker (dateTimeViewRenderers) #9207

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pickers] Add `DigitalClock` to `DesktopDateTimePicker` #8946

[pickers] Add `DigitalClock` to `DesktopDateTimePicker` #8946

LukasTy commented May 10, 2023 •

edited

Loading

mui-bot commented May 10, 2023 •

edited

Loading

ArtHell commented May 12, 2023

LukasTy commented May 15, 2023

github-actions bot commented May 19, 2023

LukasTy May 19, 2023

flaviendelangle left a comment

flaviendelangle May 22, 2023

flaviendelangle May 22, 2023

LukasTy May 22, 2023

joserodolfofreitas left a comment

flaviendelangle commented May 22, 2023

LukasTy commented May 24, 2023

github-actions bot commented May 25, 2023

LukasTy commented May 26, 2023

joserodolfofreitas left a comment

[pickers] Add DigitalClock to DesktopDateTimePicker #8946

[pickers] Add DigitalClock to DesktopDateTimePicker #8946

Conversation

LukasTy commented May 10, 2023 • edited Loading

mui-bot commented May 10, 2023 • edited Loading

Netlify deploy preview

Updated pages

ArtHell commented May 12, 2023

LukasTy commented May 15, 2023

github-actions bot commented May 19, 2023

LukasTy May 19, 2023

Choose a reason for hiding this comment

flaviendelangle left a comment

Choose a reason for hiding this comment

flaviendelangle May 22, 2023

Choose a reason for hiding this comment

flaviendelangle May 22, 2023

Choose a reason for hiding this comment

LukasTy May 22, 2023

Choose a reason for hiding this comment

joserodolfofreitas left a comment

Choose a reason for hiding this comment

flaviendelangle commented May 22, 2023

LukasTy commented May 24, 2023

github-actions bot commented May 25, 2023

LukasTy commented May 26, 2023

joserodolfofreitas left a comment

Choose a reason for hiding this comment

[pickers] Add `DigitalClock` to `DesktopDateTimePicker` #8946

[pickers] Add `DigitalClock` to `DesktopDateTimePicker` #8946

LukasTy commented May 10, 2023 •

edited

Loading

mui-bot commented May 10, 2023 •

edited

Loading