🇩🇪Germany @sascha_meissner

sascha_meissner → created an issue.

Thx @scott_euser for your work.
I did also improve the placeholders, adding available arguments.

Maybe the description texts still need a little bit work, personally i more like the proposal from #17

@scott_euser AFAIS,

- onInit will run on every load regardless of the consent, it is not possible to check to consent state in this callback as klaro is not fully initialized.

onAccept and onDecline will also run on every load, but based on the (implicit) consent, as well as when consent changes on runtime (explicit).

So you would have to move your code accordingly, probably you wont need the onInit callback at all.

Still you are right, the placeholders for the callbacks should be more explanative and should also show the available arguments, because , even though not explicitly mentioned in the upstream klaro-js docs, there are some:

- onInit (opts) opts.config, opts.service, opts.vars

- onAccept/onDecline(opts) opts.config, opts.consents, opts.confirmed, opts.service, opts.vars

// true || false
klaro.getManager().getConsent(opts.service.name)

for me, adding the urls to an existing and _enabled_ service works.

Maybe it would even be more clean if also disabled services are checked, because even if the service is not enabled, it is known?

Thx for reporting @deuteron
I guess this makes totally sense, so far I created a MR adding br to the allowed tags and increasing the maxlength to 999.
In order to keep the schema to string i have to set a maxlength or it defaults to 128. Tested my change with existing data without problems.

"edge case" 😂

thx for testing this!

Thx for reporting this @steffenr
And sorry for the fix to take some time ...

Thanks rkoller for your feedback and engagement.
I´m sure we can adjust the behaviour as expected once we know what this would be :)
I´m happy it works consistently now... the bug with edge macos and VO i realy cant explain and it doesnt realy make sense.
But also i guess people using this combination is only a few. I mean the total usuage share for edge is below 5% including microsoft users, i dont even find data about macOs users with edge, so i would be fine living with that. Does this bug only occur in this MR100 ?

Fixed

Fixed

sascha_meissner → made their first commit to this issue’s fork.

sascha_meissner → created an issue.

Unfortunatly i dont have a macOS at the moment to test voiceover.
But i made a little changes to make it more like the drupal core modals, so that you tab from first to last and from last to first.
I also made some little changes how the "functional tab handles" are displayed and also added a text-content, hoping this might fix something for voiceover....

For me the behaviour using tab navigation is now the same in firefox/chrome/edge , also using nvda.

Thank you rkoller for testing this!

IMHO the best option would be to use the <dialog> element, but this would be something rather to be implemented within the klaro-js library. I´m going to go through your feedback and try to improve the behaviour. It´s just a quickshot solution so far i just tested in nvda and firefox, so this feedback is very very helpful.

I added a "focus trap" to the klaro dialog using some javascript that will add two functional tabbable elements at first and last position, if you focus them, the focus will be set accordingly to the actual first or last tabbable element. This way your focus is trapped within the dialog.
Please review

sascha_meissner → made their first commit to this issue’s fork.

sascha_meissner → made their first commit to this issue’s fork.

i wasnt able to reproduce the issue out of the box. Did you somehow manually removed the configuration?

Reviewed the tests, looking good to me :)

Works

I couldn't find something similar in Klaro yet?

FYI Within the General settings there is an option "Show toggle button" which adds a toggle in the bottom right.
Maybe you did oversee that? Still i see no reason why not adding `menu_links_discovered_alter` as well..

Simplified the code + added additional selector ".klaroLink"

tested with:

<button class="klaroLink">test</button> 
<a href="#klaro">test2</a> 
<a rel="open-consent-manager">test3</a>

works fine

sascha_meissner → made their first commit to this issue’s fork.

Just for referrence, How EU cookie compliance does it: https://git.drupalcode.org/project/eu-cookie-compliance/-/blob/8.x-1.x/e... (geo lookup for the ip)

In case we want to add this feature i suggest to make a veeery lightweight decision based on the users timezone.
Which does not involve processing potencially sensitive data.
Could be as simple as

Intl.DateTimeFormat().resolvedOptions().timeZone.contains("Europe")

sascha_meissner → made their first commit to this issue’s fork.

Hey @amourow, thx for reporting and patching this, TBH when i first looked at it i didnt really understand or wasnt realy sure about what the implications could possibly be. Now i reviewed and tested thoroughly and i can say it fixes the issue and it seems not to introduce new ones ;) So we try to get this merged soon

Reviewed and throughly tested

