OFFLINE-GmbH / oc-mall-plugin

:convenience_store: E-commerce solution for October CMS
https://offline-gmbh.github.io/oc-mall-plugin
MIT License
170 stars 113 forks source link

JQuery must load before component 'enhancedEcommerceAnalytics' #337

Closed haralake closed 5 years ago

haralake commented 5 years ago

Enhanced E-commerce Analytics must loaded on head of the page, that also means you must load jquery on page head before component. Is there a way to prevent this behavior and load jquery on bottom of the body and still use component 'enhancedEcommerceAnalytics'?

Perhaps using :

$( document ).ready(function() {
        console.log( "document loaded" );
    });
tobias-kuendig commented 5 years ago

Just add this to your head:

<script>
    window.dataLayer = window.dataLayer || [];
</script>

Then add the component wherever you want. This code piece above makes sure that the dataLayer object is only defined once even if you include the code multiple times.

haralake commented 5 years ago

I change layout.htm according to your instructions:

<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport"
          content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0">
    <meta http-equiv="X-UA-Compatible" content="ie=edge">
    <meta name="description" content="{{ this.page.meta_description }}">
    <title>{{ this.page.title }}</title>
    {% styles %}
    <link rel="stylesheet" href="{{ 'assets/main.css' | theme }}">
<script>
    window.dataLayer = window.dataLayer || [];
</script>
</head>
<body>
{% partial 'header' %}
{% partial 'navigation' %}

<main class="container mx-auto py-8">
    {% page %}
</main>

{% partial 'footer' %}

    <script src="https://code.jquery.com/jquery-3.2.1.min.js"
            integrity="sha256-hwg4gsxgFZhOsEEamdOYGBf13FyQuiTwlAQgxVSNgt4="
            crossorigin="anonymous"
    ></script>
    {% component 'enhancedEcommerceAnalytics' %}

<script src="{{ 'assets/app.js' | theme }}"></script>
{% framework extras %}
{% scripts %}

{% flash %}
    <script>
        $(function() {
            $.oc.flashMsg({type: '{{ type }}', text: '{{ message }}'})
        })
    </script>
{% endflash %}
</body>
</html>

but when i open products "category-1" i get error:

Uncaught ReferenceError: $ is not defined
    at category-1:469
tobias-kuendig commented 5 years ago

What code is on line 469?

haralake commented 5 years ago

This is the code:

    // Create lookup object for item clicks.
    var lookup = {}
    data.ecommerce.impressions.map(function (item) {
        lookup[item.id] = item
    })

    // Handle item clicks
    $('[data-mall-item-id]').on('click', function (e) {
        var link = this
        var item = lookup[this.dataset.mallItemId]
        if (!item) {
            return;
        }
haralake commented 5 years ago

Now i get error:

category-1:472 Uncaught ReferenceError: $ is not defined
    at category-1:472

472 line is: $('[data-mall-item-id]').on('click', function (e) {

tobias-kuendig commented 5 years ago

Thank you! Can you check if these changes fix the problem for you: 23bba89

haralake commented 5 years ago

I test your changes but unfortunately doesn't fix this problem. I still get the same error.

tobias-kuendig commented 5 years ago

Are you sure it's the same error? At least the line number should have changed. There is also a missing $(function(){}) wrapper that is required with your setup. Please check this as well: 4d94b66

haralake commented 5 years ago

The line changes according to the product category I have opened. But the error always comes from this code: $('[data-mall-item-id]').on('click', function (e) {

I also check your latest fix without success. If i put jquery script on page head before component as your demo theme everything works ok.

tobias-kuendig commented 5 years ago

jquery is included before the line quoted above, right? And the line is wrapped in a $(funtion())?

haralake commented 5 years ago

Jquery is included after the line. Do you see any possible mistakes on layout.htm i posted earlier?

tobias-kuendig commented 5 years ago

This is your setup:

 <script src="https://code.jquery.com/jquery-3.2.1.min.js"
            integrity="sha256-hwg4gsxgFZhOsEEamdOYGBf13FyQuiTwlAQgxVSNgt4="
            crossorigin="anonymous"
    ></script>
    {% component 'enhancedEcommerceAnalytics' %}

<script src="{{ 'assets/app.js' | theme }}"></script>
{% framework extras %}
{% scripts %}

If you have wrapped everything in {% put scripts %} as the commit above added, it is impossible that the code becomes before jquery. You have to check out why this happens. Either you did not modify the partials right or you modified the wrong partials (do you have overrides in your theme maybe?)

haralake commented 5 years ago

You have right. This happen when i try to override products component. I just copy paste the original folder from mall plugin without any changes and i get the error. Ηow can I avoid that?

tobias-kuendig commented 5 years ago

Please read up on overriding component partials: https://octobercms.com/docs/cms/components#overriding-partials

If you copy the whole folder in your theme directory your changes will override the base plugin partials. Only copy into your theme what you want to modify.

damsfx commented 1 year ago

Are you sure it's the same error? At least the line number should have changed. There is also a missing $(function(){}) wrapper that is required with your setup. Please check this as well: 4d94b66

I think the problem is more global because the plugin makes use of

$(function() { ... });

who is a short-hand for jQuery :

$(document).ready(function() { ... });

This assume jQuery is yet loaded before every call to that short-hand.

I think it would be better to use vanilla javascript event listeners :

document.addEventListener("DOMContentLoaded", function() {
  // code
});

or to put scripts in OctoberCms {% scripts %} tag see #651.