Question

I've created a JavaScript object to hold onto a value set by a user checking a checbox in a ColorBox.

I am relatively new to jQuery and programming JavaScript "the right way" and wanted to be sure that the below mechanism for capturing the users check action was a best practice for JavaScript in general. Further, since I am employing jQuery is there a simpler method to hold onto their action that I should be utilizing?

function Check() {
    this.Checked = false;
}

obj = new Check;

$(document).ready(function() {
    $('.cboxelement').colorbox({ html: '<input id="inactivate" type="checkbox" name="inactivatemachine">&nbsp;<label for="inactivate">Inactivate Machine</label>' });

    $(document).bind('cbox_cleanup', function() {
        obj.Checked = $.fn.colorbox.getContent().children('#inactivate').is(':checked');
    });

    $(document).bind('cbox_closed', function() {
        if ($($.fn.colorbox.element()).attr('id').match('Remove') && obj.Checked) {
            var row = $($.fn.colorbox.element()).parents('tr');
            row.fadeOut(1000, function() {
                row.remove();
            });
        }
    });
});
Was it helpful?

Solution

Personally, I would attach the value(s) to an object directly using jQuery's built-in data() method. I'm not really entirely sure what you are trying to do but, you can, for instance, attach values to a "namespace" in the DOM for use later one.

$('body').data('colorbox.checked',true);

Then you would retrieve the value later by:

var isChecked = $('body').data('colorbox.checked');

You run the data() method on any jquery object. I would say this is best-practice as far as jQuery goes.

OTHER TIPS

You could capture the reference in a closure, which avoids global data and makes it easier to have multiple Checks. However, in this case it appears to be binding to the single colorbox, so I don't know that you could usefully have multiple instances.

function Check() {
    this.Checked = false;

    var obj = this; // 'this' doesn't get preserved in closures

    $(document).ready(function() {
       ... as before
    )};
}

var check = new Check; // Still need to store a reference somewhere.
  • $($.fn.colorbox.element()) is redundant. $.fn.colorbox.element() is already a jquery element.

  • It's common use (in the examples i watched, at least) to prepend a $ to variables referencing jquery elements.
    So, var $rows = $.fn.colorbox.element().parents('tr'); gives instantly the idea that it is referencing jquery element(s).

  • I am afraid fadeOut won't work on rows in IE6 (if i recall correctly). You should be able to hide all the content inside the <tr> before removing it.

Can't help on the "simplify" thing because i don't know the colorbox's best uses.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top