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

porting this plugin base on version sf #17

Open
wants to merge 18 commits into
base: 4.0
Choose a base branch
from

Conversation

hoand-vn
Copy link
Contributor

概要(Overview・Refs Issue)

  • PullRequestの目的、関連するIssue番号など

方針(Policy)

  • このPullRequestを作るにあたって考慮したものや除外した内容
     - Change some logic business.

実装に関する補足(Appendix)

  • will continue change for official version

@ryo-endo ryo-endo changed the base branch from master to 3.n July 10, 2018 05:56
*/
public function enable($config, $app)
public function enable($config = [], Application $app = null, ContainerInterface $container = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

enableの中の処理は、3.nでも必要ですか?

Copy link
Contributor

@ryo-endo ryo-endo left a comment

Choose a reason for hiding this comment

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

  • 管理画面への要素の追加は、TemplateEvent::addSnipetを使用してください。
    EC-CUBE#3244

  • フロントへの要素の追加は、TwigBlockを作成してください。
    EC-CUBE#3232
    既存のhtmlを変更する必要がある場合は、TemplateEvent::addSnipetを使用してください。

*/
public function onFormPreSubmit(FormEvent $event)
{
/** @var Category $Category */

Choose a reason for hiding this comment

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

Formへのマッピングはもう不要なはずなので、このEventSubscriber自体不要なはずです。

'maxlength' => $this->eccubeConfig['category_text_area_len'],
'placeholder' => $this->translator->trans('admin.plugin.category.content'),
],
])->addEventSubscriber(new EventSubscriber());

Choose a reason for hiding this comment

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

Eventで処理する必要はないです。

Choose a reason for hiding this comment

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

Hi @chihiro-adachi the reason I use event is:

  • I append content input into CategoryType by CategoryTypeExtension.
  • content input was not render on form (new category form & edit-inline category form)
  • Then when submit form (new category or edit-inline category), the POST does not contains key:value of content
  • It make value of content input always be null (see \Symfony\Component\Form\Form::submit( line 572))

@@ -0,0 +1,2 @@
parameters:
category_text_area_len: 4000

Choose a reason for hiding this comment

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

命名規約に合わせてください。
category_content_text_area_len

EC-CUBE/sample-payment-plugin#6

@@ -0,0 +1,7 @@
<?php
return [
'admin.plugin.category.content' => 'コンテンツを入力してください(HTMLタグ使用可)',

Choose a reason for hiding this comment

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

命名規約にあわせてください
EC-CUBE/sample-payment-plugin#6

config.yml Outdated
orm.path:
- /Resource/doctrine
event: Event
event: EventSubscriber

Choose a reason for hiding this comment

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

不要

@hoand-vn
Copy link
Contributor Author

for fixing

@hoand-vn hoand-vn closed this Jul 11, 2018
@hoand-vn
Copy link
Contributor Author

finish fixing

@hoand-vn hoand-vn reopened this Jul 13, 2018
@ryo-endo ryo-endo added this to the 4.0 milestone Aug 13, 2018
@okazy okazy changed the base branch from 3.n to 4.0 November 6, 2018 06:56
@kanako-kina
Copy link

@chihiro-adachi @ryo-endo 再レビューお願いします。

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.

5 participants