-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Make symfony forms easier to extend from modules using form_rest #12585
Comments
what about something like smarty extend and smarty blocks? how it is working in Twig? so developer would create: with content only like: {extend 'originalfile'} i'm not familiar with Twig enough to tell you how this should be done but i'm assuming that there's some option in Twig to do that, right? Aaand, if that's not a good idea, i would love to see 2nd option, hooks are not bad and replacing whole file just to add something is imho much worst idea |
@kpodemski it is possible to use blocks. However, it does not work because |
oh :( it was very convenient way to do such a stuff using Smarty, I'll try to do some research, it's hard to believe that Twig is less flexible than Smarty :P |
@kpodemski its not because of Twig, it because how PrestaShop uses it to find templates. :) |
i'm in favour of seconf solution,
and the both templates below:
OURMODULE/OURMODULE/form.html.twig
the main advangage, is that we can reuse the same FormBuilder on the hook storing the datas it works pretty well |
I like the hook solution too because it reuses a well known and powerful mechanism of PrestaShop, so we stay in common ground and helps developers travel from 1.6 to 1.7 and to Symfony using things they are familiar with. Maybe Mickael was rather for solution 1 because I think it benefits from Twig cache optimization while solution 2 is specific to PrestaShop and less prone to templating performance boost. |
ping @matks as a reminder. ^^ |
I have crazy but rather quite good and simple idea we can implement here. We can choose 2 option which is mentioned here #12585 (comment) but rather when enforcing everyone to add custom hooks inside every template we can hide implementation here: https://github.com/PrestaShop/PrestaShop/blob/develop/src/PrestaShopBundle/Resources/views/Admin/TwigTemplateForm/form_div_layout.html.twig#L343 in short: we add display hook call in benefits:
since we need to user form_rest everywhere developers will only need to be reminded just to add form_rest inside the |
@tomas862 this is sneaky 😄but indeed it solves the issue of making sure everybody puts the required hook. Only thing I'm worried about is performance overheard. I'd like us to analyze it (maybe we need a POC) to make sure the additional processing there is not too expensive for template rendering. Because this hook is going to be called everywhere we use |
After discussing with @eternoendless, here's the plan:
Bad display, without rendering: https://prnt.sc/nc96pk |
Good news: we were wrong, it works ! 🎉 See PrestaShop/docs#274 |
Form theme has been updated, form_rest will hooking capabilities will be implemented in 1.7.7 following #12585 (comment) |
Follow up will be done #16482 I close this now. |
Modules can extend Symfony forms by adding new fields, but form templating is a bit tricky, see example below.
Let's say we want to customize Email configuration form. We have added new fields using hook, but form does not look pretty. So we want to customize template.
Take this https://github.com/PrestaShop/PrestaShop/blob/develop/src/PrestaShopBundle/Resources/views/Admin/Configure/AdvancedParameters/Email/index.html.twig#L42 as example.
In order to render our field we need to replicate exact path of template in our module
modules/my_module/views/PrestaShop/Admin/Configure/AdvancedParameters/Email/index.html.twig
and copy whole content of original template inside our module's template. Then we can add our custom field rendering and it works. However, if template were to change (styling, classes, layout & etc) in core, module would still be using outdated template (unless updated whole content manually).I see 2 options how we could improve that:
By doing this, multiple modules can add custom fields to forms but only 1 module can extend template and render it how it wants to.
i know @mickaelandrieu was against 2nd option, but i dont see why? hooks are still present in PS and they wont go away at least until next major, right? so why not letting multiple modules extend symfony forms? 🤔
so @mickaelandrieu @matks wdyt?
The text was updated successfully, but these errors were encountered: