-
Notifications
You must be signed in to change notification settings - Fork 3.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
Show custom validity message only when field is empty. #13499
Conversation
This allows browser to show still show it's default message for other validation failures like invalid string for email field.
Also changed the context classes to return "notEmpty" message instead of "requirePresence" message as I explained here #13498 (comment). |
The message is meant to be shown for the form input, so the field is presesent hence using "requirePresense" failure message is not logical.
8d967a4
to
5f56611
Compare
src/View/Helper/FormHelper.php
Outdated
@@ -1345,7 +1345,7 @@ protected function _magicOptions(string $fieldName, array $options, bool $allowO | |||
|
|||
if ($this->getConfig('autoSetCustomValidity')) { | |||
$options['oninvalid'] = "this.setCustomValidity(''); " | |||
. "if (!this.validity.valid) this.setCustomValidity('$message')"; | |||
. "if (!this.value) this.setCustomValidity('$message')"; |
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.
Will this still take HTML5 pattern validations into account?
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.
I updated the fiddle. As for the email field, the custom validity message is shown only when field is empty. When input value fails to match the pattern it shows browser's default message.
@@ -53,7 +53,7 @@ public function setUp(): void | |||
public function testGetRequiredMessage() | |||
{ | |||
$validator = new Validator(); | |||
$validator->requirePresence('title', true, 'Don\'t forget a title!'); | |||
$validator->notEmpty('title', 'Don\'t forget a title!'); |
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.
Seems like this entire test could be removed if it is for testing requirePresence() interactions with message generation.
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.
The test is required. Instead of requirePresence() message it does need to check for notEmpty() message. There are no other tests for the same.
This allows browser to still show its default message
for other validation failures like invalid string for email field.