0

I have the following issue i would like to get some help for. There is a combobox (select) where i choose an item and i get back a dinamic table from php. The table contains example names. Firstname, Lastname and ID(which is hidden). When i click on the table i get the value of the ID of the selected row. So far it is works fine. The problem that the event doesnt want to fire for first. After that it works fine but i need it for first as i have a function which auto click on the first row but this doesnt work until i solve this problem. I made a code which works fine with a html table. But not with the dinamic one. Please help.

Here is the code works fine with dinamic table but just after 2nd click:

function nametableclick() {

    var rows = document.getElementById("nametable").rows;
            for(var i = 0; i < rows.length; i++)        
        {                           
             rows[i].onclick = function() 
        {
                    data=(this.cells[3].innerHTML);                 

        var data = data;

            $.ajax({
                type: "POST",
                url: "list.php",
                data: "data="+data,
                Type: "json",
                success: function(msg) {

                    msg = JSON.parse(msg);
                    $("#dob").html(msg.dob);
                    $("#age").html(msg.age);
                    $("#sex").html(msg.sex);

                }
             });            
            };

        };                          
         };

And here is the code works well but just with html table: (Actually is same but i use onload)

onload = function() {       
    var rows = document.getElementById("nametable").rows;
            for(var i = 0; i < rows.length; i++)        
        {                           
             rows[i].onclick = function() 
        {
                    data=(this.cells[3].innerHTML);                 

            var data = data;

            $.ajax({
                type: "POST",
                url: "list.php",
                data: "data="+data,
                Type: "json",
                success: function(msg) {

                    msg = JSON.parse(msg);
                    $("#dob").html(msg.dob);
                    $("#age").html(msg.age);
                    $("#sex").html(msg.sex);

                }
            });         
        };
    };
    $("#nametable tr:eq(0) td:first-child").click();                            
         };

When i use the onload function for the dinamic table it just doesnt work at all. Thanks for any help in advance.

Amy
  • 4,034
  • 1
  • 20
  • 34
bontoo
  • 137
  • 1
  • 19
  • 1
    could you make [Fiddel](http://jsfiddle.net) for better understanding? – Amy Oct 22 '14 at 11:52
  • if you drop the `Type` property and have the server return `application/json` (or use `dataType: 'json'`) then you don't need to parse the response yourself, jQuery will do that for you. – Sukima Oct 22 '14 at 12:02
  • If you have `$.ajax` what is the requirement that forces the need to loop over `getElementById(...).rows`? Use jQuery chains: `$('#nametable tr').on('click', ...)` – Sukima Oct 22 '14 at 12:04

2 Answers2

0

This question does not suit well for an answer. Instead, I'll do some code analysis.

onload = function() ... - well not terrible but kinda sloppy. Also looks like this is possible a global namespace leak. I'm going to assume this should be window.onload in which case I'd wonder why jQuery's ready event isn't used $(function() { ... }).

var rows = document.getElementById("nametable").rows;
for(var i = 0; i < rows.length; i++) {
  rows[i].onclick = function() { ... };
}

Ok now were again running away from jQuery as if it was diseased some how. And then were looping over the array of rows only to construct a new function each time and attach them to the onclick (again avoiding jQuery)? Constructing functions inside a loop is a very bad idea and most linters will complain loudly about that. A suggestion:

$('#nametable tr').on('click', function() { ... });

This will attach the click handler to all the <TR> rows in the table with the id="nametable" attribute.

data=(this.cells[3].innerHTML);
var data = data;

My heart skipped a beat here!. First your pulling out the HTML content into (what I thought was a global variable) until I saw the next line and realized we have variable hoisting. But wait your assigning data to itself. Lastly, the name data doesn't provide any context as to the content of the innerHTML. Since I don't have the data I could only guess so in these examples I'll leave it as data. In the future think about picking names which provide context to their content and use. That way when you read the code you don't have to hunt for what the variables are for or how to use them.

var data = $(this, 'td:eq(3)').text();

Finally, the use of data is to directly concatenate it into a post request. I would assume HTML is not desired in that server API. Not to mention the avoidance of jQuery's parameter building by forcing the data to a string. Instead use a JS object:

$.ajax({
  type: 'POST',
  url:  'list.php',
  data: {data: data} // This is a very poorly designed server API
}).then(function(data) {
  ...
});

Also, the use of Type: 'json' suggests that your server is not returning proper HTTP headers. First off there is no Type property for jQuery's ajax instead I think you wanted dataType. However the need for a dataType suggests the server is not sending the proper headers. If the PHP script were to return application/json instead of plain/text then jQuery could parse the response for you avoiding the need for JSON.parse on your own which can be a bit error prone.

$("#dob").html(msg.dob);
$("#age").html(msg.age);
$("#sex").html(msg.sex);

Be warned by using html() your directly injecting HTML into the DOM that you received from a third party. This is a big cross site scripting vulnerability. Use text() instead to push data into the DOM unless you know and can assert the trust of your server and the connection to it (SSL to avoid man in the middle). Probably not important for this example but still worth keeping in mind because it's far to easy to have this show up in the wild.

$("#nametable tr:eq(0) td:first-child")

When you have a selector like this it is far easier and readable to instead provide contextual hooks instead of relying on the make up of the DOM. Add things like class="clickable-row" or class="person-data dob" to your HTML markup. It makes for maintenance and readability.

Sukima
  • 9,965
  • 3
  • 46
  • 60
0

Thanks for the quick reply. Im sure if there are lot of mistakes as i just started to learn this(i mean php html ajax ect.) a few weeks ago so i dont clearly understand everything and i use things i should not use or should do it another way. But there is a simple program i would like to make it done and learn from that. So when i dont know something im trying to get some info (like: w3schools.com) or check other topics which similar what im looking for.

Sorry i left there the

Var data = data;

My mistake. Dont need there. i was trying out something before and left there. Does not make any different anyway.

next: The onload = function() { i found in another topic as solved result and it works with a static table but not with dynamic. I have tried the following. i did not mentioned: window.onload = nametableclick;

function nametableclick() { data here }

But does not work with dynamic table either.

Next:

var rows = document.getElementById("nametable").rows;
        for(var i = 0; i < rows.length; i++)        
        {                           
            rows[i].onclick = function() 
            {
                data2=(this.cells[3].innerHTML);

What it does for me it finds the selected row and comes back with the value of the 3rd(actually 4th) cell which is the ID in my prog. I need this cos i want to sent this value to the php to get all the data from the table where ID = the value. And it works fine.

As i mentioned the prog works fine even if it is not the best way to do it. Slowly i gonna learn how to do it better way. But at the moment the only problem with that is that the dynamic table onclick event fires only after the 1st click. Thanks and sorry if im a bit hard case. :-)))

Oh 1 more thing: "First off there is no Type property for jQuery's ajax instead I think you wanted dataType."

For some reason if i type dataType it just does not work at all. I have no idea why. I watched some training videos and read some short courses about ajax and some of them mentioned using dataType some of them just simple type. I followed everything but did not worked for me. i spent like 5 hours another day to find out why actually i have a topics here with that question as well. get data from mysql with ajax and json into different textareas And accidently i tried with uppercase T once and it worked. Have no idea why.

Community
  • 1
  • 1
bontoo
  • 137
  • 1
  • 19