atk4 / ui

Robust and easy to use PHP Framework for Web Apps
https://atk4-ui.readthedocs.io
MIT License
439 stars 104 forks source link

Dropdown renderRowFunction support for changing values #2172

Closed mkrecek234 closed 4 months ago

mkrecek234 commented 4 months ago

Situation:

Use case: 1) In a text field you want to store a list of comma-separated written-language tags. Before, you could easily implement this like this:

$tagsControl = $form->addControl('tags',
                    [Form\Control\Dropdown::class,  'multiple' => true,
                        'dropdownOptions' => ['allowAdditions' => true, 'forceSelection' => false]
                    ]);
$tagsControl->setModel(new TagsList($model->getPersistence());

                $tagsControl->renderRowFunction = function ($record) {
                    return [
                        'value' => $record->get('tag'),
                        'title' => $record->get('tag')
                    ];
                };

In this case, you might have a TagsList model which shows you tags like Likes food, Is tall, Is married, ... if you then populate the field by selecting some tags, the model would store this as "likes food,is married" - exactly how a lot of other software requires tags list (e.g. Mailchimp)

2) Another use case can be a simple email recipient list which should be stored as "udmxnjs@mail.com,john.miller@gmail.com" - if you have a contact table to select recipients from, you could even make it look nicer, so the dropdown shows the names as titles Karen Smith X John Miller X - this could before easily be implemented by:

                $recipientControl->renderRowFunction = function ($record) {
                    return [
                        'value' => $record->get('email'),
                        'title' => $record->get('name')
                    ];
                };

In latest develop, it is no longer possible to overwrite the returned value in a renderRowFunction, so the above use cases would then always store as lists of model idFields (which often are integers), so the tags list then would store as 1,3 and recipient list as 23,17 for example in the database.

Knowing that the multiple control is developed in a special way still, the general usage and possibilities today are great, flexible and easy-to-code.

If we don't see a good way to keep up the possibility overwriting values for dropdowns with models, for people having the same use cases you can (for example) derive your own dropdown and overwrite the _addRowCallback function (see discussion in https://github.com/atk4/ui/pull/1991#issuecomment-1952477320).

mvorisek commented 4 months ago

The render/parse must play together. Your usecase with multiple is valid and should be supported as 1:N relation somehow.

The parsing of form submit is currently done in https://github.com/atk4/ui/blob/6c609cfa53/src/Form.php#L439 and the type must match the render type, which is not the case with renderRowFunction.

Also Dropdown multiple must send the data from client as json array, as value might contain , (or any other character).

related with #1742

mvorisek commented 4 months ago

Does this mean all Dropdown multiple is broken now? No tests?

mkrecek234 commented 4 months ago

Does this mean all Dropdown multiple is broken now? No tests?

Everything works, no issues - actually I am investigating in these situations that you simply change the idField to the tag's name field for such dropdowns - this should work without any modification of the renderRowFunction as it is now. This is probably the correct way to go. Suggest to close this issues for this reason.

The 1:N generally is indeed the logically correct way, I think though that sometimes for coding-speed you just want a simple text field with comma-separated multiple list entries. And the dropdown field offers this as well easily.

Ideally the multiple dropdown has two ways of working: either creating the comma-(or whatever-)separated items list (or JSON) OR it populates/edits a hasMany child model. The second option would be really great (off-topic here).

mvorisek commented 4 months ago

Let's close it until there is an usecase that someone is willing to pursue.