search

inboundnow / retired-leads

Track visitor activity, capture and manage incoming leads, and send collected emails to your email service provider for WordPress
http://www.inboundnow.com/leads/
11 stars 3 forks source link

[inbound forms] Checboxes as required field revamp #96

Closed atwellpub closed 9 years ago

atwellpub commented 9 years ago

Currently if a checkbox field is marked as required then we only mark the first checkbox field as required. Otherwise all checkboxes will be required. Anyway the systems does not make sense to me right now. What it is now is a hack added in for a customer because there was zero requirement support for checkboxes at one time and the customer wanted an agreement checkbox.

In the future we need to make sure validation for checkboxes means at least one checkbox is clicked.

DavidWells commented 9 years ago

Will need to implement this in JS i think. I dont think required tags in html5 support this

On Mon, Nov 10, 2014 at 9:37 AM, Hudson Atwell notifications@github.com wrote:

Currently if a checkbox field is marked as required then we only mark the first checkbox field as required. Otherwise all checkboxes will be required. Anyway the systems does not make sense to me right now. What it is now is a hack added in for a customer because there was zero requirement support for checkboxes at one time and the customer wanted an agreement checkbox.

In the future we need to make sure validation for checkboxes means at least one checkbox is clicked.

— Reply to this email directly or view it on GitHub https://github.com/inboundnow/leads/issues/96.

atwellpub commented 9 years ago

Hey Kirit,

So far so good. Here's the review and critique with some more instruction:

needs addressing

Request

Then make a merge request to merge your local changes into our main repository and I'll accept the merge request and close the issue.

atwellpub commented 9 years ago

Also Kirit,

I noticed you forked the repository. This is how we want people who are not employees to contribute, but for you It would be much better if you created your local repositories from our main repos instead of forking them.

I have given your account permission to write commits to them. This way you do not have to keep a separate code base and make merge requests that we have to accept. Having a forked repository will cause you to spend much time staying in up to date with our main codebase and also take extra time via merge requests.

What I will ask you to do is to work with our repositories directly, but use SourceTree to manage them and whenever working on a new issue use SourceTree's 'GitFlow' to start 'New Features' for every issue you address. Then we can merge feature branches into our main branches once they are complete.

http://blog.sourcetreeapp.com/2012/08/01/smart-branching-with-sourcetree-and-git-flow/

DavidWells commented 9 years ago

Hey @kiritDholakiya just a heads up.

We follow these php formatting guidelines: https://make.wordpress.org/core/handbook/coding-standards/php/ and these standards for JS: https://github.com/airbnb/javascript

kiritDholakiya commented 9 years ago

Thanks, i will follow this guideline while working in plugins....

Thanks and Regards, Kirit Dholakiya +91 7698822655

On Fri, Jan 23, 2015 at 2:31 AM, David Wells notifications@github.com wrote:

Hey @kiritDholakiya https://github.com/kiritDholakiya just a heads up.

We follow these php formatting guidelines: https://make.wordpress.org/core/handbook/coding-standards/php/ and these standards for JS: https://github.com/airbnb/javascript

— Reply to this email directly or view it on GitHub https://github.com/inboundnow/leads/issues/96#issuecomment-71097762.

atwellpub commented 9 years ago

Bug: https://github.com/inboundnow/cta/blob/master/shared/classes/class.form.php#L433

When no checboxes present then alert gets fired.