CODEWITHSUNDEEP
javascriptjqueryjquery-plugins
benjaminz
benjaminz

Reputation: 3228

Self-made jQuery plugin triggered but not working

I wrote a simple plugin to validate if input fields that needs to be number are indeed numbers.

The plugin can be successfully loaded and triggered, but it doesn't do anything. However, if I manually type $(selector).validateNum(); in the console, it works perfectly. Anyone knows why?

Please check this jsBin for the live version:

jsBin: http://jsbin.com/tusunuweto/1/edit?html,js,console,output

Code below:

  $.fn.validateNum = function () {
    console.log('validation started');
    var numInputFields = this.find('input.number');

    $(numInputFields).each(function () {
      if ( isNaN( Number( $(this).val() ) ) ) {
        $(this).keyup(function () {
          alert("This field must be a number");
        });
      }
    });
  };

In my index.html file:

<script type="text/javascript">

  $(function() {
    $('#selector').validateNum();
  });

</script>

Upvotes: 0

Views: 75

Answers (2)

Mulan
Mulan

Reputation: 135227

A couple of issues I see

You're not chaining properly

This isn't inherently bad (because .find does work the way you've used it) but it could afford to be more explicit. For example, if you call $(".foo, .bar").validateNum() it should be clear that it's going to find inputs in each selected element

You'll see this pattern very commonly in other community plugins

$.fn.myPlugin = function() {
  // `this` is the jQuery object that matches the selected elements

  // don't forget `return` to make your plugin chainable
  return this.each(function() {
    // iterate through the selected elements performing some operation
  });
};

But! as you'll see next, we don't even need to use .find at all!

You're not using event delegation

You're specifically binding a .keyup listener to each input. If there's a lot of inputs on your page, this could become very resource usage intensive.

This method uses delegation so you're not binding a listener to every single input

$.fn.validateNum = function() {
  console.log("validation started");
  return this.each(function() {
    $(this).on("keyup", ".number", function(event) {
      var num = parseInt($(this).val(), 10);
      if (isNaN(num)) {
        alert("This field must be a number");
      }
    });
  });
};

$(".testPlugin").validateNum();

Upvotes: 0

Wesley Smith
Wesley Smith

Reputation: 19571

You need to move your isNaN check into the click handler, I also added a regex to remove non numbers.

This works, but if you want to do validation like this, you might want to look into a library like .validate

 $(function () {

     $.fn.validateNum = function () {
         console.log('validation started');
         var numInputFields = this.find('input.number');
         $(numInputFields).each(function () {
             $(this).keyup(function () { // you need to move your isNaN check into the click handler
                 if (isNaN(Number($(this).val()))) {
                     $(this).val( $(this).val().replace(/[^0-9.]/g, "") ); // added this to remove non-numbers
                     alert("This field must be a number");
                 }
             });
         });
     };
     
     $(".testPlugin").validateNum();
 });
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="testPlugin">
<input class="number" />

Upvotes: 1

Related Questions