Skip to content
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

Expose properties of ITrackingConsentFeature and cookie policy in Liquid #17148

Merged
merged 5 commits into from
Dec 12, 2024

Conversation

gvkries
Copy link
Contributor

@gvkries gvkries commented Dec 7, 2024

Fixes #17134

@hishamco
Copy link
Member

hishamco commented Dec 7, 2024

IMHO this should be a part of Cookies Consent module

@MikeAlhayek
Copy link
Member

@hishamco do we have Consent module? We seems to be using the consent interface is multiple places and there isn't a consent module for this.

@hishamco
Copy link
Member

hishamco commented Dec 8, 2024

@hishamco do we have Consent module? We seems to be using the consent interface is multiple places and there isn't a consent module for this.

Nope, I already did one, also Lombiq, that's why I'm asking here, unless we could create an abstraction project for it

@MikeAlhayek
Copy link
Member

Then this PR is good where it is. Feel free to open another PR providing a consent module and move things to it if you add it

@hishamco
Copy link
Member

hishamco commented Dec 8, 2024

My concern is anyone will add his Liquid filters or tags to be supported in his/her module, which doesn't make sense to me

@MikeAlhayek
Copy link
Member

If there is already a module, that would be places in its corresponding module. If there isn't one, we can't request the PR creator to create a full blown module just to add Liquid filters.

If you create the consent module before releasing v3, we can move this filter there before the release.

@hishamco
Copy link
Member

hishamco commented Dec 8, 2024

@gvkries it would be nice if you added a Cookie Consent module to make this functional

gvkries and others added 3 commits December 8, 2024 13:51
…ackingConsentValue.cs

Co-authored-by: Mike Alhayek <mike@crestapps.com>
…ackingConsentValue.cs

Co-authored-by: Mike Alhayek <mike@crestapps.com>
…ackingConsentValue.cs

Co-authored-by: Mike Alhayek <mike@crestapps.com>
@gvkries
Copy link
Contributor Author

gvkries commented Dec 8, 2024

@gvkries it would be nice if you added a Cookie Consent module to make this functional

No, I don't want to duplicate the work that you, Lombiq, and others have already done. This PR only provides the ITrackingConsentFeature as well as some of the cookie options that are useful for theme authors. It can easily be used to a) check tracking consent in Liquid and b) create a basic consent function, e.g. using JS by using the cookie name/value. If a full module is desired, I think yours and Lombiq's modules are a good choice.

@MikeAlhayek
Copy link
Member

Anything else @hishamco ?

@MikeAlhayek MikeAlhayek merged commit ae4bb19 into OrchardCMS:main Dec 12, 2024
7 checks passed
@gvkries gvkries deleted the gvkries/liquid-cookieconsent branch December 12, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose ITrackingConsentFeature properties in Liquid
3 participants