sulu / SuluThemeBundle

MIT License
18 stars 15 forks source link

Symfony 5 compatibility and Switch to SyliusThemeBundle #19

Closed mario-fehr closed 3 years ago

mario-fehr commented 4 years ago
Q A
Bug fix? no
New feature? no
BC breaks? yes
Deprecations? no
Fixed tickets fixes #17
Related issues/PRs #17
License MIT

What's in this PR?

This PR removes LiipThemeBundle dependency and added SyliusThemeBundle to have Symfony 5 compatibility.

BC Breaks/Deprecations

Due to the exchange of bundles, the folder structure must be adapted and the configuration exchanged for old projects.

To Do

mario-fehr commented 4 years ago

@danrot @alexander-schranz Alex told me to open a PR to show my work so far testing the SyliusThemeBundle in combination with Sulu. But I need some help with the test and pipline.

danrot commented 4 years ago

@mfehr94 But otherwise it is already working as expected? If it is only about the tests you can get back to me some day in the office, that's probably the easiest way to fix that :slightly_smiling_face:

JKetelaar commented 4 years ago

We're using this in our development environment. Seems to work perfectly!

For anyone who wants to have a workaround for Symfony 5 & Theme support: composer.json

    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/mfehr94/SuluThemeBundle"
        }
    ]

...

    "require": {
        "sulu/theme-bundle": "dev-feature/symfony5"
    }
JKetelaar commented 4 years ago

After testing this, it does cause issues when editing a page from the Sulu admin. I will provide a stacktrace later today.

dsb-hannes-flaig commented 3 years ago

Ive followed @JKetelaar approach and added the Bundle to a fresh Sulu skeleton made with composer create-project sulu/skeleton cms_sulu_skeleton -n.

Problem When trying to save a page in the admin panel, the preview area does not show a preview but instead displays a symfony error:

grafik

Stacktrace:

TypeError:
Argument 1 passed to Sylius\Bundle\ThemeBundle\Repository\InMemoryThemeRepository::findOneByName() must be of the type string, null given, called in /var/www/fly/cms_sulu_skeleton/vendor/sulu/theme-bundle/EventListener/SetThemeEventListener.php on line 63

  at vendor/sylius/theme-bundle/src/Repository/InMemoryThemeRepository.php:42
  at Sylius\Bundle\ThemeBundle\Repository\InMemoryThemeRepository->findOneByName(null)
     (vendor/sulu/theme-bundle/EventListener/SetThemeEventListener.php:63)
  at Sulu\Bundle\ThemeBundle\EventListener\SetThemeEventListener->setActiveThemeOnPreviewPreRender(object(PreRenderEvent), 'sulu.preview.pre-render', object(EventDispatcher))
     (vendor/symfony/event-dispatcher/EventDispatcher.php:270)
  at Symfony\Component\EventDispatcher\EventDispatcher::Symfony\Component\EventDispatcher\{closure}(object(PreRenderEvent), 'sulu.preview.pre-render', object(EventDispatcher))
     (vendor/symfony/event-dispatcher/EventDispatcher.php:230)
  at Symfony\Component\EventDispatcher\EventDispatcher->callListeners(array(object(Closure)), 'sulu.preview.pre-render', object(PreRenderEvent))
     (vendor/symfony/event-dispatcher/EventDispatcher.php:59)
  at Symfony\Component\EventDispatcher\EventDispatcher->dispatch(object(PreRenderEvent), 'sulu.preview.pre-render')
     (vendor/sulu/sulu/src/Sulu/Bundle/PreviewBundle/Preview/Renderer/PreviewRenderer.php:173)
  at Sulu\Bundle\PreviewBundle\Preview\Renderer\PreviewRenderer->render(object(PageDocument), '10334765-80cb-4cb1-aec5-dac045963da6', 'example', 'en', false, -1)
     (vendor/sulu/sulu/src/Sulu/Bundle/PreviewBundle/Preview/Preview.php:140)
  at Sulu\Bundle\PreviewBundle\Preview\Preview->render('2c3502e7ea458d8cd27fa865fa651f72', 'example', 'en', -1)
     (vendor/sulu/sulu/src/Sulu/Bundle/PreviewBundle/Controller/PreviewController.php:77)
  at Sulu\Bundle\PreviewBundle\Controller\PreviewController->renderAction(object(Request))
     (vendor/symfony/http-kernel/HttpKernel.php:157)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:79)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:196)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (public/index.php:66) 
alexander-schranz commented 3 years ago

@SirCoffeeMonkey Thank you for report. Did you configure the <theme> in your webspace.xml? Can you debug what $event->getAttribute('webspace') does return?

mario-fehr commented 3 years ago

@SirCoffeeMonkey if you have <theme> configured, you need to make shure that you match the naming convention of the SyliusThemeBundle. https://github.com/Sylius/SyliusThemeBundle/blob/master/docs/theme_configuration_reference.md So if we take this example it must look like this <theme>vendor/sylius-theme</theme> Unfortiantly the naming convention for the theme name must be composer style with a slash for example vendor/sylius-theme or massive/crimson-theme

mario-fehr commented 3 years ago

Addition to that your theme need a composer.json (it is possible to change the file name by config) file with the theme infos https://github.com/Sylius/SyliusThemeBundle/blob/master/docs/index.md.

My sylius_theme.yaml looks like this:

    sources:
        filesystem:
            filename: theme.json // default: composer.json
            directories:
                - "%kernel.project_dir%/themes"

and the theme.json file in my themes/CrimsonTheme folder like this:

  "name": "massive/crimson-theme",
  "authors": [
    {
      "name": "Mario Fehr",
    }
  ],
  "extra": {
    "sylius-theme": {
      "title": "Crimson Theme"
    }
  }
}
dsb-hannes-flaig commented 3 years ago

@alexander-schranz thank you for your swift response. I forgot to properly configure the sylius theme bundle. i have added following line to my webspace.xml: <theme>test/test-theme</theme> Then i added the following lines to the config/packages/sylius_theme.yaml (which you have to manually create):

sylius_theme:
  sources:
    filesystem:
      directories:
        - "%kernel.project_dir%/themes"

Finally i created following folder structure: \<ProjectDir>/themes/YourTheme and added a composer.json in the YourTheme directory, with the following content:

{
  "name" : "test/test-theme",
  "extra": {
    "sylius-theme": {
      "title": "Test Theme"
    }
  }
}

For more details as to what can be configured in the theme composer.json you can refer to the links @mfehr94 kindly provided.

Now it is running and no longer throwing any errors 👍 😁 Thank you for putting me on the right path.

mario-fehr commented 3 years ago

@Prokyonn thanks for fixing the tests and dokumentationen

alexander-schranz commented 3 years ago

Thank you @mfehr94 @Prokyonn !