-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: add closure validation rule #6951
Conversation
a133002
to
b954991
Compare
Added the docs. |
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.
Good feat👌
@@ -333,7 +334,7 @@ protected function processRules( | |||
|
|||
// @phpstan-ignore-next-line $error may be set by rule methods. | |||
$this->errors[$field] = $error ?? $this->getErrorMessage( | |||
$rule, | |||
$this->isClosure($rule) ? $i : $rule, |
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 don't find this any easier to read than using instanceof
inline.
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'm not against it. However, this method would make more sense if we used it more than once.
Added the closure parameters. |
be44e9c
to
ca19a98
Compare
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.
Nice improvements.
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 like the new closure method definition, but glad it is optional.
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.
Kenjis code works well.
I don't know why, but I think it should cover feature label
for convenience. I used the following method to translate the field name. But I would have preferred it as a label
.
$validation->setRules(
[
'foo' => [
'required',
static function ($value, $data, &$error, $field) {
if ($value !== 'abc') {
$translateField = lang("Foo.$field");
$error = 'The ' . $translateField . ' value is not "abc"';
return false;
}
return true;
},
],
],
);
24fd69c
to
76c0e24
Compare
76c0e24
to
02038b9
Compare
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.
Thanks @kenjis , great👍.
Description
Checklist: