Jump to content

Cookie Management Banner

adrian

By adrian,
in Modules/Plugins

 Share

Recommended Posts

wbmnfktr Hero Member

wbmnfktr

Posted
Posted
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.

Link to comment
Share on other sites
wbmnfktr Hero Member

wbmnfktr

Posted
Posted

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;
}

 

Link to comment
Share on other sites
adrian Hero Member

adrian

Posted
  • Author
Posted

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
Link to comment
Share on other sites
adrian Hero Member

adrian

Posted
  • Author
Posted

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
Link to comment
Share on other sites
adrian Hero Member

adrian

Posted
  • Author
Posted
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
Link to comment
Share on other sites
wbmnfktr Hero Member

wbmnfktr

Posted
Posted

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
Link to comment
Share on other sites
adrian Hero Member

adrian

Posted
  • Author
Posted
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.

Link to comment
Share on other sites
wbmnfktr Hero Member

wbmnfktr

Posted
Posted
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
Link to comment
Share on other sites
tpr Hero Member

tpr

Posted
Posted

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
Link to comment
Share on other sites
adrian Hero Member

adrian

Posted
  • Author
Posted
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
Link to comment
Share on other sites
iank Sr. Member

iank

Posted
Posted

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
Link to comment
Share on other sites
adrian Hero Member

adrian

Posted
  • Author
Posted

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
Link to comment
Share on other sites
wbmnfktr Hero Member

wbmnfktr

Posted
Posted

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
Link to comment
Share on other sites
adrian Hero Member

adrian

Posted
  • Author
Posted
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?

Link to comment
Share on other sites
adrian Hero Member

adrian

Posted
  • Author
Posted

@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
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share
Go to topic listing

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...