Jump to content
adrian

Cookie Management Banner

Recommended Posts

1 hour ago, adrian said:

I am curious though what scenario this is useful for - why do you prefer to manually inject them?

Ok... so this is my workflow:

At first I add all my necessary JS files into the <head> of my template.

<script defer src="/site/templates/scripts/vendor/jquery-3.3.1.js"></script>
<script defer src="/site/templates/scripts/plugins/plugin-a.js"></script>
<script defer src="/site/templates/scripts/plugins/plugin-b.js"></script>
<script defer src="/site/templates/scripts/plugins/typekit.js"></script>
<script defer src="/site/templates/scripts/plugins/tracking.js"></script>

This includes jQuery, all plugins I need for that project and sometimes tracking and Typekit.

To make things go faster I add the defer attribute to my calls and let do ProCache all the minify stuff which ends in a single-file request like this:

<script defer src=/site/assets/pwpc/pwpc-[RANDOMSTRING].js></script>

With your module enabled things break right now.

Your script tag is without defer and will be placed at the bottom of the template - therefore it ends in a separate file and won't work as expected.
I changed the module code right now and added the defer attribute and placed everything at the end of my template.

With the option to disable auto-injection I could easily keep my workflow, update most of my projects with ease and everything works and loads fast.

Share this post


Link to post
Share on other sites

I just ran into an issue using the Fixed Overlay at Top of Page placement option.

The overlay was showing even after giving/denying consent.

A look into your CSS showed that there is no real definition of hiding that banner.

I added a few lines at the end of the CSS file and everything works as expected now - with the fixed at the top option.

.pwcmb {
    display: none;
    visibility: hidden;
}
.pwcmb.js-show {
    display: block;
    visibility: visible;
}

 

Share this post


Link to post
Share on other sites

New version just committed that adds support for turning off autoloading of css and js assets files. It also fixes the strange issues that @wbmnfktr was having on some pages. It also adds a "pwcmb-active" class to the body of the page when the banner is displayed.

  • Like 6

Share this post


Link to post
Share on other sites

A significant update:

1) new auto-accept mode

2) fixes an issue when in EU only mode but the user is outside the EU - GTM tracking beacon is now properly sent

3) tweaks to support older versions of jQuery.

Please note that these changes are basically just a port of recent changes to the original Drupal version of the module so let me know if you notice any problems.

  • Like 3

Share this post


Link to post
Share on other sites
Just now, wbmnfktr said:

What's happening here in detail now?

You know what - given that I am not really the author of this logic, I think it's just an easy for you to take a look yourself (in the js file) as it is for me to explain it 🙂

Sorry if that seems lazy, but I don't really have time to maintain this module beyond bug fixes etc - I just want to make sure the PW community has access to it.

 

  • Like 1

Share this post


Link to post
Share on other sites

I'm fine with that. Thought you already tested this new option and looked into it.

But for those who want to know more... auto-accept mode is kind of a silent opt-in.

While the first view only triggers the banner itself and returns false for if(localStorage.getItem('pwcmbAllowCookies') == 'y') the second and all follow-up page views allow tracking and therefore return true for if(localStorage.getItem('pwcmbAllowCookies') == 'y'). Users will still see the banner and have to either accept (again) or have to opt-out.

 

  • Like 1

Share this post


Link to post
Share on other sites
2 minutes ago, wbmnfktr said:

Thought you already tested this new option and looked into it.

A little bit to make sure nothing appeared to be broken, but not thoroughly.

You probably noticed from the JS that this module is very focused on GoogleTagManager, although you can still use it without GTM.

Share this post


Link to post
Share on other sites
1 minute ago, adrian said:

You probably noticed from the JS that this module is very focused on GoogleTagManager, although you can still use it without GTM.

I saw it was mentioned somewhere but haven't done any further testing with GTM, GA, Matomo or anything else by now. 
My first site with this module will go online the next days and I will test all three mentioned trackers then. 

  • Like 1

Share this post


Link to post
Share on other sites

If the module is so tightly tied to the drupal module then converting the js from jquery to pure js would mean constantly tracking changes and too much possibility to fail somewhere in the conversion. The best would be then to ask the drupal devs if they are willing to drop jquery - if yes, we could help in the first step but after that they should be able to add updates (in pure js). I don't think every drupal site is using jquery, or is it so?

  • Like 3

Share this post


Link to post
Share on other sites
Just now, tpr said:

