Skip to content
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.

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

Добавляем в tpl кастомные параметры из вызова #742

Merged
merged 3 commits into from
Dec 18, 2022

Conversation

Alexij2
Copy link
Contributor

@Alexij2 Alexij2 commented Oct 23, 2022

Что оно делает?

Добавляем в tpl кастомные параметры из вызова сниппета, чтобы ими пользоваться в самой tpl-ке вывода.
Уже есть все свойства к wrapp обертке вывода, но это далеко не удобно.

в msProduct, Добавление свойств, как это сделано в pdoResources Если считаете что нужно по-другому - поправьте, но думаю было бы хорошо, чтобы логика поведения была одинакова как в pdoToolse сниппетах так и msProduct.

Связанные проблема(ы)/PR(ы)

Дополнение к
#711

Дополнение к 
modx-pro#711
в msProduct, Добавление свойств, как это сделано в pdoResources
Если считаете что нужно по-другому - поправьте, но думаю было бы хорошо, чтобы логика поведения была одинакова как в pdoToolse сниппетах так и msProduct.
@biz87
Copy link
Member

biz87 commented Nov 29, 2022

  1. Рекомендую соблюдать стандарты PSR-12 (в pdoTools они тоже не соблюдены. Там одно и то же в разных местах по разному написано)
  2. Я не понял почему у тебя параметры вызова добавляются в сниппет только при непустом результате? А если товаров не найдено, то параметры не нужны?
  3. Почему параметры вызова передаются при переборе в каждый товар? Это специально так задумано?
    $row = array_merge($row, $options,$addplace); Насколько я понял в pdoResources параметры сниппета передаются в обертку $output = $pdoFetch->getChunk($tplWrapper, array_merge($additionalPlaceholders, ['output' => $output]),
  4. Если я в параметрах вызова укажу существующее свойство товара например price = 100 - этот параметр перезапишет свойство товара.
  5. У меня вопросы к именованию переменных $addplace = $tmprops = []; это что за имя? Если ты делаешь по примеру pdoTools - почему бы не сделать как там? $additionalPlaceholders = $properties = []; Это же существенно понятнее.

$addplace = $tmprops = [];
if (isset($this) && $this instanceof modSnippet && $this->get('properties')) {
$tmprops = $this->get('properties');
}
Copy link
Member

Choose a reason for hiding this comment

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

Перенос строки в этом месте не рекомендован. Должен быть один пробел вместо переноса

}
}


Copy link
Member

Choose a reason for hiding this comment

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

Лишний перенос строки

@@ -224,7 +242,7 @@

$opt_time_start = microtime(true);
$options = $modx->call('msProductData', 'loadOptions', array($modx, $row['id']));
$row = array_merge($row, $options);
$row = array_merge($row, $options,$addplace);
Copy link
Member

Choose a reason for hiding this comment

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

Между параметрами рекомендован пробел

@Alexij2
Copy link
Contributor Author

Alexij2 commented Nov 29, 2022

PRщик из меня хреновый. Каюсь. Про логику работы значит не верно сделал. Раз все плохо, можно закрывать и просто сделать лучше. я и слал PR в надежде что профи посмотрит и сделает как надо.
Тогда буду в след. раз исушку оформлять, чтобы небыло больно глазам. Просто PR более наглядно покажет логику или ее отсутствие.
Спасибо.

@biz87
Copy link
Member

biz87 commented Nov 29, 2022

PRщик из меня хреновый. Каюсь. Про логику работы значит не верно сделал. Раз все плохо, можно закрывать и просто сделать лучше. я и слал PR в надежде что профи посмотрит и сделает как надо. Тогда буду в след. раз исушку оформлять, чтобы небыло больно глазам. Просто PR более наглядно покажет логику или ее отсутствие. Спасибо.

Может лучше доработать все таки?

@Alexij2
Copy link
Contributor Author

Alexij2 commented Dec 10, 2022

Покопался в pdoTools. Подробнее остановлюсь на вопросах:

  1. Рекомендую соблюдать стандарты PSR-12 (в pdoTools они тоже не соблюдены. Там одно и то же в разных местах по разному написано)

Хорошо. Постараюсь не нарушать пе-се-еры. Пока для меня это сложно.

  1. Я не понял почему у тебя параметры вызова добавляются в сниппет только при непустом результате? А если товаров не найдено, то параметры не нужны?

Ну, да. логично. Если товаров нет, то нет и смысла добавлять параметры. ониж в коде pdoFetch для каждого row пишутся.

  1. Почему параметры вызова передаются при переборе в каждый товар? Это специально так задумано?

Да, так и задумано. просто в pdoResource это делается не в сниппете, а на уровне pdoFetch для каждой строки.
(строка 133 класса pdofetch.class.php.)

  1. Если я в параметрах вызова укажу существующее свойство товара например price = 100 - этот параметр перезапишет свойство товара.

Да, исправлю приоритет.

  1. У меня вопросы к именованию переменных $addplace = $tmprops = []; это что за имя? ......

Боялся пересечений по переменным. пере-пиарю-по новой с исправлениями )

Исправил, подправил.
Предлагаю еще для $outer параметры всовывать сразу на уровне output. как в pdoToolse.
Думаю, проблем не должно возникнуть.
спасибо!
Забыл указать msProduct.
@biz87 biz87 merged commit ecbf8ac into modx-pro:master Dec 18, 2022
biz87 added a commit that referenced this pull request Dec 18, 2022
biz87 added a commit that referenced this pull request Dec 18, 2022
Несколько правок к #742
GulomovCreative pushed a commit to GulomovCreative/miniShop2 that referenced this pull request Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants