Ngstack 833 cookie consent cookie policy improvements new#210
Conversation
| .ng-cc-cookie-policy-text { | ||
| position: absolute; | ||
| inset: 0; | ||
| z-index: 10000000; |
There was a problem hiding this comment.
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/
There was a problem hiding this comment.
Yes, it was not enough, unfortunately toggle buttons are leaking on the front of page due to this fix
Not ideal of course. Please suggest any solution if you have one.
| z-index: 10000000; | ||
| transition: transform .3s ease; | ||
| transform: translateX(100%); | ||
| background: #fff; |
There was a problem hiding this comment.
Added sass variable, please notice that this whole file has few other hex defined colors in it.
| <div class="ng-cc-modal"> | ||
| {% endif %} | ||
|
|
||
| {# cookie policy addition #} |
| transform: translate(0, -50%); | ||
| } | ||
| } | ||
| &.btn-icon-chevron-left { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Changed like this and added few more tweaks to support button combinations.
| @@ -0,0 +1,3 @@ | |||
| ngsite_view_payload: | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This was used to activate payload I guess. Was not working without it. Please suggest me how to do it? The same file and line of code, just to do it inside netgen/site-bundle?
There was a problem hiding this comment.
If this is used exclusively to display the cookie policy, then the controller and route should reflect this. You don't need to pass the content ID around when the controller can just load the cookie policy text by itself and output it.
Besides, having this generic controller take ANY content ID and just display the content can be a security issue.
| 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" |
There was a problem hiding this comment.
Please add german label "Zurück"
| rotateArrowClass: 'rotate-arrow', | ||
| shownClass: 'shown', | ||
| cookieModal: '.ng-cc-modal', | ||
| cookiePolicyShow: '.ng-cc-js-link-cookie-policy a', |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Also, a small nitpicker: can this key not be cookiePolicyShow but cookiePolicyShowTrigger?
There was a problem hiding this comment.
Not sure is this posible at least in easy way because link iz inside content text. Not sure is it worth doing some magic stuff in my opinion.
Changed the name of cookiePolicyShow variable.
mlukac
force-pushed
the
NGSTACK-833-cookie-consent-cookie-policy-improvements-new
branch
from
a705763 to
2348b0a
Compare
3 participants
No description provided.