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

Ngstack 833 cookie consent cookie policy improvements new #210

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions assets/js/components-noncritical.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import SwiperBase from './components/SwiperBase.component';
import SwiperThumb from './components/SwiperThumb.component';
import VideoPoster from './components/VideoPoster.component';
import SkipToMainContent from './components/SkipToMainContent.component';
import FetchPageAndRender from './components/FetchPageAndRender.component';

// Configuration
const componentConfiguration = [
Expand All @@ -38,6 +39,11 @@ const componentConfiguration = [
optionalListToggle: '.optional-list-toggle',
rotateArrowClass: 'rotate-arrow',
shownClass: 'shown',
cookieModal: '.ng-cc-modal',
cookiePolicyShow: '.ng-cc-js-link-cookie-policy a',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to handle this to put the .ng-cc-js-link-cookie-policy class on link itself and not on wrapper and use the link as this value?

cookiePolicyShow: '.ng-cc-js-link-cookie-policy', // where html is text

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, a small nitpicker: can this key not be cookiePolicyShow but cookiePolicyShowTrigger?

cookiePolicyHide: '.cookie-policy-hide',
cookiePolicyText: '.ng-cc-cookie-policy-text',
cookiePolicyShownClass: 'cookie-policy-shown',
},
},
{
Expand Down Expand Up @@ -214,6 +220,15 @@ const componentConfiguration = [
Component: SkipToMainContent,
selector: '#skip-to-main-content',
},
{
Component: FetchPageAndRender,
selector: '.js-fetch-page',
options: {
triggerWrapper: '.js-fetch-page-trigger-wrapper',
pagePlaceholder: '.js-fetch-page-placeholder',
pageRemove: '.js-fetch-page-remove',
},
},
];

addDocumentEventListeners(componentConfiguration);
20 changes: 20 additions & 0 deletions assets/js/components/CookieControl.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ export default class CookieControl {
this.optionalSaveBtn = element.querySelectorAll(options.optionalSaveBtn);
this.optionalList = element.querySelector(options.optionalList);
this.optionalListToggle = element.querySelector(options.optionalListToggle);
this.cookieModal = element.querySelector(options.cookieModal);
this.cookiePolicyShow = element.querySelector(options.cookiePolicyShow);
this.cookiePolicyHide = element.querySelector(options.cookiePolicyHide);
this.cookiePolicyText = element.querySelector(options.cookiePolicyText);
this.cookiePolicyShownClass = options.cookiePolicyShownClass;

this.init();
}
Expand All @@ -18,9 +23,21 @@ export default class CookieControl {
this.optionalSaveBtn.forEach((element) =>
element.addEventListener('click', CookieControl.handleConsentChange)
);
this.cookiePolicyShow.addEventListener('click', (e) => {
e.preventDefault();
this.cookieModal.classList.add(this.cookiePolicyShownClass);
document.querySelector('.ng-cc-cookie-policy-text').scrollTop = 0;
});
this.cookiePolicyHide.addEventListener('click', (e) => {
e.preventDefault();
this.cookieModal.classList.remove(this.cookiePolicyShownClass);
});
}

