Skip to content
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] Use renderer interceptor on DateTimePicker #12441

Merged

Conversation

LukasTy
Copy link
Member

@LukasTy LukasTy commented Mar 13, 2024

Follow up on the DateTimeRangePicker solution with rendererInterceptor instead of dateTimeViewRenderers.

The BC is two-fold:

  1. We no longer expose dateTimeViewRenderers;
  2. Providing a specific time view renderer (i.e. renderTimeViewClock) will no longer revert back to the v5 (v6) behavior, where only date or time view is shown at one time (changed behavior in 1st demo here)

Visual BC showcase

Before After
Screenshot 2024-03-20 at 13 58 46 Screenshot 2024-03-20 at 13 59 07

Changelog

  • The DesktopDateTimePicker view rendering has been optimized by using the same technique as for DesktopDateTimeRangePicker.
    • The dateTimeViewRenderers have been removed in favor of reusing existing time view renderers (renderTimeViewClock, renderDigitalClockTimeView and renderMultiSectionDigitalClockTimeView) and date view renderer (renderDateViewCalendar).
    • Passing renderTimeViewClock to time view renderers will no longer revert to the old behavior of rendering only date or time view.

@LukasTy LukasTy added breaking change component: pickers This is the name of the generic UI component, not the React module! component: DateTimePicker The React component. v7.x labels Mar 13, 2024
@LukasTy LukasTy self-assigned this Mar 13, 2024
@LukasTy LukasTy added this to the 7.0.0 milestone Mar 13, 2024
@mui-bot
Copy link

mui-bot commented Mar 13, 2024

Copy link
Member

@flaviendelangle flaviendelangle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Probably a nitpick, but the icon to switch time views overlap with the clock

Copy link
Member

@flaviendelangle flaviendelangle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement!
Nothing breaking here?

@LukasTy
Copy link
Member Author

LukasTy commented Mar 20, 2024

Probably a nitpick, but the icon to switch time views overlap with the clock

I've checked and it seems that the explicit width: 'auto' was not needed.
I've removed it to avoid this problem.

Nice improvement!
Nothing breaking here?

  1. The removal of dateTimeViewRenderers
  2. The fact that adding renderTimeViewClock will no longer render DesktopDateTimePicker in a v5 manner (only date or time view is visible at a single point in time)

@flaviendelangle I've updated the PR description, could you check it? 🤔

@flaviendelangle
Copy link
Member

@LukasTy I think this would be more complete:

The dateTimeViewRenderers have been removed in favor of reusing existing timeViewRenderers.

=>

The dateTimeViewRenderers have been removed in favor of reusing existing time view renderers (renderTimeViewClock, renderDigitalClockTimeView and renderMultiSectionDigitalClockTimeView) and date view renderers (renderDateViewCalendar).

And a diff example would probably be a nice addition, both for the changelog and the migration guide

@LukasTy
Copy link
Member Author

LukasTy commented Mar 20, 2024

And a diff example would probably be a nice addition, both for the changelog and the migration guide

@flaviendelangle I've updated the PR description and added a migration guide entry.
Though, I'm not sure what kind of diff we could provide here... 🤷
There is no direct path to replicate the old behavior for DesktopDateTimePicker as it was just a temporary measure to avoid a behavior change in a minor release. 🤔

@flaviendelangle
Copy link
Member

Though, I'm not sure what kind of diff we could provide here... 🤷

True, it's not great to have a BC without a clear diff but maybe it's too much of an headache here

@LukasTy LukasTy merged commit abe28f6 into mui:next Mar 20, 2024
17 checks passed
@LukasTy LukasTy deleted the use-renderer-interceptor-for-date-time-picker branch March 20, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: DateTimePicker The React component. component: pickers This is the name of the generic UI component, not the React module! v7.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants