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

STENCIL-2934 Add recaptcha v2 support to cornerstone. #951

Merged
merged 1 commit into from
Mar 10, 2017
Merged

STENCIL-2934 Add recaptcha v2 support to cornerstone. #951

merged 1 commit into from
Mar 10, 2017

Conversation

junedkazi
Copy link
Contributor

@junedkazi junedkazi commented Mar 7, 2017

What?

Adding support for google recaptcha v2.

Tickets / Documentation

Add links to any relevant tickets and documentation.

Do Not Merge

It depends on https://github.com/bigcommerce/bigcommerce/pull/17715.

zip:
Cornerstone-recaptcha-v2-1.5.3.zip

"hide": "Hide Reviews",
"new": "Write a Review",
"show": "Show Reviews",
"header": "{total, plural, =0{0 Reviews} one {# Review} other {# Reviews}}",
"rating": "Rating",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Duplicate key.

<div class="form-field">
<label class="form-label" for="contact_question">{{lang 'forms.contact_us.question'}}
<small>{{lang 'common.required' }}</small>
</label>
<textarea name="contact_question" id="contact_question" rows="5" cols="50" class="form-input"></textarea>
</div>

{{#if forms.contact.recaptcha.enabled}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the field down after the question description field since it is a practice that it should be the last element before the submit.

Copy link
Contributor

@mcampa mcampa Mar 7, 2017

Choose a reason for hiding this comment

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

do we need to wrap it in the if block?
I think forms.contact.recaptcha.markup will be empty when is disabled.

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 would like to keep the captcha enabled check in place irrespective of what is returned in the markup field.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's not how we do this kind of things in Stencil, we removed most of all boolean Blueprint variables because we can check agains the data

@junedkazi junedkazi changed the title [WIP] STENCIL-2934 Add recaptcha v2 support to cornerstone. STENCIL-2934 Add recaptcha v2 support to cornerstone. Mar 7, 2017
@junedkazi
Copy link
Contributor Author

@bigcommerce/stencil-team

Copy link
Contributor

@mcampa mcampa left a comment

Choose a reason for hiding this comment

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

Need a changelog entry

<div class="form-field">
<label class="form-label" for="contact_question">{{lang 'forms.contact_us.question'}}
<small>{{lang 'common.required' }}</small>
</label>
<textarea name="contact_question" id="contact_question" rows="5" cols="50" class="form-input"></textarea>
</div>

{{#if forms.contact.recaptcha.enabled}}
Copy link
Contributor

@mcampa mcampa Mar 7, 2017

Choose a reason for hiding this comment

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

do we need to wrap it in the if block?
I think forms.contact.recaptcha.markup will be empty when is disabled.

Copy link
Contributor Author

@junedkazi junedkazi left a comment

Choose a reason for hiding this comment

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

We need a change log entry for this. Also since this is going to get merged first can we also add a new line at the end of last entry since if you don't the last release title wraps up.

@nickdengler
Copy link
Contributor

💚 LGTM 👍

Copy link
Contributor

@mcampa mcampa left a comment

Choose a reason for hiding this comment

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

👍

@mcampa mcampa merged commit 84fcc93 into bigcommerce:master Mar 10, 2017
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.

3 participants