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

[MonthPicker] New prop shouldDisableMonth #4708

Merged
merged 6 commits into from
May 6, 2022

Conversation

someone635
Copy link
Contributor

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.

@mui-bot
Copy link

mui-bot commented Apr 29, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 279.1 557.1 425.7 401.96 98.254
Sort 100k rows ms 464.7 1,003.3 730 803.24 192.841
Select 100k rows ms 129.5 300 278.3 245.34 62.842
Deselect 100k rows ms 194.4 251.8 201.9 211.46 20.653

Generated by 🚫 dangerJS against ee37213

@someone635 someone635 changed the title Add shouldDisableMonth [MonthPicker] Add shouldDisableMonth Apr 29, 2022
@someone635 someone635 force-pushed the add_shouldDisableMonth branch from 37c0a58 to 73bf514 Compare April 29, 2022 18:04
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.

Thanks for your contribution !
The feature implemented make sense, my comments are just about code organization.

@flaviendelangle flaviendelangle added the component: pickers This is the name of the generic UI component, not the React module! label May 4, 2022
@flaviendelangle flaviendelangle changed the title [MonthPicker] Add shouldDisableMonth [MonthPicker] New prop shouldDisableMonth May 4, 2022
someone635 and others added 2 commits May 5, 2022 08:28
Co-authored-by: Flavien DELANGLE <flaviendelangle@gmail.com>
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label May 5, 2022
@github-actions
Copy link

github-actions bot commented May 5, 2022

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 May 5, 2022
@someone635
Copy link
Contributor Author

Suggested review changes implemented

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.

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>;
Copy link
Member

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

@flaviendelangle flaviendelangle merged commit 45a3b34 into mui:master May 6, 2022
@someone635 someone635 deleted the add_shouldDisableMonth branch May 6, 2022 11:33
alexfauquette pushed a commit to alexfauquette/mui-x that referenced this pull request Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

3 participants