Jump to content
Larry Ullman's Book Forums

Removing Objects From Array On Button Click Event


Recommended Posts

Hey, Guys

 

I'm currently working on car advertisement system. Every add in the system must have a customer benefit for it to be valid. That benefit may be defined as a deducted money amount from the total or as one or more "add-ons" included without charge. Add-ons might be a GPS-system worth $x dollars and Air-condition worth $y dollars. The total customer benefit is summarized from the two, and is thus:

Total = deducted_amount + (total_value_of_all_add_ons);

The total is set using a total of three input fields.

Add-ons:

---> Input 1: Name of the add-on, for example "GPS-system"

---> Input 2: Value of the add-on, for example  "1000"

Deducted price:

---> Input 3: Fixed deducted amount, for example: "2000"

 

When adding an add-on, the following happens:

- The add-on's price is added to the total

- An JSON object is added to an array by partsList.push({"part":"GPS", "value="1000"}

- An List element is added to a definition list (<dl></dl>) on the form (<dt>GPS</dt><dd>1000 <button.....>Remove</button></dd>).

 

Problems:

1. How do I remove an object from the array by using the "Remove"-button seen right above here? (Calling the function NYBG.removeAddOnAt(POS) with the correct position);

2. How do I remove the definition item from the list when the Remove-button is clicked?

 

See code and the script in action:

http://juvenorge.no/dev/prototype.php (Add some items at the top first)

 

Hope someone can help out here. :)

Link to comment
Share on other sites

1) It looks like you already have a removeAddOnAt method in your code that calls the splice method with the pos parameter.

That seems sufficient. Is that not working?

Basically, I'd set up an onclick event handler for each of the Remove buttons, and when one is clicked, send the appropriate position argument to the removeAddOnAt method.

 

