-
-
Notifications
You must be signed in to change notification settings - Fork 192
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
generate the form attributes based on validations #70
base: master
Are you sure you want to change the base?
Conversation
start to generate the form fields based on the validations.
@samdark can we have this for 2.0.7? |
we may even have this for 2.0.5 ;) extensions are now released separately from framework. |
@cebe what do you think on https://github.com/yiisoft/yii2-gii/pull/70/files#diff-4704dd066f5268da66c24cb6ef42b0e3R66 ? Should we make a huge |
echo " <?= " . $generator->generateActiveField($attribute) . " ?>\n\n"; | ||
<?php | ||
$weights = [ | ||
'yii\validators\ExistValidator' => 2, |
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.
Could a simple ordered list instead of weights.
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.
what do you mean?
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.
$priority = [
'yii\validators\ExistValidator',
'yii\validators\ImageValidator',
// ...
];
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.
then we should use something like
$priority = array_flip([
'yii\validators\ExistValidator',
'yii\validators\ImageValidator',
// ...
]);
ok will fix it
How about the line 66 which I think is the really problematic line because its whats missing to make this functional?
@samdark i made a commit with a mock up on how the switch will look like. what do you think about it? and how do you wanna add it to a method in the generator class? |
I think it's OK. A method is a good idea. |
@samdark @cebe I think its ready to get tested http://pastie.org/10669273 here is an example of what I get with one of my most complex models |
'yii\validators\EmailValidator' => 1, | ||
'yii\validators\BooleanValidator' => 2, | ||
'yii\validators\RangeValidator' => 3, | ||
'faryshta\\validators\\EnumValidator' => 4, |
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.
What does it do in the core? :)
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 will take it out then. I was also thinking on a way to do it extendable so plugins could create their generators on gii without making an entire new gii.
any idea?
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.
Make it non-static then and move it to module level.
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 understand this tip. can you explain further?
specially on how to let other modules or libraries define their own associated widgets for the validators they define such as my enum validator
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've suggested taking static $priorities
out from the function to the class level i.e. public $validators
or something like that.
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.
This way it could be configured externally.
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.
Also it doesn't really make sense to specify priorities as integers. Order of values in array should be enough.
@Faryshta is it abandoned? |
@samdark what need for accept PR? Refactoring? |
// code to generate each attribute based on the validators saved sofar. | ||
switch ($validator->className()) { | ||
|
||
case 'yii\validators\NumberValidator': |
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.
Instead of hardcoding we can introduce something like NumberValidatorFieldGenerator
. This way introducing our own generators will be possible. These could be configured via what's called $priorities
now.
@mkiselev refactoring, tests, phpdoc, docs about how to expand this to your own validators. |
with your suggestion of the static property i don't know how to do it myself |
i haven't find the best strategy to generate the active fields yet. @samdark any advice there?