-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[MonthPicker] New prop shouldDisableMonth
#4708
Conversation
These are the results for the performance tests:
|
37c0a58
to
73bf514
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution !
The feature implemented make sense, my comments are just about code organization.
shouldDisableMonth
Co-authored-by: Flavien DELANGLE <flaviendelangle@gmail.com>
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Suggested review changes implemented |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great !
Thanks for your help, it should be released next week 👍
* @param {TDate} month The new year. | ||
* @returns {void|Promise} - | ||
*/ | ||
onMonthChange?: (month: TDate) => void | Promise<void>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related to this PR but onMonthChange
can return a promise and not onYearChange
, the typing feels wrong.
I will have a look
Reopening here a PR I opened previously on @mui/lab
Took the liberty to add this small functionality, as I needed it in my project. I wonder if there was a rationale behind having shouldDisableYear and not shouldDisableMonth.
Use case: I want users to select editions of a monthly magazine, yet the magazine isn't systematically published every month.
The shouldDisableMonth has exactly the same functionality as shouldDisableYear, it disables the month only on the "month" view. It is still possible to access dates of that month using the "day" view and using arrows, so it may have to be combined with shouldDisableDate for 100% disabling.