HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Plugin that conditionally displays elements based on form values

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
conditionallyelementsdisplayspluginthatbasedvaluesform

Problem

I've just released a jQuery plugin that conditionally displays elements based on form values. I would appreciate any suggestions on how to improve both the code and its usefulness.

Here's the demo page

  • Would this be useful to you?



  • What other functionality should I include?



  • Any refactoring or structure changes that I should make?



  • I am worried about exposing 4 different functions. Is this too many?



The Goal

The plugin helps in situations where an element should hide or show based on values in other elements. In particular, it excels when there are multiple conditions that need to be satisfied in order to hide or show.

Additional concepts to keep in mind

  • Use as jQuery plugin



  • Easy to chain rules together



  • Understandable by reading a line of code



  • Can build/add custom conditions



Example of business rules to solve

Let's say we have the following rules for elements:

-
Display a set of checkboxes if

  • Zip is between 19000 and 20000



  • Income is lower than 15000



-
Display another set of checkboxes if

  • City is 'Philadelphia'



  • Income is lower than 40000



-
Display city select box if

  • Zip is between 19100 and 19400



Ideal look

$('.elements_to_display')
   .reactIf( '#some_form_element', SatisfiesFirstCondition)
   .reactIf( '#some_other_form_element', SatisfiesAnotherCondition)
   .reactIf( '#some_other_form_element', SatisfiesAnotherCondition);


Page JS

var IS = $.extend({}, $.fn.reactor.helpers);

$('.cities')    
    .reactIf('#zip', IS.Between(19100, 19400))
    .reactIf('#zip', IS.NotBlank);

$('.philly_middle_to_low_income')
    .reactIf('#income_2011', IS.LessThan(40000))
    .reactIf('#cities_select', IS.EqualTo('philadelphia'));

$('.low_income_select_zips')
    .reactIf('#income_2011', IS.LessThan(15000))
    .reactIf('#zip', IS.BetweenSameLength(19000, 20000))
    .reactIf('#zip', IS.NotBlank);

$('.reactor').trigger('change.reactor');


Plugin react.js

```
(function($){

$.fn.reactTo = function(sele

Solution

-
The syntax is nice, but the necessity for the user to declare IS themselves is not ideal. You should look for a different solution. One possibility could be to supply the name of the conditional function as a string and its arguments as additional arguments of reactIf. That way the conditional functions would no longer need to be of higher-order (not that that is a bad thing). Example:

$('.cities').reactIf('#zip', "Between", 19100, 19400);

// ...
$.fn.reactIf = function(sel, exp_func) {

    var $sel = $(sel);
    var args = arguments.slice(2);
    var _func = function() {
        return $.fn.reactor.helpers[exp_func].apply($sel, args);                               
    };

    // ...
}

$.fn.reactor.helpers = {
  // ...
  Between: function(min, max) {
    var v = $(this).val();
     return(!(v && (parseInt(v) > max || parseInt(v) < min)));
  },
  // ...
}


-
These is one more problem with the conditional functions: You supply a jQuery object as the this argument toapply, so it's not needed to wrap this in another jQuery call inside the conditional functions. You should either change to apply call to:

return exp_func.apply($sel[0]);


or in the conditional functions:

var v = this.val();


-
I'm not sure if it's a good idea to mark elements with a class. This can go wrong, for example, if a second JavaScript removes all classes from an element. Instead of

if (!$(this).hasClass('reactor')) { $(this).reactor(); }

 var conditions_arry = $(this).data('conditions.reactor');
 if (!$.isArray(conditions_arry)) { conditions_arry = []};


I would use

var conditions_arry = $(this).data('conditions.reactor');
 if (!$.isArray(conditions_arry)) {
   $(this).reactor();
   conditions_arry = [];
 };


and similarly in reactor().

-
You should consider short-circuiting the $.each() loop calling the conditional functions (which also makes the && unnecessary):

$.each(conditionalArray, function() {
    r = this.call();
    return r; // Stops the `each` loop if r is `false`
  });

Code Snippets

$('.cities').reactIf('#zip', "Between", 19100, 19400);

// ...
$.fn.reactIf = function(sel, exp_func) {

    var $sel = $(sel);
    var args = arguments.slice(2);
    var _func = function() {
        return $.fn.reactor.helpers[exp_func].apply($sel, args);                               
    };

    // ...
}

$.fn.reactor.helpers = {
  // ...
  Between: function(min, max) {
    var v = $(this).val();
     return(!(v && (parseInt(v) > max || parseInt(v) < min)));
  },
  // ...
}
return exp_func.apply($sel[0]);
var v = this.val();
if (!$(this).hasClass('reactor')) { $(this).reactor(); }

 var conditions_arry = $(this).data('conditions.reactor');
 if (!$.isArray(conditions_arry)) { conditions_arry = []};
var conditions_arry = $(this).data('conditions.reactor');
 if (!$.isArray(conditions_arry)) {
   $(this).reactor();
   conditions_arry = [];
 };

Context

StackExchange Code Review Q#3104, answer score: 5

Revisions (0)

No revisions yet.