static initNetgenCookieControl() {
if (this.element === null) {
return;
}
// eslint-disable-next-line no-underscore-dangle
const cookieControl = new NetgenCookieControl(window.__ngCcConfig);
cookieControl.init();
Expand All @@ -38,6 +55,9 @@ export default class CookieControl {
}

static handleConsentChange(event) {
if (this.element === null) {
return;
}
event.preventDefault();

GTM.push('ngcc', GTM.EVENTS.CHANGED);
Expand Down
72 changes: 72 additions & 0 deletions assets/js/components/FetchPageAndRender.component.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/* REQUIRED HTML STRUCTURE FOR IT TO WORK

FETCH PAGE AND RENDER WRAPPER
<div class="js-fetch-page" data-fetch-page-content-id="CONTENT_ID">

FETCHED PAGE PLACEHOLDER
<div class="js-fetch-page-placeholder"></div>

WRAPPED FETCH TRIGGER LINK FOR EZRICHTEXT TEXTS
<div class="js-fetch-page-trigger-wrapper"><a href=""></a></div>
OR JUST FETCH TRIGGER LINK
<a class="js-fetch-page-trigger-wrapper"></a>

FETCHED PAGE REMOVAL LINK FROM PLACEHOLDER
<a class="js-fetch-page-remove"></a>

</div>

*/
export default class FetchPageAndRender {
constructor(element, options) {
this.element = element;
this.options = options;

this.triggerWrapper = document.querySelector(options.triggerWrapper);
this.pagePlaceholder = document.querySelector(options.pagePlaceholder);
this.pageRemove = document.querySelector(options.pageRemove);

this.init();
}

init() {
if (this.triggerWrapper instanceof HTMLAnchorElement) {
this.triggerWrapper.addEventListener('click', this.handleFetchPage.bind(this));
} else {
this.triggerWrapper
.querySelector('a')
.addEventListener('click', this.handleFetchPage.bind(this));
}
}

handleFetchPage(event) {
event.preventDefault();

const url = `/view-payload/${this.element.dataset.fetchPageContentId}`;

fetch(url)
.then((response) => {
console.log(url, response);
if (!response.ok) {
throw new Error();
}
return response.text();
})
.then(this.handleRenderPage.bind(this))
.catch((error) => {
console.error(`Failed to fetch page content: ${error}`);
});
}

handleRenderPage(page) {
const template = document.createElement('template');
template.innerHTML = page.trim();

this.pagePlaceholder.appendChild(template.content);

this.pageRemove.addEventListener('click', (e) => {
e.preventDefault();
this.pagePlaceholder.innerHTML = '';
});
}
}
63 changes: 63 additions & 0 deletions assets/sass/_cookie_control.scss
Original file line number Diff line number Diff line change
Expand Up @@ -389,4 +389,67 @@
&[open] {
opacity: 1;
}

// cookie policy addition
.ng-cc-cookie-policy-text {
position: absolute;
inset: 0;
z-index: 10000000;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100000 was not enough? :D

please add a proper value based on the bootstrap z-index states:
https://getbootstrap.com/docs/5.2/layout/z-index/

transition: transform .3s ease;
transform: translateX(100%);
background: #fff;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use $white

overflow-y: auto;
.wrapper {
padding: 2rem 0 1rem 0;
margin: 0 2.5rem;
> * + * {
margin-top: 2rem;
}
}
.title {
margin-top: 2rem;
}
.intro {
margin-top: 1rem;
}
.body h3 {
margin-top: 2rem;
font-size: 1.25rem;
}
p {
margin-top: 1rem;
}
}
.ng-cc-modal {
overflow-x: hidden;
min-height: 180px;
max-width: calc(100% - 60px);
transition: min-height .3s ease, max-width .3s ease;
overscroll-behavior: contain;
&.cookie-policy-shown {
overflow: hidden;
min-height: 75vh;
max-width: 1024px;
.ng-cc-cookie-policy-text {
transform: translateX(0);
overscroll-behavior: contain;
}
}
}
@include media-breakpoint-down(lg) {
.ng-cc-modal {
min-height: 180px;
max-width: none;
&.cookie-policy-shown {
min-height: 75vh;
.ng-cc-cookie-policy-text {
.wrapper {
padding: 1rem 0;
margin: 0 1rem;
}
}
}
}
}
// /cookie policy addition
}
31 changes: 31 additions & 0 deletions assets/sass/bootstrap_import/_bootstrap_override.scss
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,37 @@
transform: translate(0, -50%);
}
}
&.btn-icon-chevron-left {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand what we have here but I would suggest separating concerns regarding buttons with icons in general and this particular example:

&[class*="btn-icon-"] {
// general rules for any button that has icon in it, like gap, &after rules, aligning etc
}

&.btn-icon-chevron-left {
&:after {
@extend %icon-chevron-left; // only the particular icon rendering
}
}

that way we can easily add new icons in buttons and coloring/styles are not involved only with class btn-icon-chevron-left

&:active,
&.active,
&:not(:disabled):not(.disabled):active,
&:not(:disabled):not(.disabled).active,
&:hover,
&:focus-visible {
&:after {
filter: brightness(0) invert(1);
}
}
&:after {
content: "";
@extend %icon-chevron-left;
width: 1.125rem;
height: 1.125rem;
flex: 0 0 auto;
background-size: contain;
@include media-breakpoint-down(lg) {
width: .875rem;
height: .875rem;
}
}
align-items: center;
gap: 1rem;
display: inline-flex;
flex-direction: row-reverse;
justify-content: flex-end;
padding-left: 1.25rem;
width: auto!important;
}
}

