1
\$\begingroup\$

I have a form with 11 inputs that relate to each column on a table, which is displayed above. Each input has an id that matches the a class in the table based on the column name. I want all the inputs to be droppable, the logic for all of them is identical to below.

$(function() {
  $( "#dest_city" ).droppable({
    drop: function( event, ui ) {
        $( ".ui-selected.dest_city" ).each(function(){
            $( '#dest_city' )
                .val( $( '#dest_city' ).val() == "" ? $( this ).html() : $( '#dest_city' ).val() + ", " + $( this ).html() );
        })
    }
  });
});
$(function() {
  $( "#dest_state" ).droppable({
    drop: function( event, ui ) {
        $( ".ui-selected.dest_state" ).each(function(){
            $( '#dest_state' )
                .val( $( '#dest_state' ).val() == "" ? $( this ).html() : $( '#dest_state' ).val() + ", " + $( this ).html() );
        })
    }
  });
});
$(function() {
  $( "#car" ).droppable({
    drop: function( event, ui ) {
        $( ".ui-selected.car" ).each(function(){
            $( '#car' )
                .val( $( '#car' ).val() == "" ? $( this ).html() : $( '#car' ).val() + ", " + $( this ).html() );
        })
    }
  });
});

And the inputs -

    <td><input type="text" name="dest_city" id="dest_city" value="" size="7" /></td>
    <td><input type="text" name="dest_state" id="dest_state" value="" size="3" /></td>
    <td><input type="text" name="car" id="car" value="" size="11" /></td>

And the data -

<tr>
    <td class="drag car">AAAAAAA</td>
    <td class="drag">111111</td>
    <td class="drag">1111</td>
    <td class="drag">P</td>
    <td class="drag loc_city">FTSCOTT</td>
    <td class="drag loc_state">KS</td>
    <td class="drag">L</td>
    <td class="drag">AAAAA</td>
    <td class="drag">11111</td>
    <td class="drag">11</td>
    <td class="drag">AAAAA</td>
    <td class="drag dest_city">FRESNO</td>
    <td class="drag dest_state">CA</td>
</tr>

This checks on drop for all selected cars and inserts each of them to the input, the ternary checks if the input is empty or already has something, and adds the items to make a comma separated list.

My question is, is there any way to refactor this instead of writing the above for each input?

I am using Ruby on Rails so I can easily set up a loop to create that for each input, but that's only shorter on my end, the actual output would still have a nearly identical block of code for each input.

A .car can only be placed in #car, .dest_city in #dest_city etc...

\$\endgroup\$
2
  • \$\begingroup\$ Could you supply a code example for another "droppable" as well? That way we can see what the differences are between the two. It couldn't hurt to include some of your HTML as well. \$\endgroup\$ Jun 2, 2015 at 14:29
  • \$\begingroup\$ Sure, I've added 2 more droppables, and the input related to them. In addition an example of the data being dragged. \$\endgroup\$
    – Clark
    Jun 2, 2015 at 14:51

1 Answer 1

1
\$\begingroup\$

The first thing I would do with this code is wrap the entire code in an IIFE. This will create your own private scope and not pollute the global namespace. Also, since you are using jQuery, you can pass in the jQuery object as $ to ensure that it is correctly mapped.

(function( $ ) {
  //your code here
})( jQuery );

Next, you have each droppable in its own document.ready call. That is unnecessary. You only need one call. We will add that in at the end of the code.

Since most of your code is actually in the event handler and it is almost entirely the same for each instance, we can DRY out your code. Just pass in a string that contains the one difference between each function and cache some of the variables.

function dropHandler( item ) {
  $('.ui-selected.' + item).each( function() {
    var $curr = $('#' + item), val = $curr.val(), html = $(this).html();
    $curr.val( ( val === "" ) ? html : val + ", " + html;
  });
}

With this in place, we just need a to loop over all of your items. So add each input name into an array and loop over it creating the droppables.

var fields = ['dest_city','dest_state','car']

function init() {
  var len = fields.length;
  for( var i = 0; i < len; i++ ) {
    $('#' + fields[i] ).droppable({
      drop : function(event, ui) {
        dropHandler( this.id );
      }
    });
  }
}

Last just add a call to the init function on document.ready.

$(function(){
  init();
});

After these changes, here is the updated code:

(function( $ ){

  var fields = ['dest_city','dest_state','car'];

  function dropHandler( item ) {
    $('.ui-selected.' + item).each( function() {
      var $curr = $('#' + item), val = $curr.val(), html = $(this).html();
      $curr.val( ( val === "" ) ? html : val + ", " + html;
    });
  }

  function init() {
    var len = fields.length;
    for( var i = 0; i < len; i++ ) {
      $('#' + fields[i] ).droppable({
        drop : function(event, ui) {
          dropHandler( this.id );
        }
      });
    }
  }

  $(function(){
    init();
  });

})( jQuery );

Let me know if you have any questions or comments.

\$\endgroup\$
6
  • \$\begingroup\$ Implemented the code as you suggested, the init and dropHandler seem to be executed properly when the page loads, but the dropHandler is not executed whenever a drop is made. Here is a JSFiddle with your function jsfiddle.net/xo00LLbu And here is one with mine, non refactored. jsfiddle.net/vd81nsxv Any suggestions on what may be going wrong? \$\endgroup\$
    – Clark
    Jun 2, 2015 at 17:41
  • \$\begingroup\$ I think I see the problem. drop : dropHandler( fields[i] ); cause dropHandler to be executed immediately. I updated the code above and at the fiddle with the correct format. I believe that will work now. \$\endgroup\$ Jun 2, 2015 at 17:53
  • \$\begingroup\$ Alright, yup I can see the dropHandler is running each time something is dropped, but no values seem to be getting added to the fields. I changed the console.log("DropHandler") to console.log('.ui-selected.' + item) and the output was .ui-selected.undefined \$\endgroup\$
    – Clark
    Jun 2, 2015 at 18:04
  • \$\begingroup\$ Changed the dropHandler call to targetClass = this.id; dropHandler( targetClass ); And progress, but getting an actual error now... jsfiddle.net/jL57wf1L \$\endgroup\$
    – Clark
    Jun 2, 2015 at 18:13
  • \$\begingroup\$ Just needed parentheses here html = $(this).html; - html = $(this).html(); jsfiddle.net/jL57wf1L/1 \$\endgroup\$
    – Clark
    Jun 2, 2015 at 18:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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