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

[date picker] Fix shouldDisableDate in range pickers #5123

Merged
merged 1 commit into from
Jun 8, 2022

Conversation

flaviendelangle
Copy link
Member

Fixes #5111

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@flaviendelangle flaviendelangle added bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module! component: DateRangePicker The React component. labels Jun 5, 2022
@flaviendelangle flaviendelangle self-assigned this Jun 5, 2022
@@ -229,6 +229,7 @@ function DateRangePickerViewRaw<TInputDate, TDate>(
disablePast,
minDate,
maxDate,
shouldDisableDate,
Copy link
Member Author

Choose a reason for hiding this comment

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

This part of the code is very error-prone, maybe we could improve it later

Copy link
Member

Choose a reason for hiding this comment

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

Do you have an idea about how to improve it? I only see two ways:

  • Use providers to avoid having to propagate the information, and forget them somewhere
  • make tests that loop on all the exported components to check we do not break props propagation

Copy link
Member

Choose a reason for hiding this comment

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

Do you have an idea about how to improve it? I only see two ways:

  • Use providers to avoid having to propagate the information, and forget them somewhere
  • make tests that loop on all the exported components to check we do not break props propagation

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not think about it in depth tbh
We could indeed have a conformance test taking a component, 2-3 callbacks to interact with the DOM and which tests all the validation props (it would at least be a subset of our problem)

@mui-bot
Copy link

mui-bot commented Jun 5, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 295.7 593.1 394.6 398.62 107.602
Sort 100k rows ms 482.5 961.7 847.8 778.18 176.646
Select 100k rows ms 132.3 217.9 201.3 181.66 33.804
Deselect 100k rows ms 101.1 269.1 201.2 180.94 63.494

Generated by 🚫 dangerJS against e56313b

@flaviendelangle flaviendelangle changed the title [date picker] Fix shouldDisableDate in range pickers [date picker] Fix shouldDisableDate in range pickers Jun 5, 2022
@flaviendelangle flaviendelangle merged commit c45e95f into mui:master Jun 8, 2022
@flaviendelangle flaviendelangle deleted the disable-date-range branch June 8, 2022 07:14
joserodolfofreitas pushed a commit to joserodolfofreitas/mui-x that referenced this pull request Jun 13, 2022

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
alexfauquette pushed a commit to alexfauquette/mui-x that referenced this pull request Aug 26, 2022

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: DateRangePicker The React component. component: pickers This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DateRangePicker] Disabled dates are still selectable
3 participants