[pickers] Restructure props in `MonthPicker` / `YearPicker` and `DayPicker` #4814

flaviendelangle · 2022-05-09T14:14:31Z

Goal

Have MonthPicker / YearPicker / DayPicker usable in a standalone way. They should not rely on abstractions in CalendarPicker and they should have a coherent API when used alone (no required prop that could be nullable for instance).
Avoid JSDoc duplication for documented props like disablePast, it leads to inconsistent descriptions / default value.

Work

Packages

Create new centralized interfaces DayValidationProps, MonthValidationProps, YearValidationProps to avoid defining props like shouldDisableMonth or disablePast in several interfaces with inconsistent descriptions.
Remove onMonthChange prop from MonthPicker as it is equivalent to onChange and instead wrap the onChange passed to MonthPicker from CalendarPicker to call onMonthChange.
Remove onYearChange prop from YearPicker as it is equivalent to onChange and instead wrap the onChange passed to YearPicker from CalendarPicker to call onYearChange.
When changing year, do not go to the closest non-disabled date in YearPicker but in CalendarPicker because YearPicker should not care about the availability of days inside the year.
When changing month, go to the closest non-disabled date in CalendarPicker (was not done at all)
Remove the isDateDisabled prop from YearPicker and MonthPicker and instead add maxDate / minDate / disableFuture / disablePast to be able to use those components standalone instead of relying on an outside abstraction.
Apply default minDate / maxDate directly in the validation function to avoid missing it in some components

Tests

Write test where we select a non disabled month but the day is disabled (should go to the closest non-disabled date)
Write test with minDate / maxDate in YearPicker and MonthPicker

What's next

Clarify why on CalendarProps we sometime spread other and / or calendarState. It makes it difficult to follow props and I don't think the sub-components have enough props to justify those spreads.
Move the Sub-components section into standalone pages for YearPicker, MonthPicker and DayPicker

Breaking change

