leogout / SeoBundle

A Symfony bundle to generate SEO meta tags.
39 stars 18 forks source link

Fix bad examples and small configurator bug #5

Closed innerspirit closed 7 years ago

innerspirit commented 7 years ago

These examples did not seem to work for me. I spent a lot of time trying all your examples but none worked. The generators are loaded and setted, but those are not the objects used in the end.

Although there is a service called leogout_seo.generator.twitter (for example), the instance is not the same as what's being used by SeoGeneratorProvider. The end result is that the tags don't show up, because we are setting them on the wrong object. The new approach I used does work.

I think this has something to do with how SeoGeneratorPass loads the generators, but I am new to Symfony and I'm not sure what a better approach would look like.


This change is Reviewable

leogout commented 7 years ago

I've created a small project to try it out and it worked as in the docs. Have you registered the bundle correctly ? Have you configured it ? For better performances, the non-configured generators won't be loaded:

leogout_seo:
   basic: ~ # registers leogout_seo.generator.basic as a service

Could you create an issue instead, to help people with the same problem ?

I don't make the leogout_seo.provider.generator public on purpose: all the leogout_seo.generator.[og/basic/etc.] services are already registered and ready to be used, without using this utility service.

I've noticed something strange in your PR though: The ->setImage($url) in the URL config. I'll change the tests for this use case, it isn't covered and there is a bug here.

Thanks for that!

innerspirit commented 7 years ago

Yeah everything is set up as the readme explains, but it doesn't work, the dynamic part I mean. Configured tags do appear, but they can't be modified by a controller. Not with the setters, nor with the fromResource method. The only tags appearing are the ones on config.yml.

My template: {{ leogout_seo() }}

config.yml:

    general: ~
    og:
        type: website
        url: '%schemes%://%site_url%'
    twitter:
        card: summary
        site: '@gcba'

(all of these work properly, only dynamic tags don't work) I also tried adding twitter image here with ~, it appears empty.

Controller:

$this->get('leogout_seo.generator.twitter')->fromResource($jobPosting);

Entity implements TwitterSeoInterface with its methods.

The other approach with setters also doesn't work.

If I apply those changes in the PR, everything works fine.

leogout commented 7 years ago

Hello, I took the time to investigate and you are right, there is a critical problem I've totally missed, thank you again for your PR :D I would like it to work like in the examples but if i can't find a solution I'll use yours !

I'll try to investigate tonight. There is something wrong with the shared="false" in the leogout_seo.builder

leogout commented 7 years ago

Hello, I'll use your solution. Could you allow me to push commits to your PR please ? I'd like to change all the examples of the docs.

innerspirit commented 7 years ago

Sure, I've given you write access to my fork

leogout commented 7 years ago

Thank you very much for your help and sorry for my lack of disponibility. I'll make a hotfix of v1.0.0 now.