4

In the following code, it seems I only get the 'otherboxclick' call for the last box added (b1). If I change the order the boxes are added, it's always just the last one that works.

How can I get it to work for all the boxes?

<html>
<head>
</head>
<body>

<script type="text/javascript">
    function otherboxclick(e){
        alert("other clicked");
    }

    function color_toggle_box(boxid, color, width, height, xpos, ypos)
    {
        this.boxid = boxid;

        document.body.innerHTML += 
            "<div id='" + boxid + "'; '; style='position:absolute;left:" + xpos + "px;top:" + 
            ypos +"px;width:" + width + "px;height:" + height + 
            "px;background:#000'></div>";


        this.boxdiv = document.getElementById(boxid);
        this.boxdiv.style.background = color;
        this.boxdiv.addEventListener('click', otherboxclick, true);
    }

    var b2 = new color_toggle_box("223", "#ff0", 100, 100, 205, 100);
    alert("b2 = " + b2.boxid);
    var b3 = new color_toggle_box("323", "#0ff", 100, 100, 100, 205);
    alert("b3 = " + b3.boxid);
    var b1 = new color_toggle_box("123", "#0f0", 100, 100, 100, 100);
    alert("b1 = " + b1.boxid);
</script>

</body>
</html>
Doug Banks
  • 41
  • 2

3 Answers3

3

To explain why Lior's code works and yours doesn't:

document.body.innerHTML +=

is always a mistake. It turns your document objects into an HTML string, adds some text to that string, then re-parses the whole HTML back in to make new document objects that replace all the old ones.

In the best case, this will work but is just a bit slow. However a worse side-effect is that any information that doesn't serialise to HTML will be completely lost. That includes form field values, JavaScript properties and references, and event listeners. So, every time you construct a color_toggle_box, you are destroying all the objects you previously had listeners on; hence the listeners will never be called.

bobince
  • 528,062
  • 107
  • 651
  • 834
  • Thanks. That last comment explains it: the addEventListener doesn't serialize to html, so when I use innerHtml I lose the previous calls. I appreciate the prompt response, you guys rock! – Doug Banks Oct 28 '09 at 14:52
  • Actually, while I have someone answering questions: Suppose I want the 'otherboxclick' to be a function that does something to/with the color_toggle_box in question. I understand the otherboxclick takes an 'event' argument, from which I could get the id of the element clicked. I could use that id to get the color_toggle_box in question from some global list (like have a function that looks up boxes by id). But is there some way to use addEventListener to tie the call to a function on the right instance of a color_toggle_box, directly? – Doug Banks Oct 28 '09 at 14:55
  • Not in addEventListener itself, but you could do it by binding a parameter to the function (or binding `this` to an object). See the bottom of http://stackoverflow.com/questions/1558065/access-event-object-in-event-handler#1558289 for more about function.bind. – bobince Oct 28 '09 at 17:50
  • I wonder where to have systematic knowledge of such important details. I've yet to find any books mentioning this, being a newbie to web tech. – kakyo Feb 23 '20 at 06:09
2
    function color_toggle_box(boxid, color, width, height, xpos, ypos)
    {
        this.boxid = boxid;

        this.boxdiv = document.createElement('DIV');
        this.boxdiv.id = boxid;
        this.boxdiv.style.position = 'absolute';
        this.boxdiv.style.left = xpos + "px";
        this.boxdiv.style.top = ypos + "px";
        this.boxdiv.style.width = width + "px";
        this.boxdiv.style.height = height + "px";
        this.boxdiv.style.backgroundColor = color;

        document.body.appendChild(this.boxdiv);
        this.boxdiv.addEventListener('click', otherboxclick, true);
    }
Lior Cohen
  • 8,985
  • 2
  • 29
  • 27
  • 1
    +1 I also prefer the DOM property setting to the HTML string hacking. It may not make much difference in this case, but kludging HTML strings together without proper escaping is usually a really bad idea. – bobince Oct 28 '09 at 01:59
0

I've had this same problem, but I have a huge HTML string I need to appned to an element. I can't possible think of converting it into a DOM Element.

I ended up using a DOMParser, which can parse my HTML string into a Dom element which can be appended using appendChild

// string to DOM element
var doc  = new DOMParser().parseFromString(elementString, "text/html");

// append
 document.getElementById("myId").appendChild(doc.body.firstChild);
Coder Gautam YT
  • 1,044
  • 7
  • 20