Closed (fixed)
Project:
EU Cookie Compliance (GDPR Compliance)
Version:
8.x-1.x-dev
Component:
Code
Priority:
Major
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
31 Jul 2020 at 09:57 UTC
Updated:
1 Mar 2022 at 08:55 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
john_b commentedComment #3
svenryen commentedThat's a valid point. I'll consider it, but it doesn't have top priority. Patches are welcome :)
Comment #4
svenryen commentedComment #5
svenryen commentedI'm flagging this for 2.x. If a patch arrives, we can also have this in 1.x.
Comment #6
svenryen commentedComment #7
john_b commentedI made a patch against the 7.x-1.x branch to allow configuration of cookie values for 'disagreed', 'agreed (show thank you banner)' and 'agreed'. Along with already configurable cookie name this allows ineroperability with similar modules on other sites. In our use case, we wanted it to be interoperable with the Complianz plugin for WordPreess.
We will be using D9 in due course, so probably will not work on a patch for 7.x-2.x.
Comment #8
john_b commentedNew version of the same patch which also fixes minor code issues reported in #3194889: prop is not a function in eu_cookie_compliance_admin.js and #3194891: background color and text color do not work.
Comment #9
john_b commentedThere was an error in the above patch. Try again.
Comment #10
svenryen commentedCould you please update your patch and remove the code for #3194891: background color and text color do not work, as it's seems that's not an issue with the module?
It's also not considered a good approach to mix code for two issues in the same patch unless the project members ask for it.
Comment #11
john_b commentedNew version of patch without code for background and text color.
Comment #12
john_b commentedNew version of patch, removing from js/eu_cookie_compliance_admin.js
This code checks or unchecks the checkbox in the admin UI. The old code using the deprecated prop() does not whereas the version using attr() does.
Comment #13
svenryen commentedComment #14
svenryen commentedThe patch seems to be for D7 so I ported it to D8.
Here's also an updated D7 patch that resolves some issues and adds an install hook.
@John_B are you able to review and test?
Comment #15
svenryen commentedRe-rolled patch for D8.
Comment #16
svenryen commentedNew patch, fixing syntax switch from jquery.cookie to cookies-js.
Comment #17
svenryen commentedFixed a bug that caused the banner to not show up.
Comment #18
john_b commentedThis is largely based on my own work so I hesitate to mark it 'reviewed'. However, it has been working in production, so I will mark it 'RTBC'.
Comment #19
svenryen commentedThanks, I'll merge later.
Comment #22
svenryen commentedComment #25
anybodyThis issue caused a regression in 7.x-1.x, see #3242662: Javascript error causes site to not load properly when using aggregation