I moved the button positioning back into the js - there were some issues applying the styling on time with custom classes added so this simplified things for me.
For those who would like to make use of the patch despite the fact that it will not be included in a release - I have created and attached a more generic solution that has much less impact on the original code. The tradeoff is that you need to handle styling in your local theme for it to display perfectly. This is based on this approach to handle a throbber within a button - https://www.linkedin.com/posts/wesbos_how-do-you-make-a-button-that-does.... Here is a simple scss example of how it could be implemented:
&.submit-button {
display: grid;
grid-template-areas: "stack";
& .text {
grid-area: stack;
}
&.submit-clicked {
& .throbber {
grid-area: stack;
visibility: visible;
position: absolute;
top: 50%;
left: 50%;
transform: translate(-50%, -50%);
}
& .text {
visibility: hidden;
}
}
}
Here is an updated patch that adds the aria-disabled=true attribute to the button when it is clicked for improved accessibility.
This site uses a custom theme. I did look into the theme to see if this throbber was being controlled and styled there first but I found that the Webform module was handling this so my conclusion was that it should be updated in this contrib module. Of course this is an aesthetic change so I do not expect that everyone would find this to be the best solution, however, if the default implementation simply places the throbber after the button (see the $clickedButton.after(Drupal.theme.ajaxProgressThrobber()); line of code) then I think this is certainly a cleaner implementation. Still, this is open for debate though, one of the advantages of working on open source code like this is that we can improve things together to find better solutions.
I have attached a video of the current behaviour and a screenshot of the current throbber placement as well as screenshots of the proposed solutions found in patch 2 and 3 respectively.
Current throbber placement:
Patch 2 solution:
Patch 3 solution:
I have created an alternative solution that overlays the throbber on top of the button text so the label is not removed, the patch is attached.
The proposed patch is attached as promised.
roaldnel → created an issue.
The patch is attached.
roaldnel → created an issue.
I found the same as andreic, the code updates do not apply on 3.x-dev@dev or 8.x-3.6 for us.
roaldnel → made their first commit to this issue’s fork.
I have reviewed the merge request and the code updates do what they are supposed to do so I am happy to move forward with merging the code.
We are also seeing this error pop up every couple of minutes in our drupal-logs for at least one of our websites. This certainly seems like something that needs to be resolved.
While working with the patch applied we have noticed the following:
- The popup does not appear on the admin theme.
- The problem also occurs if you work on 2 Chrome windows. As soon as you have 2 windows open, you will only receive a notification in on of the windows and not the other, and you are logged out in the background.
We have increasingly received requests from clients to add the script in a different format presumably constructed by GTM as such I have further updated the code changes that this issue brought about to apply this new format. The primary change can be found in lines 275 to 306 of the src/Entity/Container.php file. The rest of the changes were mainly cleaning up the code. I have attached a patch with the proposed changes, any input or improvements are welcome.
The patch is attached as promised.
roaldnel → created an issue.
Hi Summit, in some of our projects we also used the EU Cookie Compliance module in combination with the EU Cookie Compliance GTM module - https://www.drupal.org/project/eu_cookie_compliance_gtm → and I also created a patch (which you should then include) that will add the new Google Consent Mode V2 consent settings - https://www.drupal.org/project/eu_cookie_compliance_gtm/issues/3332626 ✨ Make the module compatible with Google Consent Mode RTBC . This would be the simplest route.
I have added a readme file to the project in the .md format and released it in version 1.0.4.
This code was merged and released under version 1.0.1.
batigolix → credited roaldnel → .
Thank you for reviewing the change yustinTR. I have committed the changes and released version 1.0.17 with the updated code.
If you prefer to use a standalone GTM module I would suggest making use of the Cookiebot standalone module - https://www.drupal.org/project/cookiebot → .
The main aim of this Cookiebot + GTM module is to combine both Cookiebot and GTM as the name suggests so users of the module use it for this purpose. Since there are existing alternatives you can use to set up GTM and Cookiebot separately it does not really make much sense to spend resources to adapt this module do the same. I trust you can understand this.
I know this has been closed but I encountered the same issue when I updated one of our servers from 1.6 to 1.7. Did anyone find a solution to this issue?
The patch is attached as promised.
roaldnel → created an issue.
The patch is attached.
roaldnel → created an issue.
This module needs a detailed readme file, just updating it to a readme.md file will not do much to enhance the user experience. Nonetheless, this issue can be used to update and track readme improvements.
The package entry is not required and was removed in version 1.0.3.
This has been added to release 1.0.3.
The info file was updated to include version 11 in release 1.0.16.
This has now been released in version 1.0.16.
We have tested this patch on a variety of projects and based on the feedback from fotisp this is now considered to have been reviewed and tested.
I have created an updated patch the applies the changes to the 2.x code base. Similar to post #5 by gebiss, Google Consent Mode v2 tags can also be added as their own categories or I simply added them under the marketing category as a collection which applies the access as expected based on the selection made by the user for "Marketing" cookies:
{
"ad_personalization": "@status",
"ad_storage": "@status",
"ad_user_data": "@status"
}
I have updated the patch to update the script to Google Consent Mode v2 (see https://www.cookiebot.com/en/google-consent-mode-v2-integration/).
Thank you for your feedback, this really helps. I have created an improved patch that strips out the changes that were introduced in v1.0.8 (see https://www.drupal.org/project/cookiebot_gtm/issues/3367888 📌 GTM needs to load before the Cookiebot script Fixed ) so this specific feature can be tested in isolation. I also added an enabled / disabled selector to the config form that way it can be turned off if need be. The patch is attached.
The main issue was that our client was complaining that their GTM stats were not working as expected. Until now I have not had too much time to investigate this in too much detail but I suspect that the more strict blocking was functioning as it should but it was just not the expected result. That being said, I tend to agree with you that it does what you would expect. Regardless, now that the functionality can easily be disabled I think we can give it another go so let's see how this patch fares and hopefully we can include it in a release asap.
I have updated this task to needs work.
Drupal 10 compatibility has been implemented and released in a previous issue - https://www.drupal.org/project/cookiebot_gtm/issues/3286780 📌 Automated Drupal 10 compatibility fixes Fixed . I think this can be closed now.
Thank you for your input fotisp. If you read the motivation behind the reversal of the code changes here - https://www.drupal.org/project/cookiebot_gtm/issues/3381424 🐛 Reordering of script loading (v1.0.9) breaks Google Analytics Fixed , you should note that the patches introduced issues with Google Analytics (GA). Unless we can implement these changes in a way that does not negatively impact GA we cannot re-introduce this into the production code. If you have suggestions to adapt the previous patch in a way that can accommodate GA then you are welcome to post it here. Still, this is a good ticket that we can use to explore options on how to resolve this issue.
I rerolled the patch since the previous one could not be applied any longer.
The code has been merged and Drupal 8 support was dropped.
Added D10 support back in since it has been released already, the updated patch is attached.
And here is the updated patch
Thanks Arantxio, I have removed the D10 support change and I also removed support for Drupal 8 as you suggested in this issue: https://www.drupal.org/project/cm_commerce/issues/3386410#comment-15332689 📌 Drupal 10 update Needs work .
Thank you Arantxio, I will remove Drupal 8 support in the release for this Issue: https://www.drupal.org/project/cm_commerce/issues/3393194 📌 Updated processing and error handling Needs work .
The patch has been applied and the code has been merged so this task is now considered as fixed.
Due to the nature of how the composer file seems to be pulled in the patch could not be applied. This also explains why the initial patch to add the composer file did not cause any issues during testing. This was committed directly in the codebase and a new release was created.
The proposed patch is attached.
roaldnel → created an issue.
@jsacksick It seems like the code was committed but it does not get pulled through via composer. Can you possibly create a new release for the module with the updated code for the community to use? Thanks in advance!
We have been using this patch in production for a while now and it works. Can this be merged soon?
I noticed that now as well, here is a new upload.
Here is the patch for version 2.
roaldnel → created an issue.
Here is the proposed patch.
roaldnel → created an issue.
We have also been using this in production for a while. Can this please be merged? Thanks!
This patch works as intended to make the module compatible with Drupal 10. Can a new release be made to officially make the module Drupal 10 compatible? Drupal 10 has been around for a while now so I am sure everyone using this module would appreciate it.
The patch was updated with codesniffer improvements applied.
Updated the status to needs review.
The patch containing the updated code is attached.
roaldnel → created an issue.
This update stemmed from changes made in a custom version of this module we used in one of our projects. The changes are more extensive and the focus is wider that described in this issue so this issue will be replaced by a more appropriate one. This ticket is now considered closed.
Further changes were required, a new patch is attached.
The proposed patch is attached.
roaldnel → created an issue.
Yes! miimooo, this is exactly the route I went to get this to work as a custom module for one of our projects, I think this is a great solution!
I have added a new custom field to the configuration form so users can now set custom URLs for the google tag manager server. The code was released in version 1.0.15.
I have tested the latest patch and I also applied changes as suggested by batigolix. The code was released in version 1.0.14.
The latest patch was applied and the code was released in version 1.0.13.
The proposed patch is attached.
roaldnel → created an issue.
Released in version 1.0.1.
Released in version 1.0.2.
The composer file has been committed and pushed to the codebase.
The patch was tested and everything seems to be working as expected.
The updated code can be found in the attached patch.
roaldnel → created an issue.
Here is an updated patch. I have added checks to apply the correct text to the different storage options.
The proposed patch as promised in the issue details.
roaldnel → created an issue.
We also encountered the same issue after upgrading our project to Drupal 10. We would appreciate it if someone could look into this, thanks in advance!
roaldnel → created an issue.
I have created an updated patch. A new translation icon was added to the icons library and the message text and icon was updated. The latest patch is attached.
Esther Tempel was the developer that laid the groundwork for this patch before I started working on it - https://www.drupal.org/u/esthertempel → .
The patch to implement this change is attached.
roaldnel → created an issue.
Okay great, thank you, I am applying the patch.
See the patch below to ensure that GTM is loaded before the Cookiebot script in the head.
roaldnel → created an issue.
We would appreciate it if the merge request above by bletch could be approved and merged as soon as possible. The fix seems to be effective.
I just encountered the same problem as idiaz.roncero above regarding guzzle. Is there a possibility that this could be fixed?