0

I'm trying to pass arguments to onclick handler. The table rows are constructed dynamically in a loop and each row contains a tag with different args. Suppose there are two rows, and when clicking the first img, the argument is always the argument corresponding to the second(last) row.

JS code

for(...){
    
   id = get_id();  // every row's id is different
   img = '<img src="/img/icon-view.png" height="20" width="20" style="cursor:pointer" 
   onclick="view_detail(id)">'

   row = ('<tr> ' + '<td>' + img + '</td>' + '</tr>')

   $("#table_1").append(row)
}

function view_detail(id){
    ...
    // the first row's id is always the second id's row
}

Comment: Jquery passing ID from <img> to function gives a solution, but the trick here is id is a variable.

<img onclick="Myfunction(this.id)" src="files/pic/Website.png" id="Website">

Finally I found a workaround which is very simple - create img element and converts to string, and then put the string in row tag.

Sample code:

var img = document.createElement("img")
img.setAttribute("id","local_variable")
img.setAttribute("onclick","view_detail(this.id)")

var img_str = img.outerHTML(img)

row = '<tr>' + '<td>' + img_str + '</td>' + '</tr>'
frogcdcn
  • 391
  • 1
  • 4
  • 14
  • 2
    Duplicate of [JavaScript closure inside loops – simple practical example](https://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example). Delete the `onclick` attribute, delete `id = get_id();`. Instead, put `.on("click", "img", ((id) => () => view_detail(id))(get_id()))` after `$("#table_1").append(row)`. – Sebastian Simon Aug 03 '20 at 03:06
  • I think you're just missing a `)` character at the end of the line where you define `row`. – Toastrackenigma Aug 03 '20 at 03:07
  • @user4642212 This isn't a duplicate of that question, there's no closure here. – Barmar Aug 03 '20 at 03:18
  • The JavaScript in `onclick` attributes is always executed in the global scope. – Barmar Aug 03 '20 at 03:18
  • @user4642212: How is that a duplicate? There's no function in the loop. –  Aug 03 '20 at 03:28
  • @slappy Yes, I admit that it’s technically not the same scenario, but the problem is very similar: `id` gets assigned and reassigned in the loop (globally, in this case), and the function `view_detail` is called much later after the loop finished, so the value of `id` already corresponds to the last row, as described in the question. – Sebastian Simon Aug 03 '20 at 03:35
  • @user4642212 But the solutions are very different. You can't fix this with a proper closure. – Barmar Aug 03 '20 at 12:22
  • @user4642212: They're trying to use `id` in the loop (in a string), so it doesn't really matter that it's declared outside. They just don't know how to concatenate it. Even if they want the variable itself, I wouldn't create two functions on every iteration, invoking one, and keeping the scope alive of the other. There are much lower overhead solutions. –  Aug 03 '20 at 19:23

2 Answers2

0

Dont use string to build DOM, but use document.createElement() instead. Then you can get instance of each target, add any event listeners, you can also pass those instances as arguments to other function as you want.

Here is a sample snippet you can try (I used button instead of img).

const tbody = document.querySelector('table tbody')
const btnAddRow = document.querySelector('button#btn-add-row')

btnAddRow.addEventListener('click', e => {
  const row = document.createElement('tr')
  const col = document.createElement('td')
  const content = document.createElement('button')
  
  content.innerHTML = `Row ${tbody.childNodes.length}`
  
  content.addEventListener('click', e => {
    alert(`${content.innerHTML} clicked!`)
  })
   
  col.append(content)
  row.append(col)
  tbody.append(row)
})
<button id="btn-add-row">Add row</button>
<hr>
<table border="1">
  <thead>
    <tr>
      <th>Action</th>
    </tr>
  </thead>
  <tbody>
  </tbody>
</table>

Another advantage is, your code becomes more readable and modern.

Nurul Huda
  • 1,438
  • 14
  • 12
0

Code in onXXX attributes is executed in the global scope, so it can't use captured local variables in a closure.

Nurul Huda's answer is ideal. But a simple fix to your code is to substitute the actual value of id rather than referencing the variable.

   id = get_id();  // every row's id is different
   img = `<img src="/img/icon-view.png" height="20" width="20" style="cursor:pointer" 
               onclick="view_detail(${id})">`
Barmar
  • 741,623
  • 53
  • 500
  • 612