Skip to content

#347 separate input name generator method from error field name gener… #357

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

#347 separate input name generator method from error field name gener… #357

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2e1bab3
#347 separate input name generator method from error field name gener…
olegbaturin Aug 25, 2021
451386b
update condition for error field name
olegbaturin Aug 25, 2021
71e4c3f
fix case with no columns
olegbaturin Aug 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions src/MultipleInputColumn.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,10 @@ public function getElementName($index, $withPrefix = true)
$index = '{' . $this->renderer->getIndexPlaceholder() . '}';
}

$elementName = $this->isRendererHasOneColumn()
? '[' . $this->name . '][' . $index . ']'
: '[' . $index . '][' . $this->name . ']';

if (!$withPrefix) {
return $elementName;
if ($this->isRendererHasOneColumn() && $this->hasModelAttribute($this->name)) {
Copy link
Owner

@unclead unclead Aug 26, 2021

Choose a reason for hiding this comment

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

см. аналогичный комментарий ниже

$elementName = '[' . $index . ']';
} else {
$elementName = '[' . $index . '][' . $this->name . ']';
}

$prefix = $this->getInputNamePrefix();
Expand Down Expand Up @@ -87,7 +85,7 @@ protected function getInputNamePrefix()
$model = $this->context->model;
if ($model instanceof Model) {
if (empty($this->renderer->columns) || ($this->isRendererHasOneColumn() && $this->hasModelAttribute($this->name))) {
return $model->formName();
return Html::getInputName($this->context->model, $this->name);
}

return Html::getInputName($this->context->model, $this->context->attribute);
Expand Down Expand Up @@ -125,10 +123,10 @@ public function getFirstError($index)
return null;
}

if ($this->isRendererHasOneColumn()) {
if ($this->isRendererHasOneColumn() && $this->hasModelAttribute($this->name)) {
Copy link
Owner

@unclead unclead Aug 26, 2021

Choose a reason for hiding this comment

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

я не очень понимаю зачем тут проверка на hasModelAttribute, ведь я могу использовать одну колонку для поля которого нет в модели и в этом случае тоже должен быть одноколоночный формат имени

Copy link
Author

@olegbaturin olegbaturin Aug 26, 2021

Choose a reason for hiding this comment

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

Одна колонка может быть двух видов. Как самостоятельный атрибут и как колонка у атрибута:

  1. Как атрибут класса - для этого делается проверка наличия атрибута у класса. В этом случае ошибку будем вешать в объекте класса модели так $this->addError('field[1]', 'Ошибка у field №1')
<?= $form->field($model, 'field')->widget(MultipleInput::class, [
    'allowEmptyList' => true,
    'addButtonPosition' => MultipleInput::POS_FOOTER,
])->label(false); ?>

Результат длжен быть <input name="field[0]"> или <input name="ClassName[field][0]">. Т.е. как обычный атрибут, только в виде массива.

Такой же рузультат будет и при условии, что в опциях задана только одна колонка.

<?= $form->field($model, 'filter')->widget(MultipleInput::class, [
    'allowEmptyList' => true,
    'addButtonPosition' => MultipleInput::POS_FOOTER,
    'columns' => [
        [
            'name'  => 'field',
            'enableError' => true,
        ],
    ],
])->label(false); ?>

Но тут я считаю, что надо добавить еще проверку на совпадение $this->context->attribute и $this->name, иначе теряется значение атрибута 'filter' в данном примере.

  1. Как обычное поле у корневого атрибута.
    input name такое же, как и при нескольких колонках <input name="filter[0][from]"> или <input name="ClassName[filter][0][from]">. Ошибку будем вешать так $this->addError('filter[0][field]', 'Ошибка у filter №1, поле field').

я могу использовать одну колонку для поля которого нет в модели и в этом случае тоже должен быть одноколоночный формат имени

Если у класса нет атрибута, то будет исключение при присвоении ему значения.

Copy link
Owner

@unclead unclead Aug 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Если у класса нет атрибута, то будет исключение при присвоении ему значения.

ну так я могу использовать виджет вообще без модели и без ActiveForm

echo MultipleInput::widget([
    'id'                => 'my-input',
    'limit'             => 10,
    'allowEmptyList'    => false,
    'enableGuessTitle'  => true,
    'min'               => 2,
    'name'              => 'name'
]);

В контроллер при этом придет просто массив данных, который я уже по своему усмотрению могу обработать.

Copy link
Owner

@unclead unclead Aug 26, 2021

Choose a reason for hiding this comment

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

и вот про этот пункт я тоже не до конца понял

<?= $form->field($model, 'filter')->widget(MultipleInput::class, [
    'allowEmptyList' => true,
    'addButtonPosition' => MultipleInput::POS_FOOTER,
    'columns' => [
        [
            'name'  => 'field',
            'enableError' => true,
        ],
    ],
])->label(false); ?>

в этом случае имя инпута должно быть ClassName['filter']['field'][0], иначе теряется весь смысл. Например, такой кейс вот в этом примере https://github.com/unclead/yii2-multiple-input/blob/master/examples/views/multiple-input.php#L51

Copy link
Author

@olegbaturin olegbaturin Aug 26, 2021

Choose a reason for hiding this comment

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

Надо значит добавлять проверку if ($model instanceof Model).
Нам же в итоге надо корректно сделать и для Model в том числе.

Copy link
Author

@olegbaturin olegbaturin Aug 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

в этом случае имя инпута должно быть ClassName['filter']['field'][0]

Тут смотря какая логика закладывается. Либо у нас массив значений атрибута с одним или несколькими полями, тогда будет один вид имени для input и для нескольких колонок и для одной. Т.е. у нас строка №1, 2, ... и в ней поля (одно или несколько). Я выбрал его для однообразия, чтобы не делать разные валидаторы для разных кейсов с одной логикой.

Второй вариант ['filter']['field'][0] - это атрибут, и у него массив колонки. Мне не очень понятна эта логика.

В случае с одной колонкой у нас должен быть фактически массив атрибута со значенем типа строка, а не массив.

Имя всегда должно начинаться с ['filter'][0], чтобы общая логика не нарушалась.

Copy link
Author

@olegbaturin olegbaturin Aug 26, 2021

Choose a reason for hiding this comment

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

Для примера. Был фильтр с одной колонкой. Добавили вторую. Либо было несколько колонок, оставили одну. Всегда имя у input должно быть одинаковым. Сейчас оно будет меняться при увеличении колонок от одной или уменьшении до одной.

$attribute = $this->name . '[' . $index . ']';
} else {
$attribute = $this->context->attribute . $this->getElementName($index, false);
$attribute = $this->context->attribute . '[' . $index . '][' . $this->name . ']';
}

$model = $this->context->model;
Expand Down