Skip to content

migrate seone function in theme hook - #124

Open
robinallezard wants to merge 5 commits into
twigfrom
feat/seone-theme-hook
Open

migrate seone function in theme hook#124
robinallezard wants to merge 5 commits into
twigfrom
feat/seone-theme-hook

Conversation

@robinallezard

Copy link
Copy Markdown
Collaborator

@anoziere anoziere left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The decoupling goal makes sense, but the migration is partial and there is a site-wide title regression.

  • base.html.twig still opens with {% set breadcrumb = SEOneBreadcrumb() %}, an unconditional module call in the base layout. Without SEOne installed, every page fatals on an unknown function. Same problem in index.html.twig (SEOneWebSite(), SEOneWebPage(), SEOneLocalBusiness()), category.html.twig (SEOnePageH1()), the Product component, and contact.html.twig. As long as those remain, the theme still hard-depends on the module.
  • The <title>{% block title %}...{% endblock %}</title> wrapper was removed from base.html.twig, but ~27 templates still define block title. Twig silently discards all of them, so no page renders a title tag anymore. The og:title/og:url/og:type/twitter meta defaults went away with it, and it's not obvious the hook side replaces them exactly.
  • src/EventListener/SeoListener.php ships four empty no-op methods, subscribes to SEOnePageh1Event while importing SEOnePageH1Event (case mismatch), and assigns getCurrentRequest() (nullable) to a non-nullable readonly Request property, which will throw outside an HTTP context.
  • The addPseToCart browser event and the SimilarContent exclude filter look unrelated to the SEOne migration; splitting them out would keep this PR easier to review.

Worth reviewing together with thelia-modules/SEOne#11 since the two halves have to land in a consistent state.

@anoziere
anoziere self-requested a review July 28, 2026 13:08
@robinallezard
robinallezard force-pushed the feat/seone-theme-hook branch from 77c110e to c5a1277 Compare July 31, 2026 13:11
@robinallezard

Copy link
Copy Markdown
Collaborator Author

1er point => Seone étant dans thelia-skeleton c'est normal qu'on est un couplage fort entre le module et le theme

2ème point => corrigé avec la nouvelle stratégie du module SEOne ( solution avec theme_hook et block twig)

les autres points sont liés à d'autres corrections apporté suite à la migration de GTM et recaptcha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants