Skip to content

Feat/theme hook - #36

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

Feat/theme hook#36
robinallezard wants to merge 4 commits into
twigfrom
feat/theme-hook

Conversation

@robinallezard

Copy link
Copy Markdown

No description provided.

@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 theme-hook approach looks right and matches the SEOne/flexy work. Two behavior changes deserve a second look:

  • Service/DataLayerProvider.php (renderJsInit): the old code concatenated renderSelectItem() and renderAddToCart() on listing views; the new version returns one template or the other. addToCart.js is no longer loaded on category/brand/search/folder/content/page views, so add/remove cart tracking stops working wherever a listing page has an add-to-cart button.
  • Listener/GoogleTagListener.php (trackPurchase): $event->getPlacedOrder()->getId() has no null check, and getSession() throws when there is no request in scope. Payment modules can fire ORDER_PAY from an IPN/webhook context, which would turn a tracking miss into a fatal during payment. Both calls should be guarded.

Minor points: {{ containerId }} in script.html.twig/noscript.html.twig is now HTML-escaped inside a script tag, where the old PHP string was raw, so an unusual configured value would corrupt the emitted JS instead of rendering as before. And gtm_js_init seems to have no remaining caller now that the hook renders it directly; if that's intentional, it could be removed.

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