Skip to content
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

Merged
merged 1 commit into from
Aug 16, 2018
Merged

Implement Invisible (v3) ReCaptcha #21

merged 1 commit into from
Aug 16, 2018

Conversation

JanGalek
Copy link
Contributor

@JanGalek JanGalek commented Aug 6, 2018

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 ?

@JanGalek
Copy link
Contributor Author

JanGalek commented Aug 7, 2018

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 ;)

@mabar
Copy link
Contributor

mabar commented Aug 7, 2018

Could you separate it from basic recaptcha? I would like to have addReCaptcha and addInvisibleReCaptcha. Their use case is diferent and I can imagine that I need both of them.

ad. data-callback

  • It's a diferent feature, please create separate PR for it.
  • If I understand, it's a name of javascript function. It is view-specific and should not be in global configuration. You can set it to control same way as other atributes.

@JanGalek
Copy link
Contributor Author

JanGalek commented Aug 7, 2018

@mabar yes Its in plan with refactoring.

@JanGalek
Copy link
Contributor Author

JanGalek commented Aug 8, 2018

@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()');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addInvisibleReCaptcha

@mabar
Copy link
Contributor

mabar commented Aug 8, 2018

I like that. Nice work.
I think javascript snippet is the only way here. But it could be done without jquery (example in recaptcha docs should be enough)

But I am curious if it could be even better. Did you think about an extension for netteForms.js?

@mabar
Copy link
Contributor

mabar commented Aug 8, 2018

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
Coding style could be (partialy) fixed with vendor/bin/codefixer

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.

@JanGalek
Copy link
Contributor Author

JanGalek commented Aug 8, 2018

@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 ?

@mabar
Copy link
Contributor

mabar commented Aug 8, 2018

@JanGalek I think you do and just don't know about it. netteForms.js is included in nittro :)
https://github.com/nittro/nittro/wiki/Forms#a-note-about-netteformsjs

Please create assets/invisibleRecaptcha.js and add note (and link to it) in docs.

@JanGalek
Copy link
Contributor Author

JanGalek commented Aug 9, 2018

@mabar changed, I'm not sure, if I added extension right.

@f3l1x
Copy link
Member

f3l1x commented Aug 9, 2018

Hi @JanGalek. Look's good to me. Have you test it? Could you please include a screenshot? You can include it into docs.

@JanGalek
Copy link
Contributor Author

JanGalek commented Aug 9, 2018

@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
@JanGalek
Copy link
Contributor Author

JanGalek commented Aug 9, 2018

@f3l1x ok I must little changed Nette form extension for compatibility with nittro. And screenshot was added.

reCAPTCHA

and small bonus, it automatic works with multiple forms which are on site without any manual changes ;)

@f3l1x
Copy link
Member

f3l1x commented Aug 10, 2018

Cool. What about the image in the right corner? It's required by rules by Google? It must be there?

* @author Milan Felix Sulc <sulcmil@gmail.com> | Jan Galek <jan.galek@troia-studio.cz>
*/
class InvisibleReCaptchaField extends HiddenField
{
Copy link
Member

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.

Copy link
Contributor Author

@JanGalek JanGalek Aug 10, 2018

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 ?

Copy link
Member

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.

@JanGalek
Copy link
Contributor Author

@f3l1x yes this must be here.

As Google captures user information from your website, you'll need to warn your users by displaying their Privacy policy and Terms and conditions.

@JanGalek
Copy link
Contributor Author

@f3l1x @mabar ping

@f3l1x
Copy link
Member

f3l1x commented Aug 16, 2018

Cool. Thank you for the great job.

@f3l1x f3l1x merged commit 11844b8 into contributte:master Aug 16, 2018
@f3l1x
Copy link
Member

f3l1x commented Aug 16, 2018

Do you wanna join us at @contributte? I think it would be great to have you.

@f3l1x f3l1x added this to the v3.1.0 milestone Aug 16, 2018
@JanGalek
Copy link
Contributor Author

@f3l1x yes, it would be a pleasure, I will write you to slack (pehapkari)

@natocTo
Copy link
Contributor

natocTo commented Aug 19, 2018

Good job. I'm Looking forward to 3.1 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Feature: invisible recaptcha
4 participants