Closed karlrez closed 2 years ago
@karlrez I finally I had chance to properly look at your PR. Thank you for the code, it generally looks great! Some small comments:
I agree with using process.env for saving the recaptcha key. So feel free to go on and use it in the code. We will add it to our .env file later on. Remember that with Next.js we need to use the prefix NEXT_PUBLIC to access the public env variable.
So far the recaptcha is applied just to subscriber form. We need to do it to the contact form too.
Is there any way to hide the recaptcha UI? Or at least show it just when the form is visible? O even better show it near the form?
@gsambrotta Thanks for reviewing this! I will update my PR to include the prefix 'NEXT_PUBLIC' for the site key, and I'll also add recaptcha for the contact form.
For the recapcha UI we can hide it, but we are required to display the branding on the forms like so:
What are your thoughts on that? I think it would be the less distracting for the user.
@gsambrotta Thanks for reviewing this! I will update my PR to include the prefix 'NEXT_PUBLIC' for the site key, and I'll also add recaptcha for the contact form.
For the recapcha UI we can hide it, but we are required to display the branding on the forms like so:
What are your thoughts on that? I think it would be the less distracting for the user.
Awesome! This is an ideal solution. Please feel free to proceed with this one :+1:
Hi @gsambrotta , I'm about ready to make this PR but I just want to run the UI by you first:
I feel like it is redundant having the recaptcha branding displayed twice when the forms are very close to each other. Do you have a preference on which one we can remove?
You can check out the Google doc here: https://developers.google.com/recaptcha/docs/faq#id-like-to-hide-the-recaptcha-badge.-what-is-allowed it mentions that the recaptcha branding needs to at least be visable in the user flow.
Thank you very much @karlrez it looks really nice now!
I agree it is redunandant. We can remove the one in the footer and keep the one just belove the contact and big subscribe form.
Apologies, looks like my last commit is not building properly. I am still troubleshooting, but If you can see what the issue is on your end, let me know and I will commit again.
@karlrez please excuse me for the delay in my answer.
I had a good look at your PR, the design is great! I love it.
I change the "FAKE_KEY"
with an env variable with a generate Google reCaptcha Key from our account and unfortunately as soon as I open (so at first reload) the home page and the about page, I get the verification from Google images like in the screenshot here:
Not sure why this shows up..I'm still using recaptcha v2 Invisible. Correct, isn't it?
I create a new branch off from yours, where I simply add the env key. I put the env key in the env.development and I get the screenshot when I simply run the dev server npm run dev
https://github.com/ElliotForWater/elliotforwater.com/tree/add-recaptcha-with-envkey
Outside of that, everything else works good for me. I was able to build the branch on stage and on production.
Hi @gsambrotta, apologies!! I overlooked one detail. I actually had selected Recaptcha v3 when setting up the site key:
v3 seems to work fine with this npm package and doesn't use the image grid.
Alright, seems to work pretty well with v3 :D Thanks!
Can you please change the value siteKey
to {process.env.NEXT_PUBLIC_RECAPTCHA_KEY}
? And after that it should be good to go.
As I said, I had no problem to build it, please pull in the last changes from develop and try again to run npm run build:stage
If you still have problem, feel free to just push it and I will have a look at it
Many thanks again @karlrez :tada:
@karlrez seems there is some problem with ESLint that doesn't allow you to pass the build step. From our CI Failed to compile.
./components/Forms/Contact/ContactForm.tsx
193:17 Error: Replace `␍⏎········ref={recaptchaRef}␍⏎········size='invisible'␍⏎········sitekey={process.env.NEXT_PUBLIC_RECAPTCHA_SITE_KEY}␍⏎·····` with `·ref={recaptchaRef}·size='invisible'·sitekey={process.env.NEXT_PUBLIC_RECAPTCHA_SITE_KEY}` prettier/prettier
./components/Forms/Subscribe/SubscribeForm.tsx
198:17 Error: Replace `␍⏎········ref={recaptchaRef}␍⏎········size='invisible'␍⏎········sitekey={process.env.NEXT_PUBLIC_RECAPTCHA_SITE_KEY}␍⏎·····` with `·ref={recaptchaRef}·size='invisible'·sitekey={process.env.NEXT_PUBLIC_RECAPTCHA_SITE_KEY}` prettier/prettier
Do you maybe have some ESLint config that override our own? Or maybe we forgot to add some in the config file and we added just to the visual studio code file... Not sure.
I didnt have my tslint set up for typescript previously, so I enabled it and I was able to see the same eslint errors and correct them. Is it still showing an issue with eslint from my last push?
Ah great! Now the eslint is ok , the problem is in npm run test
Warning: Failed prop type: The prop `sitekey` is marked as required in `ReCAPTCHA`, but its value is `undefined`.
at ReCAPTCHA (D:\a\1\s\node_modules\react-google-recaptcha\lib\recaptcha.js:28:30)
at AsyncScriptLoader (D:\a\1\s\node_modules\react-async-script\lib\async-script-loader.js:37:28)
at AsyncScriptLoader(ReCAPTCHA)
at FormProvider (D:\a\1\s\node_modules\react-hook-form\src\useFormContext.tsx:14:3)
at SubscribeForm (D:\a\1\s\components\Forms\Subscribe\SubscribeForm.tsx:22:26)
at div
at footer
at Footer (D:\a\1\s\components\Footer\Footer.tsx:7:17)
at WrapperComponent (D:\a\1\s\node_modules\enzyme-adapter-utils\src\createMountWrapper.jsx:49:26)
If you run on your local npm run build:stage
you should be able to see if it build without error.
Maybe you miss some env config to build on your local, I would be interested to know if you can build:stage or build:prod on your local and make it through.
On my local I am able to run npm run build:stage
and prod without any errors coming up.
npm run test
shows good output:
I just wasn't scrolling up to see the error with the site key. I'll keeping poking around at this to figure out how to get the .env files to work with next and jest.
Unrelated to the site key, I get this one when running tests as well:
This seems to show for multiple tests, so I'll try figure something out for that one as well.
I just wasn't scrolling up to see the error with the site key. I'll keeping poking around at this to figure out how to get the .env files to work with next and jest.
In your test file, you can simply put any mock value in the siteKey prop of the recaptcha component. Like:
<reCAPTCHA siteKey="7392792739">
Unrelated to the site key, I get this one when running tests as well:
mhmm I already encounter this warning in other projects.. I don't remember exactly, google should be your good friend here :D but i think is something related to configuration rather then the tests in itself
A bit of a workaround, but things worked out a lot more smoothly with the v3 package: https://www.npmjs.com/package/react-google-recaptcha-v3
Works similiar to the v2 package but I did make a few code changes. This package gives us a few more options, and we might need to set the useEnterprise option if you're set up with that type of site key.
@karlrez Looks and works great now Thank you so much for your contribution and the time spent on the project
@karlrez Looks and works great now Thank you so much for your contribution and the time spent on the project
@gsambrotta Happy to contribute! Thank you as well for your time, was great working with you on this.
Related Issue
Resolves #47
Description
Added Google Recaptcha v2 to subscribe forms.
Impacted Areas in Application
With a valid Recaptcha site key, verification will show:
Without Recaptcha verification, form will not submit and an error toast will show:
When form is submitted successfully, the recaptcha token is also sent with the form data:
Docs for verifying token on the server: https://developers.google.com/recaptcha/docs/verify
Steps to Test or Reproduce
Sign into google and get a site key. Make sure to add 'localhost' as a domain: https://www.google.com/recaptcha/admin/create
Replace 'FAKE KEY' with your site key:
Alternatively, we could add the Site key to our .env.development file
Related PRs
n/a
PR Checklist:
npm run test
and be sure all test passnpm run build:dev
and be sure no error blovk the build