0

With an electron app I'm working on, I failed to pass an object into the onclick event handler for an image. The event handler is specified in HTML, but the HTML is a string literal in JS.

Goal

The idea is to pass an object from class My into the event handler of the image control the object code tries to generate, see code below


class My {
    constructor() {
        this.id = '123';
    }
    GenerateCtrl() {
        let html = `
            <div class="myclass" id="my-${this.id}">
                <img class="img-btn" id="do-${this.id}" src="images/btn-img.png" role="button" onclick="Do(this, '${this}')">
            </div>
        `;
        return html;
    }
}

The above HTML will be a section of my page.

Here is the event handler

function Do(img, myobj) {
    alert(JSON.stringify(myobj));
}

The caller code when constructing the page:


const sect = document.querySelector(.myclass);

var myObj = new My();

sect.innerHTML = myObj.GenerateCtrl();

I can see the image control on the page without issues.

Expectation

On clicking the image, I expect to see the alert showing the object content.

Observation

but all I can see in the alert is "[object Object]".

Remarks

This tells me that the object doesn't come through that handler function as an argument but as a converted string.

How should I fix this?

kakyo
  • 10,460
  • 14
  • 76
  • 140
  • 1
    Do you *have* to use an inline handler? Best to [avoid them whenever possible](https://stackoverflow.com/a/59539045). Unless it's a requirement for an assignment or something, figure out a different way – CertainPerformance Feb 22 '20 at 11:26
  • @CertainPerformance well, going out of the inline, I'd have to parse the DOM tree to access the element and bind the handler, I'm afraid of the performance penalty and the code clutter for the binding. Are there canonical ways of doing this? I assume this sort of binding should be a pretty common requirement for generating HTML code. – kakyo Feb 22 '20 at 11:32
  • Since you have control over how the new elements to be inserted are being generated, there shouldn't be any performance issue at all. Maybe it'll take an extra line of code, but that's quite trivial, and is by far preferable over not being able to use a proper closure in the event listener – CertainPerformance Feb 22 '20 at 11:40

1 Answers1

1

The proper way to do this would be to insert the HTML as an element, not an HTML string, so that a listener can be attached to the inner <img> beforehand. Attaching an event listener to an element whose parent you already have a reference to is extremely cheap, there's basically no performance penalty. This is how the code could look, using createElement to create the outer element, then insert it with appendChild:

function Do(my) {
  console.log(my);
}
class My {
  constructor() {
    this.id = '123';
  }
  GenerateCtrl() {
    const div = document.createElement('div');
    Object.assign(div, { id: `my-${this.id}`, className: 'img-btn' });
    div.innerHTML = `<img class="img-btn" id="do-${this.id}" src="images/btn-img.png" role="button">`;
    div.children[0].addEventListener('click', () => Do(this));
    return div;
  }
}

var myObj = new My();
document.querySelector('.myclass').appendChild(myObj.GenerateCtrl());
<div class="myclass"></div>

Inline handlers have too many problems:

  • They have escaping issues (they need to be represented in a Javascript string, with string delimiters inside properly escaped for Javascript, and properly escaped for HTML markup)
  • They require global pollution (your current implementation requires a global window.Do function)
  • They run inside two unintuitive with statements - one for the document, one for the current element
  • They cannot reference a variable or object inside the closure that creates the inline hander

Best to avoid them.

CertainPerformance
  • 356,069
  • 52
  • 309
  • 320