js onclick for newly created element

Question

Hi I am trying to create a simple colorpicker for my project. It should dispalay DIV vith colored SPANS and each span should onclick call my function -pickColor()- with its onwn value.

The problem is that, after clicking on any Span my function is simply called with last walue from my array instead of the coresponding one. Any idea how to correct it?

here is my code

var colors = [ "303030", "777777", "da0025", "f01800", "ff4300", "fd6c05", "feab07", "ffc91e", "93c900", "54c300", "00ab62", "00c3c4", "009bf0", "006afe", "3f00dd", "9025ff", "ff3ec2", "fe0b6b"];

//color picker var picker;

function createPicker(){
//create picker wrapper
picker = document.createElement('div');
picker.id = "colorPicker";
picker.style.display = "none"; // initialy invisible !
picker.style.margin = "0";
picker.style.padding = "0.5em";
picker.style.backgroundColor = "rgb(30,30,30)";
//create picker color options
var newColor;
for (var i = colors.length - 1; i >= 0; i--) {
    newColor = document.createElement('span');
    newColor.id = colors[i];
    //set style for color option
    newColor.style.display = "inline-block";
    newColor.style.width = "50px";
    newColor.style.height = "50px";
    newColor.style.margin = "0";
    newColor.style.padding = "0";
    newColor.style.backgroundColor = "#"+colors[i];
    //add onclick function

    newColor.addEventListener("click", function f(){pickColor( colors[i] )}, true);

    //append option
    picker.appendChild(newColor);
};
//append colorPicker to file
document.getElementById('here').appendChild(picker); // value must be set to the parent elements id !
}

function displayPicker(){
picker.style.display = "inline-block";
}

function pickColor(id){
//set value
console.log(id);
var input = document.getElementById('color'); // must be the 'input' elements id that we want to set !
//console.log(input);
input.value = id;
// hide picker
picker.style.display = "none";
}

acontell · Answer 1 · 2015-01-03T14:38:40.670

2

Closure is nice though sometimes a bit confusing. Here's a Fiddle with the problem (I hope) solved and here's the js code that made it possible:

newColor.addEventListener("click", (function (param) {
            return function() {
                pickColor(colors[param]);
            };
        })(i), true);

The problem was that, due to closure, i value was -1. Yo have to pass the actual value of i to the function and, thanks to closure, use it in the function that you'll end up returning and using for the event.

UPDATE

Better approach: define function somewhere in your code:

function clickEvHandlerClosure(param) {
    return function () {
        pickColor(colors[param]);
    };
}

and then:

newColor.addEventListener("click", clickEvHandlerClosure(i), true);

fiddle

Hope it helps.

edited Jan 03 '15 at 14:38

answered Jan 03 '15 at 13:36

acontell

6,792
1
19
32

This won't pass JSLint ("Don't create functions in loops"), it does the job though. – Teemu Jan 03 '15 at 14:01
@Teemu he,he well... that wasn't requested in the first place. Secondly, I sincerely doubt that most of the js scripts seen in stackoverflow would pass JSLint. – acontell Jan 03 '15 at 14:23
thank you werry much it helped a lot – joasisk Jan 03 '15 at 14:25
Probably they wouldn't. But it's bad practice to create functions within loops, it increases memory and CPU time consumption. The IIFE here could be replaced with a pre-declared function very easily. – Teemu Jan 03 '15 at 14:27
@Teemu thanks for your feedback. I've updated the answer. Do you like it best? – acontell Jan 03 '15 at 14:33
1

Definitely better, +1 ; ). – Teemu Jan 03 '15 at 14:39

score 0 · Accepted Answer · answered Jan 03 '15 at 13:32

0

After looking at Teemu's recommend link (thanks), I realized that it gets quite tricky if you use function in addEventListener method call within a loop.

So how about doing this way, without any parameters:

newColor.addEventListener("click", pickColor, true);

In the pickColor function you can access the id in the following way:

function pickColor(){

   var id = this.id;
   //more code below

}

answered Jan 03 '15 at 13:32

jyrkim

2,849
1
24
33

but what is 'this' in this instance? – joasisk Jan 03 '15 at 14:27
@joasisk Very good question :-) Here is a quote for an answer: 'In JavaScript this always refers to the “owner” of the function we're executing, or rather, to the object that a function is a method of. ... An onclick property, though, is owned by the HTML element it belongs to.' Source: http://www.quirksmode.org/js/this.html – jyrkim Jan 03 '15 at 15:43
thanks it is werry helpful since Im a newbee and am learning by doing – joasisk Jan 04 '15 at 09:20
this will definately help a lot in future – joasisk Jan 04 '15 at 09:27
@joasisk I'm glad that I was able to provide some assistance :-) I remember when I was doing similar type of event handling for the very first time, I found it really tricky, and spend hours trying to figure out whats going on with dynamically created elements and their event handling. Anyway, you are doing great and progressing really well with JavaScript! – jyrkim Jan 04 '15 at 15:36

js onclick for newly created element

2 Answers2