0

I'll have a link "Edit" where you can click on to edit some fields. Now after you have clicked the link, the link to edit should get disabled. I've tried to achieve that with adding a class editing. So if .not('.editing') run the rest of code ... but I can click twice

What have I done wrong? Thanks for watching and helping --> Fiddle http://jsfiddle.net/s1v8n2Ld/

$('.edit-product').not('.editing').on('click', function(e){
    e.preventDefault();
    $(this).addClass("editing")
    var $table = $(this).closest('table');
    $table.find('.editable').each(function(i){
        var content = $(this).html().trim();
        $(this).wrapInner('<input type="text" name="" placeholder="'+content+'">');
    });
});

$('.edit-product2:not(.editing)').on('click', function(e){
    e.preventDefault();
    $(this).addClass("editing")
    var $table = $(this).closest('table');
    $table.find('.editable').addClass('xxxx');
    $table.find('.editable').each(function(i){
        var content = $(this).html().trim();
        $(this).wrapInner('<input type="text" name="" placeholder="'+content+'">');
    });
});



<table border="1" style="border-collapse:collapse;">
    <tr>
        <td>
            ID
        </td>
        <td>
            18
        </td>
        <td rowspan="4">
            <a href="/app_dev.php/edit/ 18" class="edit-product">Edit</a>
        </td>
    </tr>
    <tr>
        <td>
            Name
        </td>
        <td class="editable">
            New product name!
        </td>
    </tr>
    <tr>
        <td>
            Price
        </td>
        <td class="editable">
            19.99
        </td>
    </tr>
    <tr>
        <td>
            Description
        </td>
        <td class="editable">
            Lorem ipsum dolor
        </td>
    </tr>
</table>




<table border="1" style="border-collapse:collapse;">
    <tr>
        <td>
            ID
        </td>
        <td>
            18
        </td>
        <td rowspan="4">
            <a href="/app_dev.php/edit/ 18" class="edit-product2">Edit</a>
        </td>
    </tr>
    <tr>
        <td>
            Name
        </td>
        <td class="editable">
            New product name!
        </td>
    </tr>
    <tr>
        <td>
            Price
        </td>
        <td class="editable">
            19.99
        </td>
    </tr>
    <tr>
        <td>
            Description
        </td>
        <td class="editable">
            Lorem ipsum dolor
        </td>
    </tr>
</table>
caramba
  • 21,963
  • 19
  • 86
  • 127

2 Answers2

3

You need event-delegation since the class .editing is added after the dom is loaded

$(document).on('click','.edit-product:not(.editing)'

Or like @Karl-André Gagnon pointed out in the comments

check if the element has the class editing inside the handler.

$('.edit-product').on('click', function(e){
     if(!$(this).hasClass('editing')){
Anton
  • 32,245
  • 5
  • 44
  • 54
  • 1
    Or better, check if the element has the class editing inside the handler. – Karl-André Gagnon Sep 03 '14 at 12:59
  • @Karl-AndréGagnon why is that better? – caramba Sep 03 '14 at 13:11
  • 2
    @Karl-AndréGagnon added that solution to the answer aswell. – Anton Sep 03 '14 at 13:11
  • @caramba I think this answer your question quite good : http://stackoverflow.com/a/12824698/2324107. Of course I don't know entirely your situation, but in most case, direct binding is more efficient then delegation. – Karl-André Gagnon Sep 03 '14 at 13:19
  • @Karl-André Gagnon: Delegation only requires a single handler, so is more efficient memory-wise. Catching a bubbled event does not add that much overhead (the event bubbles regardless). It also postpones the jQuery selector until execution which may be a good thing as that typically happens at UI interaction speed. – iCollect.it Ltd Sep 03 '14 at 13:20
  • @TrueBlueAussie I agree on the single handler part which make it better on the initiation. But the event handler performance is a big hit. Even bigger if it is bound to the document. Also, his application seems to have a lot of *"click without reloading"*, without necessarily having dynamic content (again, here I know nothing about his application) which would make direct binding more efficient after the `document.ready`. This is probably minor performance gain but still, someplace, delegated or direct event are better than the other. – Karl-André Gagnon Sep 03 '14 at 13:49
  • @Karl-André Gagnon: Sorry but I have to disagree with "handler performance is a big hit". The events are already bubbled to document, so still only one handler is hit. As it is only in response to a user 'click' the performance hit of the selector is negligible (in this instance). Unless you can click 50,000 times per second, then it will make a difference. I generally go with delegated events on clicks as standard :) – iCollect.it Ltd Sep 03 '14 at 13:54
  • @TrueBlueAussie *"Each time the event occurs, jQuery must compare all selectors of all attached events of that type to every element in the path from the event target up to the top of the document."* So yes, I agree the event bubble, it is native to the browser. But jQuery, behind the bar, make a comparison on all the descendant from the target to the selector. One thing direct binding doesn't do. That's also the reason why the event should be bound to the closest static parent. Anyway, i'm no expert, but that's what i've read... – Karl-André Gagnon Sep 03 '14 at 14:05
  • @Karl-André Gagnon: Yes, it should be bound to the nearest non-changing parent if one is available, but `document` is the default (body is bugged so never use that). The path from the clicked element to the document is relatively short (this is much faster than a full `find` of the same selector). As I keep saying, you are concerned over speed in a case where speed is not an issue at all. How fast can you click a mouse? :) – iCollect.it Ltd Sep 03 '14 at 14:07
0

You need to use jQuery's event.stopPropagation() function, which Prevents the event from bubbling up the DOM tree, preventing any parent handlers from being notified of the event.

$('.edit-product').not('.editing').on('click', function(e){
e.stopPropagation();
munsifali
  • 1,732
  • 2
  • 24
  • 43