4
\$\begingroup\$

We have a rule that all of our validation messages must be in a summary, and thus the default "this field is required" doesn't cut it because the messages lose their context in a summary and therefore need specific field indicators.

I have a solution that I like rather well, but it soon became clear that there was a need for messages outside of just the required field (email, URL, custom methods like phoneUS, etc), so I made some additions to my function.

I've been using jQuery for a while, but I'm not an expert in the optimization area, so I wanted to get some expert help on whether the function below could be optimized...my question is, "is there actually a better way to handle custom error messages in a summary?"

$('.required, .email').each(function(index) {
  var $this = $(this);

  var label = (
    $this.is(':radio')
    ? $("label[data-name='"+$this.attr('name')+"']")
    : label = $("label[for='"+$this.attr('id')+"']")
  );

  var customMessages = [{}];

  if($this.hasClass('required')){
    customMessages.required = "'" + label.text() + "' is required.";
  }

  if($this.hasClass('email')){
    customMessages.email = "'" + label.text() + "' has an invalid email address.";
  }

  $this.rules("add", {
    messages: customMessages
  });
});

Here is the jsFiddle: http://jsfiddle.net/GD5nw/1/

\$\endgroup\$
  • \$\begingroup\$ In general, you should provide a wrapping element to run your selectors on, rather than traversing the entire DOM. Also, why are you adding an empty object literal to customMessages? You are adding properties to an array. It would make more sense to use a plain object. \$\endgroup\$ – Johan Oct 3 '14 at 16:26
2
\$\begingroup\$

This looks fine to me.

I only have 2 minor points:

  1. You could assign the .text() once in the var statements so that your string concats look like customMessages.email = "'" + label + "' has an invalid email address."; This way you are not repeating text() all over the place.
  2. var customMessages = [{}]; looks odd, because you are afterwards assigning straight to the array instead of the empty object. Simply replacing that with var customMessages = {}; works in the fiddle.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.