#347 separate input name generator method from error field name gener… #357
Conversation
src/MultipleInputColumn.php
Outdated
| @@ -125,10 +123,10 @@ public function getFirstError($index) | |||
| return null; | |||
| } | |||
|
|
|||
| if ($this->isRendererHasOneColumn()) { | |||
| if ($this->isRendererHasOneColumn() && $this->hasModelAttribute($this->name)) { | |||
There was a problem hiding this comment.
я не очень понимаю зачем тут проверка на hasModelAttribute, ведь я могу использовать одну колонку для поля которого нет в модели и в этом случае тоже должен быть одноколоночный формат имени
There was a problem hiding this comment.
Одна колонка может быть двух видов. Как самостоятельный атрибут и как колонка у атрибута:
- Как атрибут класса - для этого делается проверка наличия атрибута у класса. В этом случае ошибку будем вешать в объекте класса модели так
$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' в данном примере.
- Как обычное поле у корневого атрибута.
input name такое же, как и при нескольких колонках<input name="filter[0][from]">или<input name="ClassName[filter][0][from]">. Ошибку будем вешать так$this->addError('filter[0][field]', 'Ошибка у filter №1, поле field').
я могу использовать одну колонку для поля которого нет в модели и в этом случае тоже должен быть одноколоночный формат имени
Если у класса нет атрибута, то будет исключение при присвоении ему значения.
There was a problem hiding this comment.
Если у класса нет атрибута, то будет исключение при присвоении ему значения.
ну так я могу использовать виджет вообще без модели и без ActiveForm
echo MultipleInput::widget([
'id' => 'my-input',
'limit' => 10,
'allowEmptyList' => false,
'enableGuessTitle' => true,
'min' => 2,
'name' => 'name'
]);
В контроллер при этом придет просто массив данных, который я уже по своему усмотрению могу обработать.
There was a problem hiding this comment.
и вот про этот пункт я тоже не до конца понял
<?= $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
There was a problem hiding this comment.
Надо значит добавлять проверку if ($model instanceof Model).
Нам же в итоге надо корректно сделать и для Model в том числе.
There was a problem hiding this comment.
в этом случае имя инпута должно быть ClassName['filter']['field'][0]
Тут смотря какая логика закладывается. Либо у нас массив значений атрибута с одним или несколькими полями, тогда будет один вид имени для input и для нескольких колонок и для одной. Т.е. у нас строка №1, 2, ... и в ней поля (одно или несколько). Я выбрал его для однообразия, чтобы не делать разные валидаторы для разных кейсов с одной логикой.
Второй вариант ['filter']['field'][0] - это атрибут, и у него массив колонки. Мне не очень понятна эта логика.
В случае с одной колонкой у нас должен быть фактически массив атрибута со значенем типа строка, а не массив.
Имя всегда должно начинаться с ['filter'][0], чтобы общая логика не нарушалась.
There was a problem hiding this comment.
Для примера. Был фильтр с одной колонкой. Добавили вторую. Либо было несколько колонок, оставили одну. Всегда имя у input должно быть одинаковым. Сейчас оно будет меняться при увеличении колонок от одной или уменьшении до одной.
src/MultipleInputColumn.php
Outdated
|
|
||
| if (!$withPrefix) { | ||
| return $elementName; | ||
| if ($this->isRendererHasOneColumn() && $this->hasModelAttribute($this->name)) { |
There was a problem hiding this comment.
см. аналогичный комментарий ниже
|
@olegbaturin а ты проверил, что весь функционал работает? 😄 Тестов то нифига нет 😄 |
Пофиксил для конфига без колонок. Проверял 4 кейса с/без классом в имени input.
|
|
Вообще в текущем виде PR меняет довольно сильно логику и это может сломать обратную совместимость учитывая отсутствие нормальных тестов. @olegbaturin а ты смотрел мой быстрофикс? #355 Дело в том, что давно летает в воздухе мысль все переписать нормально в 3 версии и вот в ней уже можно ломать обратную совместимость сколько душе будет угодно 😄 |
Сломается только имя для одной колонки. Могу добавить проверок, чтобы для одной колонки и у модели нет атрибута, вид остался |
Смотрел. Не помню, что не понравилось. Может как раз разное имя для одной и нескольких колонок. Вероятно, что он сейчас и решит баг, если оставить разные имена для одной и нескольких колонок. |
вот такие изменения должны быть между мажорными версиями, т.е. в версии 3 про которую я чуть выше упоминал. И Как раз одно из планируемых изменений это унификация имени поля, чтобы не было этой магии "одна колонка или несколько". Поэтому я против таких кардинальных изменений в версии 2 и нужен фикс в рамках текущей архитектуры (который на мой взгляд я сделал в #355). Другой вариант - ты можешь использовать свой форк 😄 |
|
Вот связанный issue - #295 |
|
Вообще основная идея 3 версии - убрать всю магию и сделать все максимально линейно. Плюс вынести большинство логики в отдельные классы |
Одну колонку всё равно придется как сейчас обрабатывать отдельно. В случае когда в опциях нет колонок или когда название атрибута у модели совпадает с назанием единственной колонки (по сути переопределение дефолтной колонки). Для кейсов с моделью и без по классам разделить сразу идея приходит в голову. Слишком много условий иначе получается. |
ClassName[attr][index][field]
attr[index][field]
Если нет колонок или колонка одна и field есть в модели. Я бы добавил еще условие равенства имён attr=field.
ClassName[field][index]
field[index]
2. Метод для генерации input name используется и для генерации имени атрибута модели, на который будет присвоены ошибки. В этом изначальная причина бага. Имя атрибута для ошибок не обязательно привязывать к методу фреймворка для генерации input name. Заменять метод фреймворка для генерации input name не надо, чтобы не получать подобных багов.
3. Я бы добавил условие совпадения attr=name для генерации имени в виде ClassName[field][index] (field[index]).