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

generate the form attributes based on validations #70

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Faryshta
Copy link
Contributor

i haven't find the best strategy to generate the active fields yet. @samdark any advice there?

start to generate the form fields based on the validations.
@Faryshta
Copy link
Contributor Author

Faryshta commented Aug 6, 2015

@samdark can we have this for 2.0.7?

@cebe
Copy link
Member

cebe commented Aug 6, 2015

we may even have this for 2.0.5 ;) extensions are now released separately from framework.

@cebe cebe added this to the 2.0.5 milestone Aug 6, 2015
@Faryshta
Copy link
Contributor Author

Faryshta commented Aug 6, 2015

@cebe what do you think on https://github.com/yiisoft/yii2-gii/pull/70/files#diff-4704dd066f5268da66c24cb6ef42b0e3R66 ?

Should we make a huge switch with a case for every possible type of validator?

echo " <?= " . $generator->generateActiveField($attribute) . " ?>\n\n";
<?php
$weights = [
'yii\validators\ExistValidator' => 2,
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean?

Copy link
Member

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',
   // ...
];

Copy link
Contributor Author

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?

@Faryshta
Copy link
Contributor Author

Faryshta commented Nov 9, 2015

@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?

@samdark
Copy link
Member

samdark commented Nov 10, 2015

I think it's OK. A method is a good idea.

@Faryshta Faryshta changed the title [incomplete] generate the form attributes based on validations generate the form attributes based on validations Jan 4, 2016
@Faryshta
Copy link
Contributor Author

Faryshta commented Jan 4, 2016

@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,
Copy link
Member

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? :)

Copy link
Contributor Author

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?

Copy link
Member

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.

Copy link
Contributor Author

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

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

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.

@cebe cebe modified the milestones: 2.0.6, 2.0.5 Mar 18, 2016
@samdark
Copy link
Member

samdark commented Feb 2, 2017

@Faryshta is it abandoned?

@samdark samdark modified the milestones: 2.0.7, 2.0.6 Feb 18, 2017
@MaksimKiselev
Copy link
Contributor

@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':
Copy link
Member

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.

@samdark
Copy link
Member

samdark commented Apr 4, 2017

@mkiselev refactoring, tests, phpdoc, docs about how to expand this to your own validators.

@Faryshta
Copy link
Contributor Author

Faryshta commented Apr 4, 2017

how to expand this to your own validators.

with your suggestion of the static property i don't know how to do it myself

@samdark samdark added the type:enhancement Enhancement label Apr 4, 2017
@samdark samdark removed this from the 2.0.7 milestone Apr 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants