Submitting a form via JSON

Question

I have a file named feedback.js that I would like to improve.

function feedback(feedbackform) {

    // collect the feedbackform data while iterating over the inputs
    var data = {};
    for (var i = 0; i < feedbackform.length; i++) {
        var input = feedbackform[i];
        if (input.name && input.value) {
            data[input.name] = input.value;
        }
    }

    // construct an HTTP request
    var xhr = new XMLHttpRequest();
    xhr.open(feedbackform.method, feedbackform.action);
    xhr.setRequestHeader('Content-Type', 'application/json; charset=UTF-8');
    // send the collected data as JSON
    xhr.send(JSON.stringify(data));
    feedbackform.send.disabled = true;

    xhr.onloadend = function () {
        if (xhr.status == 200) {
            feedbackform.send.value = "Sent!";
            feedbackform.send.className = "success";
        } else {
            feedbackform.send.value = "Failed to send";
            feedbackform.send.className = "fail";
        }
    };

    return false;
}

1. Is there a modern way to collect form inputs? E.g. json enctype?
2. Is disabling the submit button a good idea to prevent multiple form submissions?
3. If the form fails to send, what's a good recourse (to prevent data loss of the filled form) I can give the user? Perhaps munge it into a mailto: command?

Are there any other tweaks I can make without relying on any dependencies, such as trying to reduce code size?

Answer 1

Fetch API is now supported by Firefox, Chrome and Opera, you could use it instead of the ugly XMLHttpRequest. How ironic, by the way, is using XMLHttpRequest to make a JSON reqest?

---

var data = {};

Since you likely won't be reassigning this variable, you could use const instead. The same is true for var xhr = new XMLHttpRequest();.

---

for (var i = 0; i < feedbackform.length; i++) {
    var input = feedbackform[i];
    if (input.name && input.value) {
        data[input.name] = input.value;
    }
}

First, that's a perfect use case for a for...of loop. Secondly, if you want to check if a string is empty, it's better to explicitly compare it with an empty string.

I would rewrite this loop like this:

for (let input of feedbackform) {
  if (input.name !== "" && input.value !== "") {
    data[input.name] = input.value
  }
}

---

feedbackform.send.disabled = true;

It's better to disable the whole form, instead of just the submit button. Did you know that you can submit a form by pressing Enter while focused in an input?

---

xhr.onloadend = function () {}

Oh come on, it's the 21^st century! Why do you still use .onevent properties? You should totally drop that and try jQuery .addEventListener(). See addEventListener vs onclick.

You can also use an arrow function here.

I would rewrite it like this:

xhr.addEventListener("loadend", () => {
  // event listener code
})

---

if (xhr.status == 200) 

Always use three-equals operator (===) for equality comparisons. See Does it matter which equals operator (== vs ===) I use in JavaScript comparisons?.

Besides, 200 isn't the only status code which means that request was successful. There is for example status code 204—No Content. Therefore, you should be checking whether the status code is in 200-299 range:

if (xhr.status >= 200 && xhr.status < 300)

---

return false;

What purpose does it serve?

Answer 2

I agree with the comments by Michał Perłakowski. The Fetch API is nice, though it was pointed out in a comment that it doesn't support aborting requests (except possibly in firefox). Other tools like SuperAgent do allow aborting requests.

1. Is there a modern way to collect form inputs? E.g. json enctype?

You might want to consider using FormData to serialize the form.

var data = new FormData(feedbackform);

Otherwise, if you still want to iterate over the elements, you might also consider using functional programming approaches for collecting the inputs. In the example below, the HTMLFormControlsCollection elements is iterated over using Array.prototype.forEach() (it also uses the spread syntax to expand the nodes into an array - [...feedbackform.elements]), though that is a feature of ecmascript-2015 so you could use Array.from(feedbackform.elements) instead). That way, you don't have to index into the array to get the value from the element, increment the index of the iterator (i.e. i++), etc..

To learn more about functional programming, try these exercises.

document.addEventListener('DOMContentLoaded', function() {
  document.getElementById('feedback').addEventListener('click', function() {
    feedback(document.getElementById('feedbackform'));
  });
});

function feedback(feedbackform) {
  var data = {};
  [...feedbackform.elements].forEach(function(element) {  
    data[element.name] = element.value;
  });
  console.log('data: ', data);
}

<form name="feedbackform" id="feedbackform">
  <input type="text" name="fname" />
  <input type="email" name="emailAddr" />
  <input type="number" name="numParts" />
  <select name="animal" required>
<option>- select animal</option>
<option value="meerkat">meerkat</option>
<option value="monkey">monkey</option>
</select>
</form>
<button id="feedback">send feedback</button>

While this might not reduce the code very much, you might find the MDN article on using Promises with XMLHttpRequest interesting. Then in theory, you could have the success callback in the function for .done() and the error handling in the callback function for .catch(). Promises are a good way to handle asynchronous operations.

2. Is disabling the submit button a good idea to prevent multiple form submissions?

Yes. However, as Michał mentioned, you might want to disable the form - perhaps disabling both would be a good idea.

3. If the form fails to send, what's a good recourse (to prevent data loss of the filled form) I can give the user? Perhaps munge it into a mailto: command?

I don't see much in initial searches online for best practices of handling AJAX errors/failures. Many web-applications display an error message to the user that an error occurred. As you mentioned, a fall-back email might be the best solution for your application.