.container {
Expand Down
3 changes: 3 additions & 0 deletions config/app/routes.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ngsite_view_payload:
Copy link
Member

@emodric emodric Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain a bit why this route is used for?

In any case, this should not be located in media-site, but in netgen/site-bundle.

path: /view-payload/{contentId}
controller: App\Controller\ViewPayload
21 changes: 21 additions & 0 deletions src/Controller/ViewPayload.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

declare(strict_types=1);

namespace App\Controller;

use Netgen\Bundle\SiteBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Response;

final class ViewPayload extends Controller
{
public function __invoke(int $contentId): Response
{
return new Response(
$this->getContentRenderer()->renderContent(
$this->getLoadService()->loadContent($contentId),
'payload',
),
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

{% import '@ibexadesign/content/macros/content_fields.html.twig' as content_fields %}

<article class="view-type view-type-{{ view_type }} ng-article vl4">
<article class="view-type view-type-{{ view_type }} ng-article">

{{ content_fields.image(content, location, 'i320') }}

Expand All @@ -17,5 +17,12 @@
<div class="short">
{{ content_fields.intro(content) }}
</div>

{% if content.fields.body is defined and not content.fields.body.empty %}
<div class="body">
{{ ng_render_field(content.fields.body) }}
</div>
{% endif %}

</div>
</article>
20 changes: 19 additions & 1 deletion templates/themes/app/pagelayout/cookie_control.html.twig
Original file line number Diff line number Diff line change
@@ -1,12 +1,30 @@
{% if ngsite.siteInfoContent.hasField('related_cookie_policy') and not ngsite.siteInfoContent.fields.related_cookie_policy.empty %}
{% set cookie_policy = ngsite.siteInfoContent.fieldRelation('related_cookie_policy') %}
{% set cookie_policy_text = cookie_policy.fieldRelation('cookie_policy_text') %}

<div id="ng-cc">
<div class="ng-cc-overlay"></div>

{% if cookie_policy.hasField('cookie_policy_text') and not cookie_policy.fields.cookie_policy_text.empty %}
<div class="ng-cc-modal js-fetch-page" data-fetch-page-content-id="{{ cookie_policy_text.id }}">
{% else %}
<div class="ng-cc-modal">
{% endif %}

{# cookie policy addition #}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for these comments.

<div class="ng-cc-cookie-policy-text">
<div class="wrapper">
<div class="ng-cc-header-in">
<a href="#" class="btn btn-default btn-icon-chevron-left cookie-policy-hide js-fetch-page-remove">{{ 'ngsite.layout.cookie_settings.go_back'|trans }}</a>
</div>
<div class="js-fetch-page-placeholder">
</div>
</div>
</div>
{# /cookie policy addition #}

<div class="ng-cc-content">
<div class="wrapper">
<div class="wrapper ng-cc-js-link-cookie-policy js-fetch-page-trigger-wrapper">
{% if not cookie_policy.fields.ribbon_description.empty %}
{{ ng_render_field(cookie_policy.fields.ribbon_description) }}
{% endif %}
Expand Down
1 change: 1 addition & 0 deletions translations/messages.de.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ ngsite.layout.powered_by: "Unterstützt durch"
ngsite.layout.cookie_settings: "Cookie Einstellungen"
ngsite.layout.cookie_settings.accepted: "Akzeptiert"
ngsite.layout.cookie_settings.not_accepted: "Nicht akzeptiert"
ngsite.layout.cookie_settings.go_back: "Go back"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add german label "Zurück"

ngsite.layout.sponsored: 'Gesponsert'

ngsite.search.placeholder: 'Suche'
Expand Down
1 change: 1 addition & 0 deletions translations/messages.en.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ ngsite.layout.powered_by: "Powered by"
ngsite.layout.cookie_settings: "Cookie settings"
ngsite.layout.cookie_settings.accepted: "Accepted"
ngsite.layout.cookie_settings.not_accepted: "Not accepted"
ngsite.layout.cookie_settings.go_back: "Go back"
ngsite.layout.sponsored: 'Sponsored'

ngsite.search.placeholder: 'Search'
Expand Down