-
Notifications
You must be signed in to change notification settings - Fork 69
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
Recaptcha initialization #191
Conversation
… for include Google API js file - Admin panel
…a Frontend, all types, set param hl in call to render.
…a Admin panel, all types. Reformat code.
@@ -50,16 +50,19 @@ define( | |||
$(window).trigger('recaptchaapiready'); | |||
}.bind(this); | |||
|
|||
element = document.createElement('script'); | |||
scriptTag = document.getElementsByTagName('script')[0]; | |||
if (window.addedScriptTag === undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you rename variable to make it not too general?
if (window.addedScriptTag === undefined) { | |
if (window.addedRecaptchaScriptTag === undefined) { |
Or maybe even better - just add variable inside this JS file and not leak it to window at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ihor-sviziev this variable should be global.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we can replace global variable on new shared service.
The reCaptcha
component will have dependency (via "import" by path) on this shared service.
Inside of the new service adding script tag will proceed only one time.
Thanks
81ffeeb
to
fb34279
Compare
fb34279
to
2a64eee
Compare
Co-Authored-By: Ihor Sviziev <[email protected]>
Description (*)
This PR accumulates commits from #183, #176 and #173
Fixed Issues (if relevant)
#21
Manual testing scenarios (*)
Questions or comments
Contribution checklist (*)