The props of MonthPicker / YearPicker and DayPicker have been reworked to make them more consistent for a standalone usage ([pickers] Restructure props in MonthPicker / YearPicker and DayPicker #4814) @flaviendelangle

MonthPicker: The prop onMonthChange has been removed, you can use onChange instead since every change is a month change

YearPicker: The prop onYearPicker has been removed, you can use onChange instead since every change is a year change

DayPicker: The prop isDateDisabled has been removed, you can now use the same validation props as for the other components (maxDate, minDate, shouldDisableDate, disableFuture and disablePast)

mui-bot · 2022-05-09T14:25:31Z

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	261.9	430.6	369.3	344.98	63.179
Sort 100k rows	ms	435.8	873	786.3	700.9	159.236
Select 100k rows	ms	101.1	201.7	173.2	165.96	34.619
Deselect 100k rows	ms	125.3	211.6	192.7	173.88	37.689

Generated by 🚫 dangerJS against 11ed3d9

flaviendelangle · 2022-05-09T14:24:56Z

docs/pages/x/api/date-pickers/mobile-date-picker.json

@@ -20,9 +20,11 @@
    "defaultCalendarMonth": { "type": { "name": "any" } },
    "DialogProps": { "type": { "name": "object" } },
    "disabled": { "type": { "name": "bool" } },
+    "disableFuture": { "type": { "name": "bool" } },


I don't know why those props were not documented, they were already here in TS

flaviendelangle · 2022-05-09T14:40:06Z

packages/x-date-pickers/src/CalendarPicker/useCalendarState.tsx

-      }) !== null,
-    [disableFuture, disablePast, maxDate, minDate, shouldDisableDate, utils],
-  );
+  const isDayDisabled = useIsDayDisabled({


Just a small wrapper to be re-used in DayPicker

flaviendelangle · 2022-05-09T14:48:36Z

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

  const selectedDate = date || now;
  const currentYear = utils.getYear(selectedDate);
  const wrapperVariant = React.useContext(WrapperVariantContext);
  const selectedYearRef = React.useRef<HTMLButtonElement>(null);
  const [focusedYear, setFocusedYear] = React.useState<number | null>(currentYear);

+  const isYearDisabled = React.useCallback(


Copies from #4807

I let you add the "Fix #4806" in PR description, and I close #4807?

github-actions · 2022-05-10T06:38:09Z

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

alexfauquette

It looks good

I had some difficulties with isMonthDisabled because as it is I would imagine it checking "is January disabled?" but instead it checks "is January 1997 disabled?" but I'm not sure we can clarify this point

packages/x-date-pickers-pro/src/DateRangePicker/DateRangePicker.tsx

packages/x-date-pickers/src/internals/hooks/validation/useTimeValidation.ts

packages/x-date-pickers/src/internals/utils/date-utils.ts

flaviendelangle · 2022-05-10T11:52:01Z

I had some difficulties with isMonthDisabled because as it is I would imagine it checking "is January disabled?" but instead it checks "is January 1997 disabled?" but I'm not sure we can clarify this point

I guess you can have use cases where you want to disable January in all years and use cases where you want to disable January just in one year
The good thing with the current isMonthDisabled is that you can do both

github-actions · 2022-05-10T12:21:09Z

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

github-actions · 2022-05-17T11:11:16Z

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

alexfauquette · 2022-05-18T14:12:17Z

Do you need a review or do you have stuff to add in this PR?

flaviendelangle · 2022-05-18T14:20:59Z

I think I'm good for a review 👍

github-actions · 2022-05-20T12:43:17Z

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

alexfauquette · 2022-05-20T13:05:08Z

packages/x-date-pickers/src/internals/hooks/validation/useDateValidation.ts

      return 'maxDate';

    default:
      return null;
  }
 };

+export const useIsDayDisabled = <TDate>({


Should we put in this file all the validations (adding month and year validation)

Such that in this folder we test the edge cases, and in components only do one test to check diasble has been applied to the component

Yes I think it would be a good idea to move all date-related validations in the validation folder
In the future

packages/x-date-pickers/src/MobileDatePicker/MobileDatePicker.test.tsx

alexfauquette

It looks good

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

…icker` (mui#4814)

[pickers] Restructure props in MonthPicker / YearPicker and DayPicker

29a27de

flaviendelangle self-assigned this May 9, 2022

Work

d6a9a48

flaviendelangle commented May 9, 2022

View reviewed changes

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

flaviendelangle commented May 9, 2022

View reviewed changes

Work

bccb96f

flaviendelangle commented May 9, 2022

View reviewed changes

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

Work

8cc0595

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

Work

197d212

alexfauquette reviewed May 10, 2022

View reviewed changes

Work

033b6e6

flaviendelangle marked this pull request as ready for review May 10, 2022 12:18

flaviendelangle changed the title ~~[pickers] Restructure props in MonthPicker / YearPicker and DayPicker~~ [pickers] Restructure props in MonthPicker / YearPicker and DayPicker May 10, 2022

flaviendelangle requested a review from alexfauquette May 10, 2022 12:18

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

Merge

5f4a30b

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

alexfauquette approved these changes May 10, 2022

View reviewed changes

flaviendelangle added the breaking change label May 10, 2022

flaviendelangle added 4 commits May 10, 2022 14:51

Work

c354ea2

Add tests

1f8cc26

Work

a1a9e7d

Merge branch 'master' into view-validation-props

61826f5

Work

5ab4b9d

flaviendelangle mentioned this pull request May 13, 2022

[DateTimePicker] disablePast is not disabling or validating time #4868

Closed

2 tasks

flaviendelangle added 2 commits May 13, 2022 15:56

Merge branch 'master' into view-validation-props

0776069

Work

cd6ac2b

flaviendelangle mentioned this pull request May 17, 2022

[DatePicker] Can't see date view for the month in which all the dates are disabled with shouldDisableDate #4917

Closed

2 tasks

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

flaviendelangle added 2 commits May 17, 2022 14:47

Merge

4e38bd8

Work

bedec13

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

Fix tests

03882c9

alexfauquette mentioned this pull request May 18, 2022

[DatePicker] Fix keyboard accessibility to first/last year #4807

Merged

1 task

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

alexfauquette reviewed May 20, 2022

View reviewed changes

packages/x-date-pickers/src/MobileDatePicker/MobileDatePicker.test.tsx Show resolved Hide resolved

alexfauquette approved these changes May 20, 2022

View reviewed changes

packages/x-date-pickers/src/CalendarPicker/CalendarPicker.tsx Outdated Show resolved Hide resolved

Merge

124cbd7

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

flaviendelangle added 3 commits May 25, 2022 12:54

Code review: Alex

3454a5e

Build api

fe35f90

Work

11ed3d9

flaviendelangle merged commit 36d662e into mui:master May 25, 2022

flaviendelangle deleted the view-validation-props branch May 25, 2022 13:56

flaviendelangle mentioned this pull request Jun 1, 2022

[DateRangePicker] Can't disable dates #5080

Closed

2 tasks

alexfauquette pushed a commit to alexfauquette/mui-x that referenced this pull request Aug 26, 2022

[pickers] Restructure props in MonthPicker / YearPicker and `DayP…

26c5886

…icker` (mui#4814)

flaviendelangle mentioned this pull request Sep 12, 2022

[pickers] Disabled month is selected by default #6121

Closed

flaviendelangle mentioned this pull request Nov 21, 2022

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

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pickers] Restructure props in `MonthPicker` / `YearPicker` and `DayPicker` #4814

[pickers] Restructure props in `MonthPicker` / `YearPicker` and `DayPicker` #4814

flaviendelangle commented May 9, 2022 •

edited

Loading

mui-bot commented May 9, 2022 •

edited

Loading

flaviendelangle May 9, 2022

flaviendelangle May 9, 2022

flaviendelangle May 9, 2022

alexfauquette May 10, 2022

github-actions bot commented May 10, 2022

alexfauquette left a comment

flaviendelangle commented May 10, 2022

github-actions bot commented May 10, 2022

github-actions bot commented May 17, 2022

alexfauquette commented May 18, 2022

flaviendelangle commented May 18, 2022

github-actions bot commented May 20, 2022

alexfauquette May 20, 2022

flaviendelangle May 25, 2022 •

edited

Loading

alexfauquette left a comment

[pickers] Restructure props in MonthPicker / YearPicker and DayPicker #4814

[pickers] Restructure props in MonthPicker / YearPicker and DayPicker #4814

Conversation

flaviendelangle commented May 9, 2022 • edited Loading

Goal

Work

Packages

Tests

What's next

Breaking change

mui-bot commented May 9, 2022 • edited Loading

flaviendelangle May 9, 2022

Choose a reason for hiding this comment

flaviendelangle May 9, 2022

Choose a reason for hiding this comment

flaviendelangle May 9, 2022

Choose a reason for hiding this comment

alexfauquette May 10, 2022

Choose a reason for hiding this comment

github-actions bot commented May 10, 2022

alexfauquette left a comment

Choose a reason for hiding this comment

flaviendelangle commented May 10, 2022

github-actions bot commented May 10, 2022

github-actions bot commented May 17, 2022

alexfauquette commented May 18, 2022

flaviendelangle commented May 18, 2022

github-actions bot commented May 20, 2022

alexfauquette May 20, 2022

Choose a reason for hiding this comment

flaviendelangle May 25, 2022 • edited Loading

Choose a reason for hiding this comment

alexfauquette left a comment

Choose a reason for hiding this comment

[pickers] Restructure props in `MonthPicker` / `YearPicker` and `DayPicker` #4814

[pickers] Restructure props in `MonthPicker` / `YearPicker` and `DayPicker` #4814

flaviendelangle commented May 9, 2022 •

edited

Loading

mui-bot commented May 9, 2022 •

edited

Loading

flaviendelangle May 25, 2022 •

edited

Loading