2) After removing the proper element from the array of add-ons (see #1), I'd re-display the list with the modified array.

 

I feel like I didn't really answer your question for #1, but I'm not really sure what you're asking or what the problem is.

Please lemme know, and I'll try to help more.

 

By the way, your JS is looking pretty darn good these days.

Nice job!

Link to comment
Share on other sites

Basically, I have problems writing the onclick listener you describe. How can I make the handler differentiate between the buttons, calling the  NYBG.removeAddOnAt(POS) function with the position at the button and rebuild the list.

 

I need something along the lines of: (some pseudo-code here)

// Target ALL buttons with the class .removeAddOn
$(".removeAddOn").click(function(){
      alert("1"); 
      // Then I must get the ID... Something along the lines of...
      // But I'm not really sure how to do it...
      var id = $(this).id;
      NYBG.removeAddOnAt(id);
      // rebuild list with NYBG.getAddOnList()
});

But that won't activate when I click on any of the buttons. Once I've bound an event listener to it, I also need a way to get the ID to delete. When that is done, I can simply build the definition list again as you pointed out.

 

Edit: Thank you for the nice words. I'm doing my best. :)

Edited by Antonio Conte
Link to comment
Share on other sites

Oh, okay. I thought that was what you were asking, but I didn't want to assume anything, which is why I wanted to wait for your response.

 

There are two ways you can do this: the simpler way, and the way I prefer (i.e., the harder way).

 

For the simpler way, you basically do what you suggested, which is use the ID of the button you clicked on to figure out what the position is.

Looking at your code, I can see that you have IDs like "remove0" and "remove1" attached to the Remove buttons. You can use that number part at the end to get the proper position in the array.

There are any number of ways to do this in JS, but probably the easiest would be to do the following within the callback function assigned to the onclick event:

 



parseInt(this.id.substr(6), 10);


 

this.id corresponds to the ID of the button, substr(6) gets a substring of the ID from the 7th character and on, and parseInt will turn the returned substring into an integer.

Of course, this assumes that there are always exactly six characters before the number you want, but if all your buttons start with "remove", then this should work for any number.

You can also use a regex for more flexibility, but I think that that's total overkill in this case.

 

For the method I prefer though, you don't need the ID at all.

This is nice because you can make the ID whatever you want, and you don't have to worry about it. To do this though, we're going to have to use JS closures to our advantage, which is slightly more advanced. Also, I'm going to use vanilla JS for demo purposes, as I don't know the exact jQuery code necessary to do this off the top of my head. Adapting it to jQuery should be easy enough though.

 

Anyway, you first want to get a NodeList (i.e., an array) of all the Remove buttons as following:

 



var btns = document.getElementById('total').getElementsByTagName('button');


 

You then want to loop through the NodeList and assign an onclick event to each button as follows:

 



var i,
  len;


for (i = 0, len = btns.length; i < len; i += 1) {
  
  btns[i].onclick = removeAddOn(i);
  
}


 

The key to this is the sending of the i argument to the callback function.

This is confusing to most people because you're not allowed to assign a function call to an event handler; you have to assign a function reference instead.

However, within the removeAddOn function, if we return a function reference, then the event handler will be properly set up, and we can use the i variable in the state it's in each time the removeAddOn function is called within the removeAddOn function. This is JS closure!

 

And as I just mentioned, you now need to return a function within the removeAddOn function to get this to work. The following should be sufficient:

 



function removeAddOn(pos) {
  
  return function () {
    
    // Call removeAddOnAt method here with the pos parameter passed to the removeAddOn function.
    // Re-display the list with the new array here.
    
  };
  
}


 

Anyway, you can use either method, but I prefer the second method, as it's more flexible and less dependent on the markup, and it takes advantage of JS closures, which requires less processing in the end.

Please let me know if this helps.

  • Upvote 1
Link to comment
Share on other sites

I'm familiar with clojures. You should know that Laravel are heavily based on them. That was one of the things that impressed me the most, as some of their solutions result in extremely beautiful code. :)

 

I'm not really familiar with the DOM yet, though. The definition list is obviously rebuild each time any of the event listeners ran. I only attached the listeners to the buttons when the addOn event was caught. That took me longer to figure out than I care to admit...

My solution was to add a new method where everything regarding results was done. I attached the listeners there instead.

 

Thanks, Jon. You rock!

Link to comment
Share on other sites

Yes, each time you use innerHTML (or some (jQuery) derivative thereof), you have to re-attach the event handlers to the Remove buttons.

I don't know if you need a new method for that purpose, but whatever works for you, eh?

 

So at this point, do you have it working the way you want?

Link to comment
Share on other sites

Yeah, it works perfectly. I just updated the original linked script, so you can see the code there. I aso simplified it a bit by adding the IDs directly to the heading and definition list elements.

 

Benefit.js: http://juvenorge.no/dev/benefit.js

Script: http://juvenorge.no/dev/prototype.php

 

This is the most relevant, new code:

    // Calculate data totals
    function calculateTotals() {
        // Display total and add-on list
        total.text("Total " + NYBG.getFormatedTotal() + ",- kr");
        list.html(NYBG.getAddOnList());

		// Assign values to hidden input fields
		benefitValue.val(NYBG.getTotal());
		benefitList.val(NYBG.getAddOnListAsJSON());
      
        // Attach new button listeners
        setButtonListeners();
    }
	
	// Adds button listeners to all buttons
	function setButtonListeners() {
		var i,
			size;
		
		btns = document.getElementById('total-list').getElementsByTagName('button');
		
		for (i = 0, size = btns.length; i < size; i+=1) {
			btns[i].onclick = setClojureForRemoveAddOnAt(i);
		}
	}
	
	// Closure for removing add-ons on click events
	function setClojureForRemoveAddOnAt(pos) {
		return function () {
			// Remove object from array
			NYBG.removeAddOnAt(pos);

			// Calculate values
            calculateTotals();
		};
	}

The other event listeners simple call calculateTotals() now, and so does the closure. I think it was a very good solution, Jon. I'm wouldn't have thought of this on my own.

 

Thank you very much!

Link to comment
Share on other sites

No problem. Glad you got it working with the more elegant closure solution.

By the way, you can port my code over to jQuery if you want. For example, instead of using:

btns = document.getElementById('total-list').getElementsByTagName('button');

You could use:

btns = $('#total-list button');

Also, not sure if you're aware of this, but in your code, you didn't put the var keyword in front of btns, which causes the btns variable to be implicitly declared globally. Not sure if that's an issue for you or not.

  • Upvote 1
Link to comment
Share on other sites

Nah. I think I'll port the other way around to be honest with you. I've used mostly jQuery for referencing DOM elements so far as I've found it easier. I had no luck with innerHTML or val, so I went for .text(), .val() and .html() instead. I've gotten that to work recently, so I don't really need the jQuery stuff that much now. I have to include it because of Bootstrap anyway, but I'll try to keep my own code independent from any framework.

 

One step at the time, though. ;)

 

About the global state. btns is declared just below NYBG, so it should be local scope. That's an easy miss, though. Thanks anyway.

Link to comment
Share on other sites

 Share

×
×
  • Create New...