Impl. typecasting for non-display data

[mvorisek]

continue #2168

HTML attributes like data should be formatted "more using machine readable format".

fix #2015

[mkrecek234]

Hi @mvorisek Sorry for not being able to review within the last 2 days - I see an issue already with this, as multiple Dropdowns are rendering values wrongly ignoring fully and custom renderRowFunction. E.g. You have a dropdown with this renderRow function:

        $this->renderRowFunction = function(\Atk4\Filestore\Model\File $row) {

            return [
                'value' => $row->get('token'),
                'title' => $row->get('meta_filename')
            ];
        };

Then, this line will return the idFieldeven though you want value to be rendered individually as being token:

ui/src/Form/Control/Dropdown.php

Line 277 in 21ac4a3

$this->_tItem->set('value', $this->getApp()->uiPersistence->typecastAttributeSaveField($this->model->getField($this->model->idField), $row->getId()));

It has to be $res['value'] and not $row->getId() - otherwise there is no point in returning values in a renderRowFunction anymore. I would prefer to remove this BC break as it is not really needed and limits the freedom of the developer to customize.

[mvorisek]

@mkrecek234 that was introduced in 4b334a3 (#2168) and related #2165

please submit a separate issue for it with a) link to this post, b) minimal self containing repro, c) expected behaviour/fix, and if you can, PR is welcomed.

[mkrecek234]

@mvorisek You certainly had reasons why you wanted to drop the possibility to adjust value which I would object, as there are use cases where it is helpful - so it mainly is to revert your changes with regard to not allowing renderRowFunction for models to set value back to where it was before.

Can we keep it simple and adjust your unreviewed code like this:

        if ($this->model !== null) {
            $res = ($this->renderRowFunction)($row);
            $this->_tItem->set('value', array_key_exists('value', $res) ? $res['value'] : $this->getApp()->uiPersistence->typecastAttributeSaveField($this->model->getField($this->model->idField), $row->getId()));
        } else {
            $res = ($this->renderRowFunction)($row, $key); // @phpstan-ignore-line https://github.com/phpstan/phpstan/issues/10283#issuecomment-1850438891
            $this->_tItem->set('value', (string) $res['value']); // @phpstan-ignore-line https://github.com/phpstan/phpstan/issues/10283
        }

[mvorisek]

Thank you for pointing the issue out. The reason is anything rendered in a custom renderer is wrong as it needs to be processed/parsed when selected - this time, using non-custom parser. So there is more going on and it is your job to do this.

[mkrecek234]

I adjusted my own code writing a custom parser already so if there is no interest by you or others to keep the old, easy functionality and you are not agreeing on the simplified adjustment, I'm fine keeping it as is.

[mvorisek]

Michael, developing a framework is about pursuing robust, logical but easy to use design. If you think there is something to improve, please open a separate issue.