Select row based on another table row index

  • I have 2 tables. One holds only first column of my "data table" rest is stored in another table, that is placed right to first. What I need to do is to get all the data from specific row to pass it to jqplot.



    My tables look like this. My code basically works, but I think it can be improved.



    $('table#baseTable  > tbody > tr > td').click(function() {
    var rowIndex = $(this).parent().index();
    $('div#log').html(rowIndex);

    var myData = [];

    $('#dataTable tbody tr:eq(' + rowIndex + ')').map(function() {
    return $(this.cells).get();
    }).each(function() {
    var headerVal = $(this).closest("table").find("thead > tr > th").eq($(this).index()).html();
    myData.push([headerVal, $(this).html()]);
    })

    alert(myData);
    console.log(myData);
    });


    I'm using this code in ASP page, so that every time I use UpdatePanel I must call my functions again.



    This is my plot function:



    function plot() {
    var $plot;
    var dataTableRows = $('#Grid3_br tbody tr');

    $('td.akcje').on('click', function() {
    var $newTitle = $(this).next().text();
    var myData = [],
    element = $(this),
    rowIndex = element.parent().index();

    $(dataTableRows[rowIndex]).map(function() {
    return $(this.cells).get();
    }).each(function(index, value) {
    myData.push(['M' + index, parseFloat($(this).html())]);
    })
    $('#wykres1').empty();//clears old plot. Don't know why re-plotting don't work for me
    $plot = $.jqplot('wykres1', [myData], {
    title: $newTitle,
    axes: {
    xaxis: {
    renderer: $.jqplot.CategoryAxisRenderer,
    tickOptions: {
    formatString: '%s'
    }
    },
    yaxis: {
    tickOptions: {
    formatString: '%.2f zł'
    }
    }
    },
    highlighter: {
    show: true,
    sizeAdjust: 7.5,
    tooltipAxes: 'y'
    },
    cursor: {
    show: false
    }
    });

    $(window).resize(function() {
    $plot.replot({
    resetAxes: true
    });
    });
    });
    }

  • Arne

    Arne Correct answer

    9 years ago

    I'll start with your selectors:



    First off, you should never have a selector with anything left of an id - searches for ids are about the fastest you can get on the DOM.



    Second, if you don't want to wrap other tables in your tables (I assume you don't), get rid of the immediate child stuff (P > C). Searching by tag is also pretty fast, so we use the id-element as our base for that.



    $('#baseTable tbody td').click(function() {
    var rowIndex = $(this).parent().index();
    $('#log').html(rowIndex);

    var myData= [];

    $($('#dataTable tbody tr')[rowIndex]).map(function() {
    return $(this.cells).get();
    }).each(function() {
    var headerVal = $(this).closest('table').find('thead th')
    .eq($(this).index()).html();
    myData.push([headerVal,parseFloat($(this).html())]);
    })

    alert(myData);
    console.dir(myData);
    });


    Next, you should cache elements you found, use more than once and which don't change. And usually, it's a bad choice to bind events to the elements directly. I'm assuming jQuery 1.7 is ok and you don't really want to use jQuery 1.4, so we can use on for event binding.



    var logDiv = $('#log'),
    dataTableRows = $('#dataTable tbody tr');

    $('#baseTable tbody').on('click', 'td', function() {
    var myData = [],
    element = $(this),
    rowIndex = element.parent().index();

    logDiv.html(rowIndex);

    $(dataTableRows[rowIndex]).map(function() {
    return $(this.cells).get();
    }).each(function() {
    var headerVal = $(this).closest('table').find('thead th')
    .eq($(this).index()).html();
    myData.push([headerVal,parseFloat($(this).html())]);
    })

    alert(myData);
    console.dir(myData);
    });


    Next, accessing the DOM is rather slow. If you need the data for jqplot, do you need the table headers and can the data change?



    Anyway, I'd store all data in an array beforehand. Now it looks like this:



    var logDiv = $('#log'),
    dataTable = $('#dataTable'),
    dataTableHeaders = [],
    dataTableData = [];

    // cache data table headers
    dataTable.find('thead th').each(function(col) {
    dataTableHeaders[col] = $(this).html();
    });

    // cache full row (including headers) for data table
    dataTable.find('tbody tr').each(function(row) {
    var rowData = (dataTableData[row] = []);
    $(this).children().each(function(col) {
    rowData[2 * col] = dataTableHeaders[col];
    rowData[2 * col + 1] = parseFloat($(this).html());
    });
    });

    // the onClick - but on tr instead of td and with a fast lookup
    $('#baseTable tbody').on('click', 'tr', function() {
    var rowIndex = $(this).index(),
    myData = dataTableData[rowIndex];
    logDiv.html(rowIndex);
    alert(myData);
    console.dir(myData);
    });


    I still don't like the need for index(). I like it even less than event binding on elements, so I'll compromise:



    var logDiv = $('#log'),
    dataTable = $('#dataTable'),
    dataTableHeaders = [],
    dataTableData = [];

    dataTable.find('thead th').each(function(col) {
    dataTableHeaders[col] = $(this).html();
    });

    dataTable.find('tbody tr').each(function(row) {
    var rowData = (dataTableData[row] = []);
    $(this).children().each(function(col) {
    rowData[2 * col] = dataTableHeaders[col];
    rowData[2 * col + 1] = parseFloat($(this).html());
    });
    });

    $('#baseTable tbody tr').each(function(row) {
    $(this).on('click', function() {
    var data = dataTableData[row];
    logDiv.html(row);
    alert(data);
    console.dir(data);
    });
    });


    Here's my last step (this is not really needed, but I like it): we isolate this code from the outside world and make accidental messups a lot harder. And we enable a recalculation and rebinding of the click event if anything changes (number of rows / cols or the content).



    var tableData = (function(){
    var logDiv = $('#log'),
    cacheRows = function() {
    var dataTable = $('#dataTable'),
    dataTableHeaders = [],
    dataTableData = [];
    // fetch headers
    dataTable.find('thead th').each(function(col) {
    dataTableHeaders[col] = $(this).html();
    });
    // fetch column data, prepare row arrays
    dataTable.find('tbody tr').each(function(row) {
    var rowData = (dataTableData[row] = []);
    $(this).children().each(function(col) {
    rowData[2 * col] = dataTableHeaders[col];
    rowData[2 * col + 1] = parseFloat($(this).html());
    });
    });
    // return row arrays
    return dataTableData;
    },
    bindOnClick = function() {
    var dataTableData = cacheRows();
    $('#baseTable tbody tr').each(function(row) {
    $(this).on('click', function() {
    var data = dataTableData[row];
    logDiv.html(row);
    alert(data);
    console.dir(data);
    });
    });
    };
    bindOnClick();
    return {
    refresh: bindOnClick
    };
    })();



    Now, when the tables change, you can preserve the intended behavior by calling tableData.refresh(). This is still a little messy because we never call off, but as we only use jQuery to change the DOM, we rely on it to unbind event handlers if we change elements. It's probably better to change the returned object so it has two functions: one to refresh the cache and one to rebind. rebinding is needed when the whole table was rebuilt and no DOM node is reused. Otherwise, this leaks event handlers. If you update the table infrequently, you should be fine. For frequent updates, I'd change it back to using the index function again.



    Edit: Here's an example for what I described in my comment below.



    var prepareForPlotting = function(baseSelector, dataSelector, clickhandler) {
    var cacheRows = function() {
    var dataTable = $(dataSelector),
    dataTableHeaders = [],
    dataTableData = [];
    // fetch headers
    dataTable.find('thead th').each(function(col) {
    dataTableHeaders[col] = $(this).html();
    });
    // fetch column data, prepare row arrays
    dataTable.find('tbody tr').each(function(row) {
    var rowData = (dataTableData[row] = []);
    $(this).children().each(function(col) {
    rowData[2 * col] = dataTableHeaders[col];
    rowData[2 * col + 1] = parseFloat($(this).html());
    });
    });
    // return row arrays
    return dataTableData;
    },
    bindOnClick = function() {
    var dataTableData = cacheRows();
    $(baseSelector + ' tbody tr').each(function(row) {
    $(this).on('click', function() {
    clickhandler(row, dataTableData[row]);
    });
    });
    };
    bindOnClick();
    return {
    recache: cacheRows,
    rebind: bindOnClick
    };
    }

    var logDiv = $('#log'),
    baseHandler = prepareForPlotting('#baseTable', '#dataTable',
    function(row, data) {
    logDiv.html(row);
    alert(data);
    console.dir(data);
    }
    );

    wow :) Thanks for that. I'm still trying to learn jQuery and I would like to learn the best practices at start, so any advice's like Your are very helpful. Any future updates on this are welcome :)

    Your answer is awesome! I'am looking for a way to change this in some kind of plugin, so that I will be able to use it more than ones on page. In my solution I'm using 4 tables like shown here http://jsfiddle.net/Misiu/9j5Xy/12/ this is simple. These 2 solution are used in asp 2.0 webpage, so every time I request data from server (my update panel updates) I must init all javascript again. So can Your awesome code be changed into plogin? So that I can call it like $('wrapper').plotData()? Or should I leave Your solution? Which one is better?

    In that case, I'd make it a function with the selectors as parameters. You call it once per table combination and use the return value for refreshes on new data (so you can register it as an eventhandler for a 'change' event you fire when data changes on a table). But I'd first drop the alert and the call to logDiv :-)

    Alert and log was only for test purposes. I'll edit my question and add code that I'm using right now :) So Your saying that it would be better to turn it to function with 2 parameters: one-div wrapping all tables and second-div displaying plot.

    not the div, I'd pass the selectors for baseTable and dataTable and a callback function. The callback updates the plot (so you can target different plotting areas).

    I have all tables wrapped inside one div (like in that jsfiddle link from comment above), so instead can I pass only one parameter - that wrapper div selector? But I don't understand that callback function approach. Could You explain it a bit? I'm a beginner :)

    I would pass the selectors instead of the div so you can delete the dom nodes and rebuild the table and have it still work. The callback function... I'll add an example in my answer, it would be too long for a comment.

    thanks again for Your answer and comments. I would like to click upvote button 100 times for this, but I can only once. Really grate peace of code and brilliant explanation.

    @Arne I like function identifiers, they make code more readable and easier to debug in my opinion (stack trace looks better with `name()->otherName()` instead of `function()->function()`). So I would write: ` $(this).on('click', function rowClicked() {` But I what is more important, I would avoid making functions in loops (or in `each`). Simply why should we declare _n_ (which in our case could be huge) functions if we can use one (http://jsfiddle.net/tomalec/cvPWv/2/). Is there any reason, I have missed not to use http://jsfiddle.net/tomalec/cvPWv/3/

    @tomalec yeah, function identifiers are neat for traces and could/should be used here. I'd probably do some of the stuff differently now (Use angular or backbone!)... About your second fiddle: will this work? The function is going to be called by jQuery. If I recall correctly, it will be called with the DOM element you clicked on as "this" and the event as the only argument to the function. You can't change that, so where would your function get its arguments from?

    @Arne Personaly I started to be function identifier fetishist, I see no point in usage of unnamed anonymous functions every where. Regarding fidle, it should. If you will provide fiddle with your code I can double check. You are right event is the only argument. However, according to http://api.jquery.com/on/ `.on( events [, selector ] [, data ], handler(eventObject) )` so you can add data: `{rowIndex: rowIndex}` into `event.data`. Simple, clean, and easy.

    That was regarding fiddle /2/. You are right /3/ won't work.

License under CC-BY-SA with attribution


Content dated before 7/24/2021 11:53 AM