3
\$\begingroup\$

With a table based interface where users can update the rows by pressing a button. Perhaps there is a better way of doing this, without pressing a button, i.e. post on textarea/row change (could be a selectbox column). My requirement is that it's the LEAST amount of JavaScript possible, without being cryptic.

For example, could the code here be improved (assume an uptodate Firefox/Chrome browser), ideally without changing the HTML?

$(document).ready(function () {
$('button').click(function () {
    var row = $(this).parent().parent();
    row.css("background-color", "red");
    var id = row.children(':nth-child(1)').text();
    var note = row.find('textarea').val();
    var data = {
        json: JSON.stringify({
            id: id,
            note: note
        })
    };

    $.ajax({
        url: "/echo/json/",
        type: "POST",
        data: data,
        contentType: "application/json"
    }).done(function (d) {
        console.log(d);
        row.css("background-color", "green");
    });
});
});

I want less code for the JavaScript and I was thinking how much effort if would be to remove the jQuery dependency entirely. Ashamedly it took me a long time to write that jQuery!

\$\endgroup\$
2
\$\begingroup\$
$('button').click(function () {
    var row = $(this).parent().parent();

The button selector is not specific enough. To make sure that .parent().parent() will be what you think it is, and to clarify it in the code, I would recommend this way instead:

$('tr > td > button').click(function () {

Having the id value as the text of a td field is not great. Judging by the name "id", it sounds like technical information that shouldn't really be visible. In HTML5 you can use attributes prefixed with data- for embedding technical details invisibly inside your HTML. So I would write your rows this way:

<tr data-id="goonies">
    <th>Goonies</th>
    <td>
        <textarea></textarea>
    </td>
    <td>
        <button>Save</button>
    </td>
</tr>

And then you could get id like this:

var id = row.attr('data-id');

Perhaps there is a better way of doing this, without pressing a button, i.e. post on textarea/row change (could be a selectbox column).

You could trigger your function on a change to the textarea, in addition to the button, for example:

function onRowChanged() {
    var row = $(this).parent().parent();
    row.css("background-color", "red");
    var id = row.attr('data-id');
    var note = row.find('textarea').val();
    var data = {
        json: JSON.stringify({
            id: id,
            note: note
        })
    };

    // ...
}
$('tr > td > button').click(onRowChanged);
$('tr > td > textarea').change(onRowChanged);

I want less code for the JavaScript and I was thinking how much effort if would be to remove the jQuery dependency entirely.

For the record, I don't think there's anything wrong with having some JavaScript code. On the other hand, getting rid of jQuery can be a good thing, if you only need a tiny subset of its power. It's certainly possible to rewrite this small piece of code without jQuery, but I'm not very good at that myself. Instead, I can recommend zepto.js as an ultra-light jQuery replacement. You can try it in your fiddle, just change the Framework to Zepto, and make a small modification to your AJAX call:

$.ajax({
    url: "/echo/json/",
    type: "POST",
    data: data,
    contentType: "application/json",
    complete: function (d) {
        console.log(d);
        row.css("background-color", "green");
    }
});

I hope this helps, but you might want to wait for some real JavaScript experts to respond!

\$\endgroup\$
  • \$\begingroup\$ Thanks for the suggestions. Esp Zepto.js. However I didn't see the point of duplicating the id value in data-id. It should be straight forward to lookup X table headed column value in a row imo. \$\endgroup\$ – Kai Hendry Aug 29 '14 at 0:39
  • 2
    \$\begingroup\$ In principle, it's good to separate internal details from presentation. In <td>title</td>, "title" is part of the presentation, but in <td data-id="title">Title</td> it is not. Also, looking at just <td>title</td>, you cannot tell that's an id with internal significance, but with using data-id that's quite clear and natural. \$\endgroup\$ – janos Aug 29 '14 at 7:54
1
\$\begingroup\$

Consider using a delegated event handler. The more buttons there are the more optimal it becomes:

$('#id_of_table').on("click", "button", function () {

$(this).parent().parent(); is quite fragile, should you change the HTML structure. Aside from janos' sugggestion, you should use $(this).closest("tr").


Hard-coding the background color change is a bad idea. Just set/remove a class instead and style that class.


row.children(':nth-child(1)') looks wrong to me. In this case I'd markup the first column as ths and use row.children("th").


}).done(function (d) {
    row.css("background-color", "green");
});

Use .always() to reset the color (or remove the class as I suggested), in case something goes wrong.

\$\endgroup\$
  • \$\begingroup\$ Now sure how row.children("th") finds me the value in the id column? \$\endgroup\$ – Kai Hendry Aug 29 '14 at 0:35
  • \$\begingroup\$ It's just about finding the cell. You still get the ID the same way: row.children("th").text() instead of $(this).children(':nth-child(1)').text() (or better use the data-attribute janos suggests). \$\endgroup\$ – RoToRa Aug 29 '14 at 8:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.