sampart / BreadcrumbsBundle

A small breadcrumbs bundle for Symfony
Other
188 stars 68 forks source link

Add support for template overrides with bundles path #94

Closed akalineskou closed 6 years ago

akalineskou commented 6 years ago

I'm working with Symfony 4, and tried to override the template using the method they say in the docs, and it wasn't working. Being new to the whole Symfony framework was a good chance for me to go and try to make it work. So I did, it works with both files in the templates/bundles/WhiteOctoberBreadcrumbsBundle/ folder and with custom viewTemplate passed to the wo_render_breadcrumbs function.

I only tested it with Symfony 4, I tried with Symfony 3 but for some reason my test project wasn't loading, and I didn't want to lose too much time making version 3.4 to work.

Cheers.

akalineskou commented 6 years ago

For anyone wanting to review these changes, add this to your project composer.json

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

and do a composer update, make sure to remove this after trying it out.

sampart commented 6 years ago

Thanks for contributing, @alex2005git, and sorry for the delay in replying. I'm afraid that we don't have time to test this on Symfony 3.4 for you, so you'll need to do that before we can merge this. I'll review the PR once you've confirmed it works in sf3. Thanks.

akalineskou commented 6 years ago

Hello @sampart, tested it with symfony 3.4.18 and it works.

akalineskou commented 6 years ago

@sampart I've reverted the travis and twig changes. But composer cant install framework-bundle for the old versions, and the tests fail, do you have any idea why? After updating composer symfony/templating to use the same version with symfony/framework-bundle all tests pass except for 2.3.* (dont know why)

This is the CI with the old "symfony/templating": "^3.4|^4.0" https://travis-ci.org/whiteoctober/BreadcrumbsBundle/builds/451297777

sampart commented 6 years ago

@alex2005git thanks for your work here, sorry it's proving so complicated. I'm hoping to discuss removing support for Symfony 2.3 with one of my colleagues before too long, but not sure when that will be, sorry.

As for this specific issue, I'm not sure of exactly what we should do here, but you could try switching to these constraints:

"symfony/templating": "^2.3|^3.4|^4.0"

Sorry not to have a better answer here.

akalineskou commented 6 years ago

@sampart finally managed to make them compatible.
I also had to change browser-kit.

^2.3|^3.0|^4.0 supports all the versions tested in travis.

akalineskou commented 6 years ago

@sampart I'm closing this PR since this has proven a more difficult task than anticipated, and I don't have the time to correctly implement and test it.
In a previous version of this PR I removed templating, but I thought that was a mistake on my part, but unfortunately this was a needed change to make the bundle work with relative paths, see this issue https://github.com/sensiolabs/SensioGeneratorBundle/issues/587

Well, I made the override work by setting the config to

white_october_breadcrumbs:
    viewTemplate: 'bundles/WhiteOctoberBreadcrumbsBundle/microdata.html.twig'

so in a future release when this might get implemented, it will work out of the box.

Sorry for wasting your time!

sampart commented 6 years ago

@alex2005git no need to apologise. I'm sorry that this has been such a time-consuming process for you. Thank you very much for your time and involvement here nonetheless, and do feel free to submit further issues or PRs in process - your involvement is appreciated. Thanks again, Sam.