Return to Answer

Personally I would probably do it this way. It's a bit of a different approach; you don't need this, you don't need new, and it's less code:

function ColorBox(target_id, btn) {
    var $parent = $("#" + target_id).parent();
    var $color = $('<div class="color">ops</div>').appendTo($parent);
    var $button = $('<button class="change" value="' + btn + '">' +
            btn + '</button>').appendTo($color);

    $button.on( 'click', function (event) {
        $parent.css('background', $button.val());
    });
}

$(document).ready(function () {
    ColorBox('target', 'red');
});

Whether you take this approach or do something more like @Joe's answer, there is one thing you should definitely change to work like I have it in this code. Your parent and color variables are both already jQuery objects; there is no need to wrap them in additional $() calls when you use them. So change the names of these variables to include the $ prefix as a reminder that they are jQuery objects, and then just use them directly where you need them instead of the extra $() wrapper.

If you use self as in @Joe's answer, then it would be code like:

self.$parent = $("#" + target_id).parent();
self.$color = self.$parent.append(...);

The $ prefix on these names isn't necessary, but it's a common convention to indicate a variable or property that is a jQuery object. It helps keep straight whether you need to use another $() around it.

Finally, are you aware that your parent and color variables are the same element? It looks like you're expecting color to be the <color> element, but it isn't. I changed the code so it is that element.

Personally I would probably do it this way. It's a bit of a different approach; you don't need this, you don't need new, and it's less code:

function ColorBox(target_id, btn) {
    var $parent = $("#" + target_id).parent();
    var $color = $('<div class="color">ops</div>').appendTo($parent);
    var $button = $('<button class="change" value="' + btn + '">' +
            btn + '</button>').appendTo($color);

    $button.on( 'click', function (event) {
        $parent.css('background', $button.val());
    });
}

$(document).ready(function () {
    ColorBox('target', 'red');
});

Whether you take this approach or do something more like @Joe's answer, there is one thing you should definitely change to work like I have it in this code. Your parent and color variables are both already jQuery objects; there is no need to wrap them in additional $() calls when you use them. So change the names of these variables to include the $ prefix as a reminder that they are jQuery objects, and then just use them directly where you need them instead of the extra $() wrapper.

If you use self as in @Joe's answer, then it would be code like:

self.$parent = $("#" + target_id).parent();
self.$color = self.$parent.append(...);

The $ prefix on these names isn't necessary, but it's a common convention to indicate a variable or property that is a jQuery object. It helps keep straight whether you need to use another $() around it.

Also, be aware that your parent and color variables are the same element. It looks like you're expecting color to be the <color> element, but it isn't. I changed the code so it is the <color> element.

Personally I would probably do it this way. It's a bit of a different approach; you don't need this, you don't need new, and it's less code:

function ColorBox(target_id, btn) {
 
    var $parent = $("#" + target_id).parent();
    var $color = $parent.append('<div class="color" >ops</div>');
    $color.append('<button class="change" value="' + btn + '">' + btn + '</button>');

    $parent.find(".change").on( 'click', function (event) {
        $parent.css('background', $(event.target).val());
    });
}

$(document).ready(function () {
    ColorBox('target', 'red');
});

Whether you take this approach or do something more like @Joe's answer, there is one thing you should definitely change to work like I have it in this code. Your parent and color variables are both already jQuery objects; there is no need to wrap them in additional $() calls when you use them. So change the names of these variables to include the $ prefix as a reminder that they are jQuery objects, and then just use them directly where you need them instead of the extra $() wrapper.

If you use self as in @Joe's answer, then it would be code like:

self.$parent = $("#" + target_id).parent();
self.$color = self.$parent.append(...);

The $ prefix on these names isn't necessary, but it's a common convention to indicate a variable or property that is a jQuery object. It helps keep straight whether you need to use another $() around it.

Also note this simplification:

$("#" + self.parent.attr('id') + " .change")...

can be written much more simply and clearly as:

$parent.find(".change")...