If the module is so tightly tied to the drupal module then converting the js from jquery to pure js would mean constantly tracking changes and too much possibility to fail somewhere in the conversion. The best would be then to ask the drupal devs if they are willing to drop jquery - if yes, we could help in the first step but after that they should be able to add updates (in pure js).

Yeah, I completely agree - it seems that VentureWeb always adds jQuery to their sites, but I rarely do. Let me check with them and see if they'd be willing to work with a non-jquery version. I have commit access to the Drupal version's repo, so if you or someone else would like to convert the PW version, I'd happily port those changes back to Drupal so things are easier going forward.

  • Like 4

Share this post


Link to post
Share on other sites

Adrian,

I just noticed on the "Manage Preferences" banner, the two options "I accept" and "I do not accept" are checkboxes, meaning it's possible to select both of these, which could be confusing for the user.   It only saves the "I accept" value to the local storage if both are checked, but if you popup the banner again without a page refresh, both checkboxes remain checked.

cookiebannercheckboxes.JPG

  • Like 1

Share this post


Link to post
Share on other sites

Hi @iank - that's strange. It looks like this for me:

consent.gif.40fa3b6885e14431761eb4e59eb01682.gif

There is some JS to uncheck the other checkbox here:

https://github.com/adrianbj/CookieManagementBanner/blob/8a1cafe555221c9de03299d1ffdd9dafd8fe4d63/assets/js/CookieManagementBanner.js#L180-L183

I don't really know why the original Drupal version of this took this approach over simple radio buttons, but it seems to work as expected here. 

I think it's a jQuery version issue. In one of the recent commits, "on" was changed to "bind" to support older versions of jQuery. I wonder if that's the problem you are seeing? Could you try changing that "bind" to "on" and see if it works for you?

  • Like 1

Share this post


Link to post
Share on other sites

I can confirm the behaviour @iank describes but I'm quite surprised that I notice this change just as of today.

I changed line 182 from [method] to .prop to get the functionality back.

$(this).parents('.pwcmb-option-wrapper').siblings().find('.pwcmb-widget__row-cb').prop('checked', !checked);

Is it me or is [method] wrong at that and some other places?

  • Like 1

Share this post


Link to post
Share on other sites

Beat me to it @wbmnfktr !  I was just about to ask a similar question.  I'm no jQuery expert but didn't recognise [method]. 

Share this post


Link to post
Share on other sites

I know [method] very well but only from documentations noone understands and not from productive code.

Especially with jQuery I miss often new things so maybe this is the new way to get things done. 😉 

Share this post


Link to post
Share on other sites
6 minutes ago, wbmnfktr said:

I can confirm the behaviour @iank describes but I'm quite surprised that I notice this change just as of today.

I changed line 182 from [method] to .prop to get the functionality back.


$(this).parents('.pwcmb-option-wrapper').siblings().find('.pwcmb-widget__row-cb').prop('checked', !checked);

Is it me or is [method] wrong at that and some other places?

"method" is being defined at the top of that file:

    // older versions of jQuery need to use the `attr` method to modify node properties, newer versions use `prop`
    var method = parseInt(jQuery.fn.jquery.split('.')[1], 10) > 6 ? 'prop' : 'attr';

It seems like jQuery 1.6 was where prop vs attr changed and that is what they are trying to deal with by using this, but maybe it's not working as expected?

Share this post


Link to post
Share on other sites

What jQuery version are you guys running?

I have been testing with 2.1.1

Share this post


Link to post
Share on other sites

Looking at that check, it's pretty clear that it ignores the main version of jQuery, eg 1, 2 or 3 and just looks for the point version being greater than .6 so it's pretty useless really.

Share this post


Link to post
Share on other sites

I'm on jQuery v3.2.1.  Tried on several different browsers too, with the same issue.  Changing to .prop as suggested by @wbmnfktr does seem to fix it.

Share this post


Link to post
Share on other sites

@wbmnfktr and @iank - would you please try reverting to the version currently on Github and then replace that line with:

var method = $.fn.jquery.replace(/\.(\d)/g,".0$1").replace(/\.0(\d{2})/g,".$1") > "1.6" ? 'prop' : 'attr';

and let me know if that works as expected.

Thanks!

  • Like 1

Share this post


Link to post
Share on other sites
Just now, wbmnfktr said:

This works as expected.

Tested in Chrome, EDGE, Firefox, Opera (all latest versions).

Thank you - I have committed that change and updated the module's directory.

  • Like 5

Share this post


Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.


  • Recently Browsing   0 members

    No registered users viewing this page.

×
×
  • Create New...