-
Notifications
You must be signed in to change notification settings - Fork 145
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
adding-lang-to-recaptcha #141
Conversation
adding documentation
adding proper link
typo
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.
@padma-vemuri I can't find the hl
as a parameter to render
documented anywhere, https://developers.google.com/recaptcha/docs/display, but this is awesome if it is working.
@dozoisch Do you think we could cut an alpha
to test on the codesandbox?
@@ -152,4 +155,5 @@ ReCAPTCHA.defaultProps = { | |||
tabindex: 0, | |||
size: "normal", | |||
badge: "bottomright", | |||
hl: "en", |
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.
💡 i think we should "default" it to undefined
so it is backward compatible?
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.
wondering, if we default it to undefined, in that case wouldn't the user must define on every case ?
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.
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.
It does say that when you do not supply a language, hl
, it attempts to derive the users language.
Auto-detects the user's language if unspecified.
Also, i assumed the js param
's were only for when loading the script like the way react-google-recaptcha
currently tries to use hl
, but again this is awesome if this works!
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.
If this work we can remove the hl parameter from the URL I guess and pass it on the JS side only. and so it would go props || globalOption || en.
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.
Great, I tried a codesandbox example using my fork. please, check!!
https://codesandbox.io/s/blissful-haze-ozk9y
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.
I don't think we should "default" to 'en'
. We should let the google decide the default language, Auto-detects the user's language if unspecified.
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.
@hartzis ah good point, so yeah props || globalOption || 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.
@hartzis makes sense. lets not default.
Merged here: bb06d1e |
I was unable to use 'lang' prop to override the language.
it is just much simpler to let user send the lang attribute to the component using the
hl
attribute. and default to en.makes everyone's live easier :D