or:

self.$parent.find(".change")...

Personally I would probably do it this way. It's a bit of a different approach; you don't need this, you don't need new, and it's less code:

function ColorBox(target_id, btn) {
    var $parent = $("#" + target_id).parent();
    var $color = $('<div class="color">ops</div>').appendTo($parent);
    var $button = $('<button class="change" value="' + btn + '">' +
            btn + '</button>').appendTo($color);

    $button.on( 'click', function (event) {
        $parent.css('background', $button.val());
    });
}

$(document).ready(function () {
    ColorBox('target', 'red');
});

Whether you take this approach or do something more like @Joe's answer, there is one thing you should definitely change to work like I have it in this code. Your parent and color variables are both already jQuery objects; there is no need to wrap them in additional $() calls when you use them. So change the names of these variables to include the $ prefix as a reminder that they are jQuery objects, and then just use them directly where you need them instead of the extra $() wrapper.

If you use self as in @Joe's answer, then it would be code like:

self.$parent = $("#" + target_id).parent();
self.$color = self.$parent.append(...);

The $ prefix on these names isn't necessary, but it's a common convention to indicate a variable or property that is a jQuery object. It helps keep straight whether you need to use another $() around it.

Finally, are you aware that your parent and color variables are the same element? It looks like you're expecting color to be the <color> element, but it isn't. I changed the code so it is that element.

Personally I would probably do it this way. It's a bit of a different approach; you don't need to use this at all:

function ColorBox(target_id, btn) {

    var $parent = $("#" + target_id).parent();
    var $color = $parent.append('<div class="color" >ops</div>');
    $color.append('<button class="change" value="' + btn + '">' + btn + '</button>');

    function changeColor(element) {
        $parent.css('background', $(element).val());
        return true;
    }
    $("#" + $parent.attr('id') + " .change").on( 'click', function (event) {
        changeColor(event.target);
    });
 
}

$(document).ready(function () {
    ColorBox('target', 'red');
});

Whether you take this approach or do something more like @Joe's answer, there is one thing you should definitely change to work like I have it in this code. Your parent and color variables are both already jQuery objects; there is no need to wrap them in additional $() calls when you use them. So change the names of these variables to include the $ prefix as a reminder that they are jQuery objects, and then just use them directly where you need them instead of the extra $() wrapper.

If you use self as in @Joe's answer, then it would be code like:

self.$parent = $("#" + target_id).parent();
self.$color = self.$parent.append(...);

The $ prefix on these names isn't necessary, but it's a common convention to indicate a variable or property that is a jQuery object. It helps keep straight whether you need to use another $() around it.

Personally I would probably do it this way. It's a bit of a different approach; you don't need this, you don't need new, and it's less code:

function ColorBox(target_id, btn) {

    var $parent = $("#" + target_id).parent();
    var $color = $parent.append('<div class="color" >ops</div>');
    $color.append('<button class="change" value="' + btn + '">' + btn + '</button>');

    $parent.find(".change").on( 'click', function (event) {
        $parent.css('background', $(event.target).val());
    });
}

$(document).ready(function () {
    ColorBox('target', 'red');
});

Whether you take this approach or do something more like @Joe's answer, there is one thing you should definitely change to work like I have it in this code. Your parent and color variables are both already jQuery objects; there is no need to wrap them in additional $() calls when you use them. So change the names of these variables to include the $ prefix as a reminder that they are jQuery objects, and then just use them directly where you need them instead of the extra $() wrapper.

If you use self as in @Joe's answer, then it would be code like:

self.$parent = $("#" + target_id).parent();
self.$color = self.$parent.append(...);

The $ prefix on these names isn't necessary, but it's a common convention to indicate a variable or property that is a jQuery object. It helps keep straight whether you need to use another $() around it.

Also note this simplification:

$("#" + self.parent.attr('id') + " .change")...

can be written much more simply and clearly as:

$parent.find(".change")...

or:

self.$parent.find(".change")...