sascha_meissner → made their first commit to this issue’s fork.

Added a config to the config/install from a project where we use klaro and simple_popup_blocks that has already the correct cookie regexes and a callback code the will remove popups accordingly, please review

sascha_meissner → made their first commit to this issue’s fork.

IMHO it doesnt realy make sense to to add contextual consent for outgoing links, for OP´s example

  <a data-name="chatbot" data-href="https://example.com/live-chat-url">
    Chat with Support
  </a>

Consent would have to be obtained on example.com, as clicking a link will not load something into the current page/host.

For relative links on the same host it doesnt really make sense to me either.

+ in the klaro-js upstream lib it was more or less a bug that anchor tags are also handled
But maybe i am overseeing something here.

Thx @anybody, correct, attribution should be given by default. AFAIR i was too lazy to add it to install as the result would be the same ;) thx for making this complete. I tested this throughly with mates so i am gonna set this to RTBC now to get this merged :)

Reviewed and tested, Looking great for me :)

Reviewed, looking good for me, setting RTBC

As suggested, i added a checkbox under "Settings->Styling" to set the library config "disable_powered_by" and reverted the changes to the text setting. Please review

Wow i´ve been off for 2 months family time and did not expect THIS to happen 😍
Thank you so much for following up on this @ressa @grevil @dydave @thomas.frobieter and everybody else on this

Thank you @ressa !

So i already putted this into a submodule for now :)
IDK whether this feature will ever make its way into the module, but personally i still like it and have the personal motivation to at least leave this clean because it´s the first contribution i ever created when i started with drupal :P

Sry guys, i started this but didnt find the time to come back to finish.

IMHO there is two things left that need work

1. Put this into a submodule, so it is better isolated, optionally and probably preferred by the maintainers.
2. Have an indicator that your toolbar is currently not displayed somewhere.

FYI: I just tested the vanilla library and have the same result, the contextual element is shown instead of the resource
Created an upstream issue https://github.com/klaro-org/klaro-js/issues/536

FYI: Initially it was build so that when under configuration->texts->consent dialog -> Powered By nothing is entered, it would not show the powered by, This seem to not work for a long time (since klaro_js upstream made a change that would display a "translation missing" if that was empty)

So i made a simple fix putting that back to work, the setting `disablePoweredBy` will be set to TRUE if there is nothing entered in the texts config, please test and review my change

After looking again over this together with Jan Kellermann

We found out that it does not work for elements with contextual consent elements, the ressource gets loaded but the contextual element is still shown.
So i´m setting this back to needs work ...!

Thank you kensae for your contribution!

I reviewed the issue and tested the code, looking great, works for me!

Fixed the flaw i mentioned above, setting back to needs review. But from my side this is looking very good right now

I reviewed the issue and the work already done.

I found only one thing that still needs work:

If I set the option Styling->Show title in notice dialog. The title will still not be displayed in the notice as the css that hides it still applies.

I´ll set the status to needs work and will try to make a fix now

I reviewed this issue with the following result:

I can reproduce the issue and solve it by just setting "Toggled by default" as well.

The codechange in drupal.js is unnecessary and just leads to execute the script twice on load.
I´ll update the MR accordingly.

@itamair, i tested my example that produced the error with 10.2.39 release and the error is gone!
Thank you very much

@yogesh For me it didnt help, also i have different markers and icons with different sizes on the same map :/

i create them like so:

    let markerIcon = L.Icon.extend({
      options: {
        iconSize: [35, 48],
        shadowSize: [0, 0],
        iconAnchor: [17, 48],
        shadowAnchor: [0, 0],
      },
    });

and get the same error Error: Invalid LatLng object: (NaN, NaN) on clicking them ... while everything works though, its just printing in the console, it doesnt stop execution or so

jan kellermann → credited sascha_meissner → .

+1 Just stumbled into this ... IMHO it makes more sense to prevent completing multiple values client side in the first place. Just adding serverside validation is good but will just raise questionmarks in the opposite direction.

Created a MR for convenience ;-)

So please ignore my patches from #11 and #12

i made a MR against 11.x-dev based on the patches

I´m sorry for just rerolling this patch in a deprecated way ... here is a rerolled patch for 10.3.10

jan kellermann → credited sascha_meissner → .

IMHO ist das Modul selber in allen Sprachen ein "Consent Manager"

Bzgl. der Bennenung der beiden Elemente noch der Hinweis:

