-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
Implement Invisible (v3) ReCaptcha #21
Conversation
I see https://product.hubspot.com/blog/quick-guide-invisible-recaptcha that I will must do more changes, pls no close, I will do rebase ;) |
Could you separate it from basic recaptcha? I would like to have ad.
|
@mabar yes Its in plan with refactoring. |
@mabar should be problem add js to docs ? (it will work with nittro too - iam testing with nittro). |
public function setMessage($message) | ||
{ | ||
if ($this->configured === TRUE) { | ||
throw new InvalidStateException('Please call setMessage() only once or don\'t pass $message over addReCaptcha()'); |
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.
addInvisibleReCaptcha
I like that. Nice work. But I am curious if it could be even better. Did you think about an extension for netteForms.js? |
Btw, please be sure that tests and qa pass. We have composer scripts for that https://github.com/contributte/reCAPTCHA/blob/master/composer.json#L36-L42 If you are using Windows so I recommend you give a try to Linux subsystem (you can get Ubuntu from Windows store), because these scripts are not compatible with Windows. |
@mabar I dont use netteForms, I'm using nittro, but can do that ;). It should be in docs or on gist or some another repository ? |
@JanGalek I think you do and just don't know about it. netteForms.js is included in nittro :) Please create |
@mabar changed, I'm not sure, if I added extension right. |
Hi @JanGalek. Look's good to me. Have you test it? Could you please include a screenshot? You can include it into docs. |
@f3l1x I tested on sandbox, now I'm going to test in my project with nittro. You can try it at https://test.gcore.cz/ (sandbox, not really validation username and password, you can write what you want). |
tests Fix cs Refactoring CS Fixed compatibility with nittro Added screenshot invisible recaptcha
@f3l1x ok I must little changed Nette form extension for compatibility with nittro. And screenshot was added. and small bonus, it automatic works with multiple forms which are on site without any manual changes ;) |
Cool. What about the image in the right corner? It's required by rules by Google? It must be there? |
* @author Milan Felix Sulc <[email protected]> | Jan Galek <[email protected]> | ||
*/ | ||
class InvisibleReCaptchaField extends HiddenField | ||
{ |
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.
What about create BaseReCaptcha field? It looks very similar to our other object.
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.
@f3l1x yes, but every field object extends from another object, should be it trait ?
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.
Oh, I didn't notice. Just keep it. :-) It's ok.
@f3l1x yes this must be here.
|
Cool. Thank you for the great job. |
Do you wanna join us at @contributte? I think it would be great to have you. |
@f3l1x yes, it would be a pleasure, I will write you to slack (pehapkari) |
Good job. I'm Looking forward to 3.1 release. |
resolve #12
I'm not sure with field must be required,
Can be some test as bot ?
Maybe should will be create new type of field ?