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

Add isolated prop to component #179

Merged
merged 2 commits into from
Jul 19, 2020
Merged

Conversation

codeth
Copy link
Contributor

@codeth codeth commented Jul 2, 2020

Google documentation details an isolated parameter that can be passed to the render.

This PR exposes isolated as a component prop, allowing it to be optionally set.

README.md Outdated
@@ -53,6 +53,7 @@ Properties used to customise the rendering:
| tabindex | number | *optional* The tabindex on the element *(__default:__ `0`)*
| type | enum | *optional* `image` or `audio` The type of initial captcha *(__defaults:__ `image`)*
| theme | enum | *optional* `light` or `dark` The theme of the widget *(__defaults:__ `light`)*. See [example][docs_theme]
| isolated | bool | *optional* For plugin owners to not interfere with existing reCAPTCHA installations on a page. If true, this reCAPTCHA instance will be part of a separate ID space. *(__default:__ `false`)*
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be great to order he properties for readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, done!

@dozoisch
Copy link
Owner

dozoisch commented Jul 2, 2020

Thanks!

@@ -174,6 +176,7 @@ ReCAPTCHA.propTypes = {
stoken: PropTypes.string,
hl: PropTypes.string,
badge: PropTypes.oneOf(["bottomright", "bottomleft", "inline"]),
isolated: PropTypes.bool,
Copy link
Collaborator

Choose a reason for hiding this comment

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

❓ should we add a default prop value for isolated?

isolated: false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose not to implement a default value because:

  1. It is not a required prop, and
  2. Google reCAPTCHA script defaults this to false anyway when not supplied - see docs

Open to discussion though!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good

@dozoisch dozoisch merged commit 3ef5bc9 into dozoisch:master Jul 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants