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

Adding required field check for non html5 supportive browsers [$50 awarded] #58

Closed atwellpub closed 10 years ago

atwellpub commented 10 years ago

In safari the required field check is not working. example:

http://screencast.com/t/2JKHklnEgvV

--- The **[$50 bounty](https://www.bountysource.com/issues/2596915-adding-required-field-check-for-non-html5-supportive-browsers?utm_campaign=plugin&utm_content=tracker%2F260752&utm_medium=issues&utm_source=github)** on this issue has been claimed at [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F260752&utm_medium=issues&utm_source=github).
DavidWells commented 10 years ago

Safari doesnt honor html5 required inputs yet. They will someday

atwellpub commented 10 years ago

We'll see if we can add in a hack in the meantime. I'm looking for the client mentioned above to fund this one. Will leave the issue open.

atwellpub commented 10 years ago

image

We will have to build a fall back container that looks like this and load it when the browser safari is detected and a required field is empty and submitted.

DavidWells commented 10 years ago

So the 'required' attribute is added here: https://github.com/inboundnow/leads/blob/master/shared/classes/class.form.php#L162

You would need to add an additional class to the input like 'inbound-required' then on submit -> if required === "" then stop submit with JS to the https://github.com/inboundnow/leads/blob/master/shared/classes/class.form.php#L379 inline JS. Might be tougher with the ajax stuff running first or it needs to bail out on the store.lead.js

On Tue, Jun 10, 2014 at 2:34 PM, Hudson Atwell notifications@github.com wrote:

[image: image] https://cloud.githubusercontent.com/assets/2002207/3237296/ca3f4cb0-f0e6-11e3-90b8-7de41dcd2d67.png

We will have to build a fall back container that looks like this and load it when the browser safari is detected and a required field is empty and submitted.

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

atwellpub commented 10 years ago

Couldn't we just have js code execute on the original 'required' class but only run the js when safari browser is detected?

DavidWells commented 10 years ago

The html5 required tag is not a class it's actually in the element.

Like

On Jun 10, 2014, at 5:52 PM, Hudson Atwell notifications@github.com wrote:

Couldn't we just have js code execute on the original 'required' class but only run the js when safari browser is detected?

— Reply to this email directly or view it on GitHub.

atwellpub commented 10 years ago

Ah I see you are right.

Ok I'll post some potential solution resources: http://wideline-studio.com/blog/html5-form-features-and-their-javascript-fallbacks http://blueashes.com/2013/web-development/html5-form-validation-fallback/

// Without Moderizr
var inputElem = document.createElement('input');
if (!('required' in inputElem)) {
    // JavaScript form validation
  }

...

// With Modernizr
if (!Modernizr.input.required) {
    // JavaScript form validation
} ```

More code examples:
From http://forums.asp.net/t/1911017.aspx?Field+Validation+for+browsers+not+supporting+html5+required+attribute

``` //Required attribute fallback
$('#formTemplate').submit(function() {
  if (!attributeSupported("required") || ($.browser.safari)) {
   //If required attribute is not supported or browser is Safari (Safari thinks that it has this attribute, but it does not work), then check all fields that has required attribute
   $("#formTemplate [required]").each(function(index) {
    if (!$(this).val()) {
     //If at least one required value is empty, then ask to fill all required fields.
     alert("Please fill all required fields.");
     return false;
    }
   });
  }
  return false; //This is a test form and I'm not going to submit it
}); ```

Whatever solution we go with we need to make sure that the js code does not run unless Safari is loaded and that the code does not interfere with our lead storing process.
atwellpub commented 10 years ago

Edvinas is attempting this bounty.

atwellpub commented 10 years ago

Hey since this enhancement is for the Inbound Form we should probably move the solution into the /shared/ folder so CTA and Landing Pages gets the fix too.

I think the js file should sit in shared/classes/js/ and the extra enqueue should sit in here: https://github.com/inboundnow/leads/blob/master/shared/classes/class.form.php#L16

btw @DavidWells do you think we should change this to wp_enqueue_scripts and add an extra hook for wp_enqueue_admin_scripts for admin files? Loading in the init may create overhead as this class model attempts to scale.