Skip to content

Commit

Permalink
Fix missing typecast in DragHandler::onOrder() method (#2167)
Browse files Browse the repository at this point in the history
  • Loading branch information
mvorisek authored Feb 13, 2024
1 parent 6801520 commit 4d77a98
Show file tree
Hide file tree
Showing 21 changed files with 85 additions and 82 deletions.
19 changes: 16 additions & 3 deletions demos/collection/multitable.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,19 @@
$finderClass = AnonymousClassNameCache::get_class(fn () => new class() extends Columns {
public array $route = [];

/**
* @return list<mixed>
*/
private function explodeSelectionValue(string $value): array
{
$res = [];
foreach ($value === '' ? [] : explode(',', $value) as $v) {
$res[] = $this->getApp()->uiPersistence->typecastLoadField($this->model->getField($this->model->idField), $v);
}

return $res;
}

#[\Override]
public function setModel(Model $model, array $route = []): void
{
Expand All @@ -34,9 +47,9 @@ public function setModel(Model $model, array $route = []): void
$table = Table::addTo($this->addColumn(), ['header' => false, 'class.very basic selectable' => true])->setStyle('cursor', 'pointer');
$table->setModel($model, [$model->titleField]);

$selections = explode(',', $this->getApp()->tryGetRequestQueryParam($this->name) ?? '');
$selections = $this->explodeSelectionValue($this->getApp()->tryGetRequestQueryParam($this->name) ?? '');

if ($selections[0]) {
if ($selections !== []) {
$table->js(true)->find('tr[data-id=' . $selections[0] . ']')->addClass('active');
}

Expand Down Expand Up @@ -73,7 +86,7 @@ public function setModel(Model $model, array $route = []): void
$table = Table::addTo($this->addColumn(), ['header' => false, 'class.very basic selectable' => true])->setStyle('cursor', 'pointer');
$table->setModel($pushModel->setLimit(10), [$pushModel->titleField]);

if ($selections) {
if ($selections !== []) {
$table->js(true)->find('tr[data-id=' . $selections[0] . ']')->addClass('active');
}

Expand Down
30 changes: 4 additions & 26 deletions demos/form-control/input2.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@
$group->addControl('line_read', ['readOnly' => true])->set('read only');
$group->addControl('line_disb', ['disabled' => true])->set('disabled');

$group = $form->addGroup('Text Area');
$group->addControl('text_norm', [Form\Control\Textarea::class])->set('editable');
$group->addControl('text_read', [Form\Control\Textarea::class, 'readOnly' => true])->set('read only');
$group->addControl('text_disb', [Form\Control\Textarea::class, 'disabled' => true])->set('disabled');
$group = $form->addGroup('Textarea');
$group->addControl('text_norm', [Form\Control\Textarea::class], ['type' => 'text'])->set("editable\nline2");
$group->addControl('text_read', [Form\Control\Textarea::class, 'readOnly' => true], ['type' => 'text'])->set("read only\nline2");
$group->addControl('text_disb', [Form\Control\Textarea::class, 'disabled' => true], ['type' => 'text'])->set("disabled\nline2");

$group = $form->addGroup('Checkbox');
$group->addControl('c_norm', [Form\Control\Checkbox::class], ['type' => 'boolean'])->set(true);
Expand Down Expand Up @@ -198,25 +198,3 @@
],
]));
$r1->onChange(new JsExpression('console.log(\'radio changed\')'));

Header::addTo($app, ['Line ends of Textarea']);

$form = Form::addTo($app);
$group = $form->addGroup('Without model');
$group->addControl('text_crlf', [Form\Control\Textarea::class])->set("First line\r\nSecond line");
$group->addControl('text_cr', [Form\Control\Textarea::class])->set("First line\rSecond line");
$group->addControl('text_lf', [Form\Control\Textarea::class])->set("First line\nSecond line");

$group = $form->addGroup('With model');
$group->addControl('m_text_crlf', [Form\Control\Textarea::class], ['type' => 'text'])->set("First line\r\nSecond line");
$group->addControl('m_text_cr', [Form\Control\Textarea::class], ['type' => 'text'])->set("First line\rSecond line");
$group->addControl('m_text_lf', [Form\Control\Textarea::class], ['type' => 'text'])->set("First line\nSecond line");

$form->onSubmit(static function (Form $form) {
// check what values are submitted
echo "We're URL encoding submitted values to be able to see what line end is actually submitted.";
foreach ($form->model->get() as $k => $v) {
var_dump([$k => urlencode($v)]);
}
echo 'As you can see - without model it submits CRLF, but with model it will normalize all to LF';
});
12 changes: 6 additions & 6 deletions demos/interactive/jssortable.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@

$sortable = JsSortable::addTo($view, ['container' => 'ul', 'draggable' => 'li', 'dataLabel' => 'name']);

$sortable->onReorder(static function (array $order, string $src, int $pos, int $oldPos) use ($app) {
$sortable->onReorder(static function (array $orderedNames, string $sourceName, int $pos, int $oldPos) use ($app) {
if ($app->tryGetRequestQueryParam('btn')) {
return new JsToast(implode(' - ', $order));
return new JsToast(implode(' - ', $orderedNames));
}

return new JsToast($src . ' moved from position ' . $oldPos . ' to ' . $pos);
});
return new JsToast($sourceName . ' moved from position ' . $oldPos . ' to ' . $pos);
}, $model->getField($model->fieldName()->name));

$button = Button::addTo($app)->set('Get countries order');
$button->on('click', $sortable->jsSendSortOrders(['btn' => '1']));
Expand All @@ -57,6 +57,6 @@
$grid->setModel((new Country($app->db))->setLimit(6));

$dragHandler = $grid->addDragHandler();
$dragHandler->onReorder(static function (array $order) {
return new JsToast('New order: ' . implode(' - ', $order));
$dragHandler->onReorder(static function (array $orderedIds) use ($grid) {
return new JsToast('New order: ' . implode(' - ', array_map(static fn ($id) => $grid->getApp()->uiPersistence->typecastSaveField($grid->model->getField($grid->model->idField), $id), $orderedIds)));
});
6 changes: 3 additions & 3 deletions demos/interactive/loader2.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
$grid->table->onRowClick($countryLoader->jsLoad(['id' => $grid->jsRow()->data('id')]));

$countryLoader->set(static function (Loader $p) {
Form::addTo($p)->setModel(
(new Country($p->getApp()->db))->load($p->getApp()->getRequestQueryParam('id'))
);
$country = new Country($p->getApp()->db);
$id = $p->getApp()->uiPersistence->typecastLoadField($country->getField($country->idField), $p->getApp()->getRequestQueryParam('id'));
Form::addTo($p)->setModel($country->load($id));
});
4 changes: 1 addition & 3 deletions src/App.php
Original file line number Diff line number Diff line change
Expand Up @@ -409,10 +409,8 @@ public function tryGetRequestPostParam(string $key): ?string

/**
* Get the value of a specific POST parameter from the HTTP request.
*
* @return mixed
*/
public function getRequestPostParam(string $key)
public function getRequestPostParam(string $key): string
{
$res = $this->tryGetRequestPostParam($key);
if ($res === null) {
Expand Down
12 changes: 5 additions & 7 deletions src/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,8 @@ class Form extends View
/**
* HTML <form> element, all inner form controls are linked to it on render
* with HTML form="form_id" attribute.
*
* @var View
*/
public $formElement;
public View $formElement;

/** @var Form\Layout A current layout of a form, needed if you call Form->addControl(). */
public $layout;
Expand Down Expand Up @@ -106,10 +104,10 @@ class Form extends View
*/
public $controlDisplaySelector = '.field';

/** @var array Use this apiConfig variable to pass API settings to Fomantic-UI in .api(). */
/** @var array<string, mixed> Use this apiConfig variable to pass API settings to Fomantic-UI in .api(). */
public $apiConfig = [];

/** @var array Use this formConfig variable to pass settings to Fomantic-UI in .from(). */
/** @var array<string, mixed> Use this formConfig variable to pass settings to Fomantic-UI in .from(). */
public $formConfig = [];

// {{{ Base Methods
Expand Down Expand Up @@ -491,7 +489,7 @@ public function fixOwningFormAttrInRenderedHtml(string $html): string
* Set Fomantic-UI Api settings to use with form. A complete list is here:
* https://fomantic-ui.com/behaviors/api.html#/settings .
*
* @param array $config
* @param array<string, mixed> $config
*
* @return $this
*/
Expand All @@ -506,7 +504,7 @@ public function setApiConfig($config)
* Set Fomantic-UI Form settings to use with form. A complete list is here:
* https://fomantic-ui.com/behaviors/form.html#/settings .
*
* @param array $config
* @param array<string, mixed> $config
*
* @return $this
*/
Expand Down
10 changes: 5 additions & 5 deletions src/Form/Control.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@
class Control extends View
{
/** @var Form|null to which this field belongs */
public $form;
public ?View $form = null;

/** @var EntityFieldPair<Model, Field>|null */
public $entityField;
public ?EntityFieldPair $entityField = null;

/** @var string */
public $controlClass = '';
Expand Down Expand Up @@ -70,7 +70,7 @@ protected function init(): void
{
parent::init();

if ($this->form && $this->entityField) {
if ($this->form !== null && $this->entityField !== null) {
if (isset($this->form->controls[$this->entityField->getFieldName()])) {
throw (new Exception('Form field already exists'))
->addMoreInfo('name', $this->entityField->getFieldName());
Expand All @@ -88,7 +88,7 @@ protected function init(): void
#[\Override]
public function set($value = null)
{
if ($this->entityField) {
if ($this->entityField !== null) {
$this->entityField->set($value);
} else {
$this->content = $value;
Expand All @@ -101,7 +101,7 @@ public function set($value = null)
protected function renderView(): void
{
// it only makes sense to have "name" property inside a field if used inside a form
if ($this->form) {
if ($this->form !== null) {
$this->template->trySet('name', $this->shortName);
}

Expand Down
6 changes: 3 additions & 3 deletions src/Form/Control/Checkbox.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ protected function init(): void
parent::init();

// checkboxes are annoying because they don't send value when they are not ticked
if ($this->form) {
if ($this->form !== null) {
$this->form->onHook(Form::HOOK_LOAD_POST, function (Form $form, array &$postRawData) {
if (!isset($postRawData[$this->shortName])) {
$postRawData[$this->shortName] = '0';
Expand All @@ -53,13 +53,13 @@ protected function renderView(): void
$this->template->set('Content', $this->label);
}

if ($this->entityField && !is_bool($this->entityField->get() ?? false)) {
if ($this->entityField !== null && !is_bool($this->entityField->get() ?? false)) {
throw (new Exception('Checkbox form control requires field with boolean type'))
->addMoreInfo('type', $this->entityField->getField()->type)
->addMoreInfo('value', $this->entityField->get());
}

if ($this->entityField ? $this->entityField->get() : $this->content) {
if ($this->entityField !== null ? $this->entityField->get() : $this->content) {
$this->template->dangerouslySetHtml('checked', 'checked="checked"');
}

Expand Down
7 changes: 2 additions & 5 deletions src/Form/Control/Dropdown.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,20 +113,17 @@ public function getValue()
{
// dropdown input tag accepts CSV formatted list of IDs
return $this->entityField !== null
? (is_array($this->entityField->get()) ? implode(', ', $this->entityField->get()) : $this->entityField->get())
? (is_array($this->entityField->get()) ? implode(', ', $this->entityField->get()) : $this->entityField->get()) // TODO is_array() should be replaced with field type condition
: parent::getValue();
}

#[\Override]
public function set($value = null)
{
if ($this->entityField) {
if ($this->entityField !== null) {
if ($this->entityField->getField()->type === 'json' && is_string($value)) {
$value = explode(',', $value);
}
$this->entityField->set($value);

return $this;
}

return parent::set($value);
Expand Down
6 changes: 3 additions & 3 deletions src/Form/Control/Lookup.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class Lookup extends Input
*
* Use this apiConfig variable to pass API settings to Fomantic-UI in .dropdown()
*
* @var array
* @var array<string, mixed>
*/
public $apiConfig = ['cache' => false];

Expand Down Expand Up @@ -334,7 +334,7 @@ protected function applyDependencyConditions(): void
$data = [];
if ($this->getApp()->hasRequestQueryParam('form')) {
parse_str($this->getApp()->getRequestQueryParam('form'), $data);
} elseif ($this->form) {
} elseif ($this->form !== null) {
$data = $this->form->model->get();
} else {
return;
Expand Down Expand Up @@ -386,7 +386,7 @@ protected function renderView(): void

$this->initDropdown($chain);

if ($this->entityField && $this->entityField->get()) {
if ($this->entityField !== null && $this->entityField->get() !== null) {
$idField = $this->idField ?? $this->model->idField;

$this->model = $this->model->loadBy($idField, $this->entityField->get());
Expand Down
2 changes: 1 addition & 1 deletion src/Form/Control/Radio.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ protected function init(): void
parent::init();

// radios are annoying because they don't send value when they are not ticked
if ($this->form) {
if ($this->form !== null) {
$this->form->onHook(Form::HOOK_LOAD_POST, function (Form $form, array &$postRawData) {
if (!isset($postRawData[$this->shortName])) {
$postRawData[$this->shortName] = '';
Expand Down
4 changes: 2 additions & 2 deletions src/Form/Control/ScopeBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ protected function init(): void

$this->scopeBuilderView = View::addTo($this, ['template' => $this->scopeBuilderTemplate]);

if ($this->form) {
if ($this->form !== null) {
$this->form->onHook(Form::HOOK_LOAD_POST, function (Form $form, array &$postRawData) {
$key = $this->entityField->getFieldName();
$postRawData[$key] = $this->queryToScope($this->getApp()->decodeJson($postRawData[$key]));
Expand Down Expand Up @@ -312,7 +312,7 @@ protected function buildQuery(Model $model): void
// this is used when selecting proper operator for the inputType (see self::$operatorsMap)
$inputsMap = array_column($this->rules, 'inputType', 'id');

if ($this->entityField && $this->entityField->get() !== null) {
if ($this->entityField !== null && $this->entityField->get() !== null) {
$scope = $this->entityField->get();
} else {
$scope = $model->scope();
Expand Down
3 changes: 3 additions & 0 deletions src/Js/JsReload.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ class JsReload implements JsExpressionable
/**
* Fomantic-UI api settings.
* ex: ['loadingDuration' => 1000].
*
* @var array<string, mixed>
*/
public array $apiConfig = [];

Expand All @@ -34,6 +36,7 @@ class JsReload implements JsExpressionable

/**
* @param array<string, string|int|JsExpressionable> $args
* @param array<string, mixed> $apiConfig
*/
public function __construct(View $view, array $args = [], JsExpressionable $afterSuccess = null, array $apiConfig = [], bool $includeStorage = false)
{
Expand Down
6 changes: 3 additions & 3 deletions src/JsCallback.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ class JsCallback extends Callback
/** @var string Text to display as a confirmation. Set with setConfirm(..). */
public $confirm;

/** @var array|null Use this apiConfig variable to pass API settings to Fomantic-UI in .api(). */
public $apiConfig;
/** @var array<string, mixed> Use this apiConfig variable to pass API settings to Fomantic-UI in .api(). */
public $apiConfig = [];

/** @var string|null Include web storage data item (key) value to be included in the request. */
public $storeName;
Expand All @@ -41,7 +41,7 @@ public function jsExecute(): JsBlock
'url' => $this->getJsUrl(),
'urlOptions' => $this->args,
'confirm' => $this->confirm,
'apiConfig' => $this->apiConfig,
'apiConfig' => $this->apiConfig !== [] ? $this->apiConfig : null,
'storeName' => $this->storeName,
])]);
}
Expand Down
18 changes: 12 additions & 6 deletions src/JsSortable.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Atk4\Ui;

use Atk4\Data\Field;
use Atk4\Ui\Js\JsChain;
use Atk4\Ui\Js\JsExpressionable;

Expand Down Expand Up @@ -65,17 +66,22 @@ protected function init(): void
/**
* Callback when container has been reorder.
*
* @param \Closure(list<string>, string, int, int): (JsExpressionable|View|string|void) $fx
* @param \Closure(list<mixed>, mixed, int, int): (JsExpressionable|View|string|void) $fx
*/
public function onReorder(\Closure $fx): void
public function onReorder(\Closure $fx, Field $idField): void
{
$this->set(function () use ($fx) {
$sortOrders = explode(',', $this->getApp()->getRequestPostParam('order'));
$source = $this->getApp()->getRequestPostParam('source');
$this->set(function () use ($fx, $idField) {
// TODO comma can be in the order/ID value
$orderedIds = explode(',', $this->getApp()->getRequestPostParam('order'));
$sourceId = $this->getApp()->getRequestPostParam('source');
$newIndex = (int) $this->getApp()->getRequestPostParam('newIndex');
$origIndex = (int) $this->getApp()->getRequestPostParam('origIndex');

return $fx($sortOrders, $source, $newIndex, $origIndex);
$typecastLoadIdFx = fn ($v) => $this->getApp()->uiPersistence->typecastLoadField($idField, $v);
$orderedIds = array_map($typecastLoadIdFx, $orderedIds);
$sourceId = $typecastLoadIdFx($sourceId);

return $fx($orderedIds, $sourceId, $newIndex, $origIndex);
});
}

Expand Down
Loading

0 comments on commit 4d77a98

Please sign in to comment.