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] Replace the Picker prefix in the view component by Calendar (eg: MonthPicker => MonthCalendar) #6389

Merged
merged 11 commits into from
Oct 11, 2022

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Oct 5, 2022

Part of #4484

I'll add the detailed breaking changes later.
But here is the list of the renaming:

  • CalendarPicker => DateCalendar (we can discuss naming it simply Calendar if you want, in which case the range version would be RangeCalendar and not DateRangeCalendar which would match Spectrum's naming)
  • CalendarPickerSkeleton => DayCalendarSkeleton
  • MonthPicker => MonthCalendar
  • YearPicker => YearCalendar

Follow up:

  • Rename ClockPicker => Clock

Changelog

Breaking changes

  • The view components allowing to pick a date or parts of a date without an input have been renamed to better fit their usage:

    -<CalendarPicker {...props} />
    +<DateCalendar  {...props} />
    -<DayPicker {...props} />
    +<DayCalendar  {...props} />
    -<CalendarPickerSkeleton {...props} />
    +<DayCalendarSkeleton  {...props} />
    -<MonthPicker {...props} />
    +<MonthCalendar  {...props} />
    -<YearPicker {...props} />
    +<YearCalendar  {...props} />
  • Component names in the theme have changed as well:

    -MuiCalendarPicker: {
    +MuiDateCalendar: {
    -MuiDayPicker: {
    +MuiDayCalendar: {
    -MuiCalendarPickerSkeleton: {
    +MuiDayCalendarSkeleton: {
    -MuiMonthPicker: {
    +MuiMonthCalendar: {
    -MuiYearPicker: {
    +MuiYearCalendar: {

… / YearPicker into DateCalendar / DayPickerCalendar / MonthCalendar / YearCalendar
@flaviendelangle flaviendelangle self-assigned this Oct 5, 2022
@flaviendelangle flaviendelangle added breaking change component: pickers This is the name of the generic UI component, not the React module! labels Oct 5, 2022
@mui-bot
Copy link

mui-bot commented Oct 5, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 479 736.6 608.4 605.82 93.101
Sort 100k rows ms 572.4 971.1 803.9 779.02 149.69
Select 100k rows ms 181.7 244.4 210.6 208.9 22.088
Deselect 100k rows ms 129.7 300 192.2 193.48 58.572

Generated by 🚫 dangerJS against 2720982

@flaviendelangle flaviendelangle changed the title [pickers] Rename CalendarPicker / CalendarPickerSkeleton/ MonthPicker/ YearPicker into DateCalendar / DayPickerCalendar / MonthCalendar / YearCalendar [pickers] Replace the Picker prefix in the view component by Calendar (eg: MonthPicker => MonthCalendar) Oct 5, 2022
@joserodolfofreitas
Copy link
Member

joserodolfofreitas commented Oct 5, 2022

Sounds great! I think we're all solid on the use of Calendar as a prefix.

As for DateCalendar => Calendar.
I think DateCalendar feels more consistent with our naming system (by using Calendar always as a Prefix to identify that's a date input view). Btw, would it make sense if it was named DayCalendar ?

On the other hand, I think RangeCalendar sounds like a better name for its situation.

Another thought regarding this naming: If we're ever to implement a scheduler component, would it use this calendar component (obviously with some customizations) ? or would it have its own implementation?

@flaviendelangle
Copy link
Member Author

flaviendelangle commented Oct 5, 2022

Btw, would it make sense if it was named DayCalendar ?

For me our current naming differentiate the date (day + month + year) and the day
We have a DayCalendar (DayPicker right now) which is private and only allow the selection of the day.
Whereas DateCalendar (CalendarPicker right now) allows the selection of the day, the year and the month

Another thought regarding this naming: If we're ever to implement a scheduler component, would it use this calendar component (obviously with some customizations) ? or would it have its own implementation?

Hard to say, maybe our DayPicker could be flexible enough at some point but I'm not sure.
Some internals would probably be shared though.

@flaviendelangle flaviendelangle marked this pull request as ready for review October 6, 2022 11:35
@flaviendelangle flaviendelangle requested review from alexfauquette, LukasTy and joserodolfofreitas and removed request for alexfauquette October 6, 2022 11:35
@github-actions
Copy link

github-actions bot commented Oct 6, 2022

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

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 6, 2022
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 6, 2022
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 10, 2022
@github-actions
Copy link

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

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 10, 2022
@github-actions
Copy link

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

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 11, 2022
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 11, 2022
@LukasTy LukasTy added the v6.x label Oct 11, 2022
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

Could we also include breaking changes mention in the PR description so it would be picked up by release script? 🤔

@flaviendelangle
Copy link
Member Author

Could we also include breaking changes mention in the PR description so it would be picked up by release script? thinking

Sure, I just waited for a confirmation on the nomenclature, I'll write those

Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

Good job, great result! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: pickers This is the name of the generic UI component, not the React module! v6.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants