Sylius / Sylius

Open Source eCommerce Framework on Symfony
https://sylius.com
Other
7.88k stars 2.09k forks source link

please make possible to inherit from templates instead of overwriting them #7491

Closed bruno-ds closed 6 years ago

bruno-ds commented 7 years ago

Hello,

Sylius is a great tool, it is so easy to customize it!

One way to make this customisation process more pleasant would be to let us inherit your template and alter only required blocks instead of having to overwrite them.

it would be possible that way (for example)

with fooBarAbstract.html.twigcontainig all your current code and fooBar.html.twig extending the abstract without defining anything, it is the one beeing calld to be rendered.

This way, if whe overwrite fooBar.html.twig whe can extend fooBarAbstract.html.twig and change only needed blocks.

I used a similar architecture on several projects, it is quite easy to maintain/use.

HellPat commented 7 years ago

@brunocombodo hey, the description is missing (think by accident). What is "them"?

bruno-ds commented 7 years ago

@psren yes, sorry, I miss-types and submited the form too ealry

HellPat commented 7 years ago

@brunocombodo this is already possible? I did that for example in https://github.com/Sylius/Sylius/pull/7441/files

Sometimes some blocks are missing and you have to overwrite the complete template. But the functionality is already given.

bruno-ds commented 7 years ago

@psren I'm rather new to symfony, so I can make a mistake, but from what I know, if you want to change for example the rendering of the products list in the shop, you have no choice than to create a file /app/Resources/SyliusShopBundle/views/Product/index.html.twig that completely overwrite /vendor/sylius/sylius/src/Sylius/Bundle/ShopBundle/Resources/views/Product/index.html.twig.

That is because this is the same file that

if you separate them, making the first being empty if not extending the one with the rendering, you can inherit, if you don't you can't (from what I know of symfony at least)

stefandoorn commented 7 years ago

I think this would make templating + upgrading way easier. Now we need to overwrite a full template, but it would be great just to use some extends and not break everything when an upgrade has to be performed. I also walked into this problem trying to adjust the main layout. The only solution it to fully overwrite it as far as I see, with the current set-up.

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

teohhanhui commented 6 years ago

It should be possible since Symfony 3.4: https://symfony.com/blog/new-in-symfony-3-4-improved-the-overriding-of-templates

But unfortunately this functionality is currently broken in Sylius.

teohhanhui commented 6 years ago

We should definitely move away from the Symfony Templating component. The shift has started since Symfony 2.7: https://symfony.com/blog/new-in-symfony-2-7-twig-as-a-first-class-citizen

It is now no longer used in new Symfony projects: https://github.com/symfony/symfony-standard/pull/1110, and work is under way to deprecate its integration in Symfony FrameworkBundle: https://github.com/symfony/symfony/pull/21035

Also see https://github.com/sonata-project/SonataBlockBundle/issues/456

teohhanhui commented 6 years ago

For a workaround, you may try blacklisting SyliusThemeBundle:

class AppKernel extends Kernel
{
    public function registerBundles(): array
    {
        $bundles = [
            // ...

            new \AppBundle\AppBundle(),
        ];

        $blacklist = [
            \Sylius\Bundle\ThemeBundle\SyliusThemeBundle::class,
        ];

        return array_filter(array_merge(parent::registerBundles(), $bundles), function ($bundle) use ($blacklist) {
            return !in_array(get_class($bundle), $blacklist, true);
        });
    }
}

It seems to work for me.

teohhanhui commented 6 years ago

Oops... Removing the SyliusThemeBundle was of course a bad idea. It breaks some parts of the admin.

Here's a cleaner workaround.

src/AppBundle/DependencyInjection/Compiler/RemoveSyliusThemeFilesystemLoaderPass.php:

<?php

declare(strict_types=1);

namespace AppBundle\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

class RemoveSyliusThemeFilesystemLoaderPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container): void
    {
        $container->removeDefinition('sylius.theme.twig.loader');
    }
}

src/AppBundle/AppBundle.php:

<?php

declare(strict_types=1);

namespace AppBundle;

use AppBundle\DependencyInjection\Compiler\RemoveSyliusThemeFilesystemLoaderPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\HttpKernel\Bundle\Bundle;

class AppBundle extends Bundle
{
    public function build(ContainerBuilder $container): void
    {
        parent::build($container);

        $container->addCompilerPass(new RemoveSyliusThemeFilesystemLoaderPass());
    }
}
teohhanhui commented 6 years ago

@pamil I hope we could remove the service. It shouldn't be a BC break as it's just a decorator. Thoughts? :smile:

malutanpetronel commented 4 years ago

I am in Sylius-standard and Iwas written a plugin there but the overwriting still does not work ?