-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make javascript injection easier for things like cookies consent, or others #366
Comments
First possible change suggest is that currently you can specify
The The The Right now everything has to be in the one file which is not convenient when needing to include multiple files from third party packages along with your own customisations. To enable having multiple files, have convention whereby for example in addition to Thus
If variant with no number is still provided, it would be embedded/included before the numbered ones. |
Steps for how to use current theme support to add CookieConsent package. |
So the idea of numbered files for stuff needed to be included in head of page is unnecessary since you can use the
The The needed for numbered files would not arise for inclusion at the end of the body if option existed for supply arbitrary HTML at the end of the body. That way could use multiple |
Note that need for including multiple Javascript files at end of body element of page only becomes a requirement where front loading them in the head element is not possible for performance or runtime reasons. |
Bit late to the party and not so much to add to the initial description, but I really like the suggestion to add Another solution to many situations like this could be to allow modification of CSP rules, as most 3rd party scripts/styles you might want to include are available from public CDNs. E.g. the mentioned cookie-consent script/css combination would be available from a CDN, but the hard-coded |
Concern over CSP is that there are different constraint types and need to know which to add them to.
|
Yeah I noticed this when going through the Django code - would it be possible/feasible to expose all |
Am a bit wary of doing that since no guarantee that the training portal will remain a Python/Django application. So any mechanism for specifying extra hostnames needs to be as simple as possible and not expose internal implementation details. As such, it may be better to just accept a list of additional host patterns and internally we make assumptions as to which of all those they should be added to, even if we end up adding hosts to a category they don't need to be in. |
I see. This definitely makes sense. I then share your concerns that hostnames passed to the portal's config might end up in the wrong/too many CSPs out of engineering necessity. In addition, I'll create a PR to document the use-case of 'including multiple additional |
@mocdaniel Even with CookieConsent injected, how were you then enabling/disabling analytics as necessary? Were you still relying on Educates analytics code injection or inserting your own? In https://cookieconsent.orestbida.com/advanced/manage-scripts.html it talks about using data attributes to mark script sections which should be enabled/disabled based on consent, but that relies on setting script type to Trying to work out whether anything else needs to be done to make existing analytics code injection work better with any cookie consent libraries. |
I inserted the same snippets you insert upon setting up Google Analytics via G-Tag by adding new entries to a custom theme |
Is your feature request related to a problem? Please describe.
Sometimes it's required to add additional javascript scripts to the Educates Platform to allow for things like cookie consent due to GDPR laws. Currently you can add these to a theme, but it's maybe overly complicated.
Describe the solution you'd like
Investigate what kind of javascripts would need to be injected (also if CSPs would need to be modified) and provide an easy way to set these up. An example of scripts could be https://cookieconsent.orestbida.com/
Describe alternatives you've considered
No response
Additional information
Current Kubernetes slack thread with some information about this: https://kubernetes.slack.com/archives/C05UWT4SKRV/p1715589756629829
The text was updated successfully, but these errors were encountered: