Skip to content

[pickers] Fire onError for partially filled date - #17514

Draft
flaviendelangle wants to merge 6 commits into
mui:masterfrom
flaviendelangle:partially-filled-date
Draft

[pickers] Fire onError for partially filled date#17514
flaviendelangle wants to merge 6 commits into
mui:masterfrom
flaviendelangle:partially-filled-date

Conversation

@flaviendelangle

Copy link
Copy Markdown
Member

Proposal to solve #17499

Question: should we show the field in error state for partially filled date (like in v7)? If so, then this PR is fine. Otherwise, we might want a different approach (maybe the behavior should be configurable through a prop).

@flaviendelangle flaviendelangle added type: regression A bug, but worse, it used to behave as expected. scope: pickers Changes related to the date/time pickers. labels Apr 23, 2025
@flaviendelangle flaviendelangle self-assigned this Apr 23, 2025
@mui-bot

mui-bot commented Apr 23, 2025

Copy link
Copy Markdown

Deploy preview: https://deploy-preview-17514--material-ui-x.netlify.app/

Bundle size report

Bundle size will be reported once CircleCI build #558866 finishes.

Generated by 🚫 dangerJS against 2a85ae9

@flaviendelangle flaviendelangle changed the title [pickers] Fire onError for partially filled date [pickers] Fire onError for partially filled date Apr 23, 2025

@LukasTy LukasTy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice idea! 👍
I like the idea, I think it should be a good compromise between the capabilities and compromises we had on v7. 👌

We are missing the changes on Picker level, Pro package and some tests for this.

@flaviendelangle

Copy link
Copy Markdown
Member Author

We are missing the changes on Picker level, Pro package and some tests for this.

The pro validation (date range, date time range and time range) always use date validation and time validation.
I need to check the behavior on the picker, but the validation value from the field should be correctly forwarded.
And we need test of course 👌

@LukasTy

LukasTy commented Apr 23, 2025

Copy link
Copy Markdown
Member

I need to check the behavior on the picker, but the validation value from the field should be correctly forwarded.

I've tested and it seemed that Picker did not trigger this error, same for the range Pickers and Fields. 🤔

@flaviendelangle

Copy link
Copy Markdown
Member Author

I'll have a look

[fieldValueManager, state.sections],
);

React.useEffect(() => {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

TODO: Add support for multi input range fields

@github-actions github-actions Bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jun 24, 2025
@github-actions

Copy link
Copy Markdown
Contributor

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

@morzel85

morzel85 commented Jul 18, 2025

Copy link
Copy Markdown

Yes, please provide convenient way to detect that input is invalid (incomplete, partial).
As of now, just having null instead of Invalid Date is a dangerous regression. Users can easily miss part of a placeholder still being visible and progress through form assuming that value was provided (validations will not block progress if field was not mandatory).

We will need a way to determine if current value is null (missing) or invalid, including a scenario when it was invalid and then the value was cleared. Maybe the most recent error status should also be passed as additional argument to change handler?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: out-of-date The pull request has merge conflicts and can't be merged. scope: pickers Changes related to the date/time pickers. type: regression A bug, but worse, it used to behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants