yireo/Yireo_GoogleTagManager2

hyva_checkout_index_index.xml is loaded without having the hyva checkout enabled.

Closed this issue · 4 comments

wahidnory commented

Currently the layout hyva_checkout_index_index.xml is loaded because of thehyva_ prefix.

However the block does not have a name so we can easily disable it in a theme layout.

<block template="Yireo_GoogleTagManager2::hyva_checkout/data-layer.phtml"
                   ifconfig="googletagmanager2/settings/enabled">
                <arguments>
                    <argument name="magewire" xsi:type="object">Yireo\GoogleTagManager2\MageWire\Checkout</argument>
                    <argument name="begin_checkout_event" xsi:type="object">Yireo\GoogleTagManager2\DataLayer\Event\BeginCheckout</argument>
                </arguments>
            </block>

We don't have Hyva checkout enabled and no magewire, this is currently causing some issues with the layout update because of the hyva_ prefix.

Perhaps this should be bound to a "has hyva checkout enabled" flag in the configuration?

Or specify a name so we can remove it easily?

Just a thought but I think the ifconfig should be set to a hyva checkout is enabled flag.

jissereitsma commented

See my earlier reply on the PR. The following should provide a fix too: cfac4c2

I am releasing this within the hour.

  • 👍1
wahidnory commented

awesome thank you ❤️

wahidnory commented

A small suggestion
in vendor/yireo/magento2-googletagmanager2/MageWire/Component.php
You check for if the magewire class exists.
But the else returns a component, while this is getting injected in the block. Which raises the following error

1 exception(s):
Exception #0 (UnexpectedValueException): Instance of Magento\Framework\View\Element\Block\ArgumentInterface is expected, got Yireo\GoogleTagManager2\MageWire\Checkout instead.

Is it an idea to perhaps let the Component class implement the Argument interface as this?

class Component implements ArgumentInterface

That way the component remains compatible as a viewmodel.

I've added the relevant merge request again.

jissereitsma commented

I totally overlooked this part. I totally overlooked this. It is merged and will be added to the next release!

  • ❤️1