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

[pickers] Add a required prop #7633

Open
2 tasks done
irshadpalayadan opened this issue Dec 15, 2021 · 21 comments
Open
2 tasks done

[pickers] Add a required prop #7633

irshadpalayadan opened this issue Dec 15, 2021 · 21 comments
Assignees
Labels
component: pickers This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature

Comments

@irshadpalayadan
Copy link

irshadpalayadan commented Dec 15, 2021

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

Screenshot 2021-12-15 at 7 28 20 PM

Expected behavior 🤔

Screenshot 2021-12-15 at 7 45 53 PM

Steps to reproduce 🕹

Steps:

Context 🔦

No response

Your environment 🌎

`npx @mui/envinfo`
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.
@irshadpalayadan irshadpalayadan added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 15, 2021
@siriwatknp siriwatknp removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 7, 2022
@siriwatknp
Copy link
Member

Please provide a CodeSandbox so that we can see how you use it, thanks!

@siriwatknp siriwatknp added the status: waiting for author Issue with insufficient information label Jan 7, 2022
@github-actions
Copy link

Since the issue is missing key information, and has been inactive for 7 days, it has been automatically closed.
If you wish to see the issue reopened, please provide the missing information.

@henrymcl
Copy link

henrymcl commented Jan 20, 2023

required is not one of the props in <DatePicker/>

That can be passed separately in InputProps and handled in your own renderInput like this:

import { useCallback, useMemo } from 'react';
import { DatePicker } from '@mui/x-date-pickers';
import { TextField } from '@mui/material';

const renderInput = (params) => {
	return <TextField {...params} required={params.InputProps.required} />;
};
const dayOfWeekFormatter = (day) => day.charAt(1);
const views = ['day', 'month', 'year'];

export default ({ value, key, setValue, name, required, label, ...props }) => {
	const callback = useCallback(() => {
		setValue((x) => ({ ...x, [key]: value }));
	}, [setValue, key]);

	const DialogProps = useMemo(() => ({ 'data-name': name }), [name]);
	const InputProps = useMemo(() => ({ required }), [required]);

	return (
		<DatePicker
			inputFormat="YYYY-MM-DD"
			dayOfWeekFormatter={dayOfWeekFormatter}
			value={value}
			onChange={callback}
			renderInput={renderInput}
			DialogProps={DialogProps}
			PopperProps={DialogProps}
			InputProps={InputProps}
			views={views}
			label={label}
			{...props}
		/>
	);
};

Injecting required field of <DatePicker/> to params for renderInput could be really user-friendly though.

@flaviendelangle flaviendelangle removed their assignment Jan 20, 2023
@flaviendelangle flaviendelangle transferred this issue from mui/material-ui Jan 20, 2023
@flaviendelangle
Copy link
Member

I'm reopening because I do agree we could have a better DX on that one.

@flaviendelangle flaviendelangle changed the title Date picker dosn't have option to provide required label with asterisk symbol. [pickers] Add a required prop Jan 20, 2023
@github-actions
Copy link

Since the issue is missing key information and has been inactive for 7 days, it has been automatically closed. If you wish to see the issue reopened, please provide the missing information.

@flaviendelangle flaviendelangle added component: pickers This is the name of the generic UI component, not the React module! and removed status: waiting for author Issue with insufficient information labels Jan 20, 2023
@garyb01
Copy link

garyb01 commented Jul 21, 2023

image
image

image

Solution:
image

@LukasTy
Copy link
Member

LukasTy commented Jul 24, 2023

Given that we support various validation options, I feel that we are sort of missing one of the most important validations—required.
I imagine that many users might often need a required field and some other validation, like disablePast, and given our current API, in order to achieve that visually and functionality it would require a combination of a lot of props and manual validation of required.
Although, it could raise a concern that currently wasn't a problem—when do we trigger the validation state? If we just simply add a required prop to our current validation pipeline, in this case, such a field would always be in an error state and that might often not be the end result that a user wants. 🤔

For this very reason, it does not reasonable to avoid adding such basic validation controls, because more often than not users might be controlling the error state to achieve their desired UX anyways. 🤔

What do you think @flaviendelangle?

@flaviendelangle
Copy link
Member

If we just simply add a required prop to our current validation pipeline, in this case, such a field would always be in an error state

Could you elaborate here? I'm not following you.

@LukasTy
Copy link
Member

LukasTy commented Jul 24, 2023

If we just simply add a required prop to our current validation pipeline, in this case, such a field would always be in an error state

Could you elaborate here? I'm not following you.

I meant that if we add a required validation and change the validation to use such a prop, then simply rendering such a component would result in it already being in an error state and that would not be what the end user would like. It could be quite common to use a deferred validation and show the error state only after a certain action (i.e. a submit button click). Thus, our solution could end up being very useful for some people, while only a marginal improvement for others.

In essence, adding this prop does sound useful as there does not seem to be any clear drawbacks. 🤔

@flaviendelangle
Copy link
Member

I'm wondering if this prop should be in our validation pipeline, or if it should just pass required={true} to the TextField to use the browser behavior.

image

This seems more consistent with the rest of the MUI stack BUT it will not work correctly with the v7 multi-HTML-input implementation.

@LukasTy
Copy link
Member

LukasTy commented Jul 24, 2023

I'm not sure we can make the native required input validation work, because we always have a value (format) when a field is focused.
IMHO, if we want to add it, it should reside in our pipeline, otherwise, just close this issue. 🤔
Should we move this task to Needs grooming to discuss it further amongst the whole team?

@flaviendelangle
Copy link
Member

Let's discuss it during grooming yes!

@michelengelen
Copy link
Member

Was there any decision on this during the grooming @flaviendelangle ?

@LukasTy
Copy link
Member

LukasTy commented Sep 12, 2023

I believe that we decided to introduce the top-level prop and add this validation in a similar manner that we have the other validation props.
The only new "development" is the discovery of how competitors handle it, as mentioned in the 1st point here: #10253 (comment)
It would be really great if we could wire the native required prop validation behavior to our field components, even though they have the value prop when focused. 🤔

@flaviendelangle
Copy link
Member

It would be really great if we could wire the native required prop validation behavior to our field components

And have it work with our multi-HTML-input structure of v7 😬

@LukasTy LukasTy added the enhancement This is not a bug, nor a new feature label Sep 15, 2023
@gitstart
Copy link
Contributor

@LukasTy we would like to pick this up

@LukasTy
Copy link
Member

LukasTy commented Oct 10, 2023

we would like to pick this up

Thank you for expressing the interest. 🙏
Feel free to work on it.
I've assigned the issue to you. 👌

@rishabmahesh
Copy link

Is there any progress on this issue?

@LukasTy
Copy link
Member

LukasTy commented May 6, 2024

@gitstart Do you have any updates on the progress?
Are you experiencing problems that we could help solve? 🤔
Or should we take over the task if we need it delivered sooner? 👌

@gitstart
Copy link
Contributor

gitstart commented May 6, 2024

Hi @LukasTy, thanks for checking in.
Unfortunately, due to a lot of internal resource reallocation on our end, we have not been able to make much progress on this issue. Please feel free to reassign the issue if it is urgent, otherwise, we will get back to it as soon as we can.
Apologies for the delay.

@LukasTy
Copy link
Member

LukasTy commented May 6, 2024

Please feel free to reassign the issue if it is urgent, otherwise, we will get back to it as soon as we can. Apologies for the delay.

Thank you for the update, no worries. 👌
We'll reassign it if we get enough resources to start work on it as well.

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! enhancement This is not a bug, nor a new feature
Projects
None yet
Development

No branches or pull requests

9 participants