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

hasOne relation should use Lookup field instead of DropDown by default #636

Closed
DarkSide666 opened this issue Jan 18, 2019 · 1 comment · Fixed by #637
Closed

hasOne relation should use Lookup field instead of DropDown by default #636

DarkSide666 opened this issue Jan 18, 2019 · 1 comment · Fixed by #637
Assignees

Comments

@DarkSide666
Copy link
Member

Currently we use \atk4\ui\FormField\DropDown field by default for hasOne fields.
See https://github.com/atk4/ui/blob/develop/src/Form.php#L383 and below.

But most of the time data models contain a lot of data and DropDown field always loads all records from referenced model and renders all its elements. This is very time and resource heavy and in case referenced data model is huge - it can even crash.

// this will by default use \atk4\ui\FormField\DropDown field in forms
// DropDown field loads all its values instantly and renders them, so
// in case of huge data model this will be slow and resource heavy.
$this->hasOne('a_id', new HugeModel());

// To use much nicer and faster alternative use \atk4\ui\FormField\Lookup
// field. It will load only few records and use ajax to load other records
// as soon as you start typing something in lookup field. So this will work
// fast and will not be resource heavy and most likely should be as default
// field for hasOne relation sometime in future.
$this->hasOne('b_id', [new HugeModel(), 'ui'=>['form'=>new \atk4\ui\FormField\Lookup()]]);

In case of big referenced data model we should use Lookup field by default to help developers avoid these issues mentioned above.

So my proposal is to replace this:

        if ($f->type != 'boolean') {
            if ($f->enum) {
                $fallback_seed = ['DropDown', 'values' => array_combine($f->enum, $f->enum)];
            } elseif ($f->values) {
                $fallback_seed = ['DropDown', 'values' => $f->values];
            } elseif (isset($f->reference)) {
                $fallback_seed = ['DropDown', 'model' => $f->reference->refModel()];
            }
        }

with this:

        if ($f->type != 'boolean') {
            if ($f->enum) {
                $fallback_seed = ['DropDown', 'values' => array_combine($f->enum, $f->enum)];
            } elseif ($f->values) {
                $fallback_seed = ['DropDown', 'values' => $f->values];
            } elseif (isset($f->reference)) {
                $fallback_seed = ['Lookup', 'model' => $f->reference->refModel()];
            }
        }

So it will still use DropDown for enum and values because these are _static ones_and most likely not a big data amount, but use Lookup field by default in case of hasOne reference, because models most likely will have a lot of data.

@romaninsh
Copy link
Member

Yes, i wanted to do that once we make sure that Lookup work well. It does. So agree to this.

DarkSide666 added a commit that referenced this issue Jan 18, 2019
DarkSide666 added a commit that referenced this issue Jan 18, 2019
* Use Lookup field for hasOne, fix #636

* add dropdown in demo to be able to compare it with lookup
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

Successfully merging a pull request may close this issue.

3 participants