Das "notice-element" kann selber auch als Dialog (options.showNoticeAsModal) dargestellt werden, das ist in der Klaro lib selber schon nicht ganz Eindeutig, also bitte nichts mit "-Dialog" ^^

Diese Benennungen werden ja ohnehin nur im administer-Bereich angezeigt, hauptsache die Unterscheidung ist klar, IMHO kann das "Einwilligen/Einwilligung" hier komplett raus, weil worum soll es sonst gehen...?

Ich würde Vorschlagen: "Verwaltungs Element" und "Hinweis Element"

jan kellermann → credited sascha_meissner → .

jan kellermann → credited sascha_meissner → .

works

Reviewed and tested the changes, works good for me, great!

@shortspoken FYI: An unknown source is only logged when the page with it is requested, so you would have to request all pages of your site to find all unknown resources (or let this do a tool like a crawler/spider)

FYI:
The autofocus option is always set to true since 3.0.0-rc7
https://git.drupalcode.org/project/klaro/-/blob/3.x/js/klaro.drupal.js#L28

Had a hard time reproducing the issue and finally found out that it only happens when having twig.config.debug=true and it happens since Drupal added utf-8 icons to the twig debug comments, which must have been a recent update.

both, jan´s and jurgen´s code fix the display of the specialchars in the browser, but also both fail to encode the icons in the twig-debug-comments back correctly :(

I have mixed feelings as the root cause is obviously a bug in the js library as described in the bottom of this issue here https://github.com/klaro-org/klaro-js/issues/503 and we cannot be sure if the area-labelledy will be just ommited when showNoticeTitle=false or if they also go with visually hiding the title when fixing this eventually. So i would propose to support the option showNoticeTitle (not set the default to true though) and the label for the notice title in the settingsform/config, but just remove the area-labelledby attribute in the already existent "accessibilityducktape" in the js-file of this drupal module if showNoticeTitle=false.

This way it would be easier to adopt to a upstream library change imho + we wouldnt have to care of an updatehook and css.
?

Nice to have: The Klaro-Block should be not reusable/only addable once?!

I like the implementation using a block.

I fixed a bug where it would append "null" to the block as well (because #klaro is not existent at that moment yet) + i also added the togglebutton to the block as well and added tabindex=1 to it, so it stays one of the firsts elements to be tabbed to even if the block is placed at the bottom.

Further i experienced several "z-index bugs" depending on which region you add the block, because the css was pretty much expecting the klaro block to be at the end of body, so i´m not sure whether this needs work in the module or if its up to the user then to style that region in their layout?

sascha_meissner → made their first commit to this issue’s fork.

Hey Monaw,

disabling filters in the way the html attribute "disabled" would do for a "vanilla html input" is not supported by this module, but that is due to the fact that exposed filters and their html-structure can also be extended or manipulated in any thinkable way.

Though you can easily hide/remove the filters with this module accordingly so that only 1 of the three filters can be chosen at a time.

Lets assume your "active value" is the string "on" and dff1=compound,dff2=mass,dff3=formula
then, for example, you could use the neq condition (not equals) on the compound admin lable like:

dff2|{neq:on}|AND|dff3|{neq:on}|

This way the compound filter will only be shown if neither mass or formula is selected. You can use the same logic for the mass and formula input as well and achieve your desired behaviour.

PS: Sry for the late reply, i´m on a longer family getaway, hope my answer still helps you

Thank you van.dordafog for your contribution, i´ll have a look into this soon.

Added a little context-filter testing to testStringSearch of LocaleTranslationUiTest

Oops, wanted to rebase current 11.x but messed up, i´ll fix this now

sascha_meissner → made their first commit to this issue’s fork.

@wim-leers

Thank you for also putting efforts into this, it´s been some time and we finally convinced our customer and their seo-agents to drop this feature due to the already mentioned lack of cross-plattform a11y (Even though i personally also like this as a desktop user) IMHO this should be an option, but not provided by the drupal implementation, rather shipped with CKE.

I´ll close this for now

The latest klaro release (rc6) included some improvements related to translations,
could you maybe check if you can reproduce your issue with a fresh install?

Added translation context "klaro" to all translatable strings

Will be included in RC-7!

Will be included in RC-7!

Will be included in RC-7!

Will be included in RC-7!

Will be included in RC-7

Will be included in RC-7

Its an already fixed and duplicate of https://git.drupalcode.org/project/klaro/-/commit/71e920dbfb923cee3008ac...

sascha_meissner → created an issue.