search

a-digital / cookie-consent-banner

Add a configurable cookie consent banner to the website.
MIT License
11 stars 13 forks source link

Plugin causing validation issues on AMP pages #8

Closed PaulCWoods closed 4 years ago

PaulCWoods commented 5 years ago

Is it possible to exclude the banner on certain templates? In my case an amp.html template. Here it’s causing validation errors since it injects css and js.

mark-adigital commented 5 years ago

Don't want to totally leave you hanging with this @PaulCWoods - and I've had similar requests previously which was why I added the settings to enable/disable the banner in specific sections.

My argument against introducing a variable to include the snippets, which would seem the most logical approach to fix the issue you are seeing as this could then be added on a per-template basis, is that it's a breaking change for everybody already using the plugin which I'm a bit reluctant to do really. Obviously, this is also a free plugin I maintain around my regular work schedule so I've not had much time to consider what the best approach may be. Perhaps it would be worth adding a setting to toggle whether the banner should be auto-injected or added with a variable, then you have the best of both worlds. I'll look to add that when I get some time.

PaulCWoods commented 5 years ago

Thanks @mark-adigital . Yes the toggle approach sounds like a sensible solution. Great little plug in by the way.

mark-adigital commented 5 years ago

No worries - and thank you! I can't take all the credit as it's a port of a Craft 2 plugin which the previous developer couldn't get around to remaking... Though with the greater visibility lent by the plugin store, it has seen quite a few updates and improvements since that initial port over!

mark-adigital commented 4 years ago

Hi @PaulCWoods - apologies for the long delay, but I just today released an update with various new configuration options, including a setting to not auto-inject the banner and instead render it with a variable as described above.

PaulCWoods commented 4 years ago

Hi @mark-adigital ! I'll get testing it at the start of next week and update the ticket, but this sounds like a great solution. Thanks again.

jordanmoore commented 4 years ago

Hey @mark-adigital, thanks for the great plugin and the work on the latest update for the auto-inject setting. I've tried switching auto-inject off and using the variable {{ craft.cookieConsentBanner.addBanner() }} but I get a server error message when I do.

aarmitage commented 4 years ago

Hi @jordanmoore - Mark is away for a couple of days and will be able to take a look at this for you next week. Hopefully this will be ok.

mark-adigital commented 4 years ago

Hi @jordanmoore , I've just added a new release which should address the error you are seeing. Hope that helps.

PaulCWoods commented 4 years ago

@mark-adigital thank you. This is working great now! Marking as closed.