Skip to content

Conversation

nlukic97
Copy link

@nlukic97 nlukic97 commented Jul 28, 2025

Solves #47 and #63.

Added DOMContentLoaded to ensure the cookie consent view is shown after the DOM is fully loaded.

I noticed the merged pull request #86 which ensures window.LaravelCookieConsent is initialized after DOMContentLoaded. With this said, it is still possible for the cookie consent box to be shown before window.LaravelCookieConsent is initialized, which would result in undefined errors when clicking on the cookie consent buttons. This PR solves this scenario.

@toonvandenbos
Copy link
Member

Hi @nlukic97 thanks for your PR. However, I do not see how these changes would solve #47 & #63 since the only thing you do is defer the removal of a CSS class (used for animation) until after the DOMContentLoaded event, which has no impact on the presence of window.LaravelCookieConsent at the time of the initCookies() call.

@nlukic97
Copy link
Author

nlukic97 commented Sep 5, 2025

At the moment, window.LaravelCookieConsent is set after DOMContentLoaded. However, the class responsible for the animation which shows the cookie consent box is removed before DOMContentLoaded. This causes the cookie box window to be visible before DOMContentLoaded, in which case clicking on the "Accept" button would not work, due to window.LaravelCookieConsent being undefined.

Edit:
This issue is most apparent with websites with longer load times/ slow internet connections. Here is a website where clicking immediately on "accept" will not do anything and log an error in the console, throttling the network speed through the chrome dev tools will make this issue more apparent.

I've added an additional commit which, until DOMContentLoaded is fired, sets the display of the cookie consent box to "hidden" rather than just setting its visibility to "transparent". The original visibility animation is still present.

…oaded while retaining the original entry animation
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.

2 participants