LeeTee
LeeTee

Reputation: 6601

jquery toggle buttons

I have a list of items which have an open /close button next to each of them. I would like to toggle the open / close buttons. This seems to be work the first time the button is selected but then the function isn't initiated the second time the button is selected. Anyone know why?

    //update status - close
    $("#close_<?=$order_id?>").click(function(){
        alert('close');
        order_update_type = 'updatestatus';
        order_status = '2';
        order_selected.set("status", order_status); 
        $("#close_<?=$order_id?>").replaceWith('<a data-align="left" class="km-button" data-role="button" id="open_<?=$order_id?>" style="float:left; margin:5px;"><span class="km-text">Re-open</span></a>');      
      });         

//update status - re-open
    $("#open_<?=$order_id?>").click(function(){
        alert('open');
        order_update_type = 'updatestatus';
        order_status = '1';
        $("#open_<?=$order_id?>").replaceWith('<a data-align="left" class="km-button" data-role="button" id="close_<?=$order_id?>" style="float:left; margin:5px;"><span class="km-text">Close</span></a>');            
      });

Upvotes: 1

Views: 524

Answers (5)

Aadaam
Aadaam

Reputation: 3739

Because 'click' is only binded to the first element, which you're replacing.

The brute force solution would be to use live('click', function..) instead of click(function..)

However, my recommendations:

  1. Create a class, like 'orderbutton'
  2. bind the click to that class, with live if necessary
  3. regain the order ID from javascript - never generate javascript code from PHP (except for JSON)
  4. create a 'collapsed' version which is originally hidden - remember, it needs to have a different ID (perhaps you should use data attribute instead of the ID attribute)
  5. move all your style definition to classes instead of inline style

edit:

Recommended HTML:

<a data-align="left" data-orderId="<?$order_id?>" 
    class="km-button orderbutton closed" 
    data-role="button" 
    id="open_<?=$order_id?>">
        <span class="km-text reopen">Re-open</span>
        <span class="km-text close">close</span>
</a>

Recommended CSS file:

.orderbutton {
   float:left; 
   margin:5px;
}
.orderbutton.closed .close {
   display:none;
}
.orderbutton.open .reopen {
   display: none;
}

Recommended JS:

$('a.orderbutton').live('click', function(){
    var order_id = $(this).data('orderId');
    var is_closed = ($(this).hasClass('closed');
    order_update_type = 'updatestatus';
    order_status = is_closed ? '2' : '1';
    order_selected.set("status", order_status); 


    if ($(this).hasClass('closed')) {
         $(this).removeClass('closed');
         $(this).addClass('open');
    } else if ($(this).hasClass('open')){
         $(this).removeClass('open');
         $(this).addClass('closed');
    }
});

Or, in shorter form:

$('a.orderbutton').live('click', function(){
    var order_id = $(this).data('orderId');
    var is_closed = ($(this).hasClass('closed');
    order_update_type = 'updatestatus';
    order_status = is_closed ? '2' : '1';
    order_selected.set("status", order_status); 

    $(this).toggleClass('open', $(this).hasClass('closed');
    $(this).toggleClass('closed', $(this).hasClass('open');
});

Upvotes: 3

RoToRa
RoToRa

Reputation: 38390

Several points (Mostly extending Aadaam's):

  1. Don't generate the JavaScript with the order number hard-coded into the selector - as a matter of fact don't put the order number into the id at all, otherwise you'll be to unnecessarily repeating the same code for each button.

  2. Since the open and close buttons are identical save the id (which you shouldn't be using like that as in point 1) and the text, don't replace the button but just toggle the text and a class as a marker:

(NB: Don't hard code styles and the class on the inner span seems unneccerery. You can select it in CSS using km-buttom > span instead.)

Example:

<a data-align="left" class="km-button" data-role="button" data-order-id="<?=$order_id?>" class="closed"><span>Re-open</span></a>

$('.km-button').click(function() { 
  var $this = $(this);
  var orderId = $this.data('order-id'); // Get Order id if you need it for something
  if ($this.hasClass('closed')) {
    // Do what needed
    $this.find('span').text('Close');
  } else {
    // Do what needed
    $this.find('span').text('Re-open');
  }
  $this.toggleClass('closed');
});

I'm assuming all your order buttons have the class .km-button to select them all at once. You may need an alternative e.g. an attribute selector [data-order-id].

Upvotes: 0

Sibu
Sibu

Reputation: 4617

why not use jquery slidetoggle

$("#first").slideToggle('slow');

Upvotes: 0

Ram
Ram

Reputation: 144659

As you are replacing the elements, you should delegate the click event, try the following:

$(document).on("click", "#close_<?=$order_id?>", function(){
     // ...      
}); 

$(document).on("click", "#open_<?=$order_id?>", function(){
     // ...      
}); 

Upvotes: 1

Dessus
Dessus

Reputation: 2177

Rather than .click you must use .live because click only works for elements in the DOM at the time of setting up the click event. The live subscription looks for DOM updates also.

    //update status - close
    $("#close_<?=$order_id?>").live('click',function(){
        alert('close');
        order_update_type = 'updatestatus';
        order_status = '2';
        order_selected.set("status", order_status); 
        $("#close_<?=$order_id?>").replaceWith('<a data-align="left" class="km-button" data-role="button" id="open_<?=$order_id?>" style="float:left; margin:5px;"><span class="km-text">Re-open</span></a>');      
      });         

    //update status - re-open
    $("#open_<?=$order_id?>").live('click',function(){
        alert('open');
        order_update_type = 'updatestatus';
        order_status = '1';
        $("#open_<?=$order_id?>").replaceWith('<a data-align="left" class="km-button" data-role="button" id="close_<?=$order_id?>" style="float:left; margin:5px;"><span class="km-text">Close</span></a>');            
      });

Upvotes: 1

Related Questions