libreform / wp-libre-form

Easy native HTML5 forms for WordPress. Version 1.5 is unmaintained, but works without issue. 2.0 has been rewritten from the ground, and can be found at https://github.com/libreform/libreform
https://wordpress.org/plugins/wp-libre-form
GNU General Public License v3.0
67 stars 27 forks source link

prevent forms to submit multiple times #121

Closed luizbills closed 6 years ago

luizbills commented 6 years ago

Disable and enable the submit buttons to prevent the form to be submitted multiple times.

k1sul1 commented 6 years ago

This changes existing behaviour. I certainly wouldn't want my button to change in appearance, and disabling buttons tends to change their appearance.

👍 for preventing multiple submissions but disabling submit buttons isn't the way, especially when you can still submit the form using the keyboard.

luizbills commented 6 years ago

for preventing multiple submissions but disabling submit buttons isn't the way, especially when you can still submit the form using the keyboard.

You can't. At least not in chrome.

This changes existing behaviour. I certainly wouldn't want my button to change in appearance, and disabling buttons tends to change their appearance.

We can use a local variable to verify if has a requisition in progress, instead of disabling the button

k1sul1 commented 6 years ago

Interesting. It seems you're right. http://jsfiddle.net/t2n3ghr7/

What do you mean with a local variable? The data is already there, in the DOM.

Doing the prevention properly would require a rewrite of the client side js, which is coming, someday. Right now references to instances and their states are not stored anywhere and the error and success "events" are shared between all forms, which will change.

And then we can have an actual js object for the state of the form.

luizbills commented 6 years ago

What do you mean with a local variable? The data is already there, in the DOM.

something like this:

formSending: {},
submitHandler: function (e) {
  var form = e.target;
  var data = new FormData(form);
  var formID = form.dataset.formId

  // prevent a request
  if ( window.wplf.formSending[formID] ) return;

  // block future requests
  window.wplf.formSending[formID] = true;

  // ...

  fetch(ajax_object.ajax_url  + '?action=wplf_submit', {
    method: "POST",
    credentials: ajax_object.ajax_credentials || 'same-origin',
    body: data
  }).then(function(response) {
    return response.text();
  }).then(function(response) {
    // reset form state
    window.wplf.formSending[formID] = false;
    // ...
k1sul1 commented 6 years ago

Something like that is coming in the future, but on a larger scale, with an API and so on.

Thinking of something like wplf.get('form-slug').addSuccessListener((e, formElement, state) => {}).

Right now the simpler implementation is just fine, unless you want to do a thousand things at the same time when an user submits the form, you won't run into any performance issues.

luizbills commented 6 years ago

So can i push this last propose or do you will close this PR?

k1sul1 commented 6 years ago

Doing it now would be overengineering, and make the upcoming rewrite harder. Backwards compatibility concerns and so on.

In other words, I don't want to expose a key that's going to get removed, because someone is going to depend on it.

b0aebd3 is the solution that I feel like is the best at the moment. Might add a e.preventDefault() inside the early return block, but it's the least amount of code solution, without any visible changes to the end user.

luizbills commented 6 years ago

b0aebd3 It's nice solution too

k1sul1 commented 6 years ago

My FF didn't try to submit the form natively, but Chrome did, so I added said preventDefault() call.

Works, I'll squashmerge this after Travis confirms it's all set.