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

required() doesn't mark an input required #9585

Open
Dreamsorcerer opened this issue Jan 15, 2024 · 6 comments
Open

required() doesn't mark an input required #9585

Dreamsorcerer opened this issue Jan 15, 2024 · 6 comments

Comments

@Dreamsorcerer
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Testing if an input is correctly required with react-testing-library fails:

const id = within(edit).getByLabelText("Id *");
expect(id).toBeRequired();

Describe the solution you'd like
In addition to adding the * to the label, I'd expect the required attribute to be set.

@fzaninotto
Copy link
Member

fzaninotto commented Jan 15, 2024

I can reproduce the problem in the storybook: https://react-admin-storybook.vercel.app/?path=/story/ra-ui-materialui-input-textinput--required

  • The first input isn't required.
  • The second input uses the required attribute and the input is properly required.
  • The third input uses the validate attribute and the input is not required.
image

However, I'm not sure if this qualifies as a bug.

The validate prop uses JS validation, the required attribute uses browser validation. You can choose to use either one, or both.

And you can still add the attribute manually:

<TextInput source="foo" validate={required()} required />

So I'm not sure we want to fix it.

I'll wait for more feedback to move on.

@Dreamsorcerer
Copy link
Contributor Author

If we explicitly don't want browser validation, then it should have aria-required="true", right? Which should then pass the test function.

@fzaninotto
Copy link
Member

I'm marking this as an enhancement (adding aria-required="true" to inputs using a required() validate function). You're welcome to submit a PR against next that implements this.

@geobde
Copy link
Contributor

geobde commented Apr 7, 2024

i would like to open an MR about this but i am not sure for the position, if i should pass the attribute as an inputProp, selectProp etc to each Input Component or somehow to pass directly to useInput hook.

@endrits079
Copy link

Validation doesn't work either on a previously disabled field
For example:

MyInputB is dependent on MyInputA value
and I mark MyInputB disabled in case there is no value for InputA I have required on both fields when I try to submit the form when both inputs are empty it validates only the first input and shows that it is required, now I provide a value for inputA and inputB is no longer disabled, I submit the form it doesn't validate inputB to be required

@Dreamsorcerer
Copy link
Contributor Author

That doesn't sound related to this issue..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants