3

I don't know if the title explain this situation well. Below is a code i wrote to create div elements when the button is pressed. Then By clicking on any of the created divs, we can change the div background by choosing a color from the drop down box. However if i click on another div and tried to change the color by selecting another color from the drop down, the previously clicked divs also gets affected by the new color. Why is this happening. I only want the selected div to change color, without affecting the previously clicked divs. I read allot of threads on this site, some of which talks about unbinding clicks, but I'm unable to solve the problem. Thanks for the help.

<script src="http://ajax.googleapis.com/ajax/libs/jquery/1.4.4/jquery.min.js" type="text/javascript"></script> 

<style> 
.aaa { width:100px; height:100px;background-color:#ccc;margin-bottom:5px;}
p{widht:500px; height:500px; margin:0px; padding:0px;border:2px solid red;color:#000;}
</style>

<select id="item_select" name="item"> 
    <option value="1">red</option> 
    <option value="2">blue</option> 
    <option value="3">green</option> 
</select>

<button>Click to create divs</button>
<p id="ptest"></p>


<script type="text/javascript">
var dividcount = 1;
$("button").click(run);
function run(){
 var myDiv = $('<div id=lar'+dividcount+' class=aaa></div>');
 $(myDiv).clone().appendTo('#ptest');
 $(dividcount++);
 $("div").bind('click',(function() {
  var box = $("div").index(this);
  var idd = (this.id);
  $("#"+idd).text("div #"+idd);
  $("select").change(function(){
   var str= $("select option:selected").text();
   $("#"+idd).css("background-color",str);
  });
 }));
}; 
</script>
Hussein
  • 42,480
  • 25
  • 113
  • 143

3 Answers3

0

Change this:

$("select").change(function(){
   var str= $("select option:selected").text();
   $("#"+idd).css("background-color",str);
});

to this:

$(this).find("select").change(function()
    var str= $(this).find("option:selected").text();
    $("#"+ idd ).css("background-color",str);
});

The problem as I see it is that each time you click on a div your are binding a change handler to all selects on the page. Make your code code search for the select within the clicked div, rather than all the selects on the page.

karim79
  • 339,989
  • 67
  • 413
  • 406
0

Finished Example: http://jsfiddle.net/fUHFp/1/ (if I understand what you're intending)

By doing this:

$("div").bind('click',(function() { 
    // ...

You're assigning a click event to every div on the page every time the button is clicked.

I assume you just want to assign it to the newly created one.

Instead of this:

$("div").bind('click',(function() {
  var box = $("div").index(this);
  var idd = (this.id);
  // ...

do this:

myDiv.bind('click',(function() {
  var box = $("div").index(this);
  var idd = (this.id);
  // ...

so you're only binding to the new <div> element.

The same will happen with the change() event you're binding inside this click as @karim79 pointed out.

Also, this line:

$(dividcount++);

...doesn't make sense. You don't need to wrap everything in a jQuery object.

Just do this:

dividcount++;

Also, you don't need to clone the <div> you just created, since you're not retaining the original. And you don't need to wrap myDiv in a jQuery object, since it already is one.

So this:

 $(myDiv).clone().appendTo('#ptest');

...could be this:

 myDiv.appendTo('#ptest');

EDIT: With regard to the .change() event, it appears as though you're trying to change the color of all the dynamic divs when you change the <select>. If so that isn't the way to do it.

You should instead select those divs by their class name aaa in the change() handler to set the color.

So take the change handler out of the run() function completely, and change it to this:

$("select").change(function(){
   var str= $(this).find("option:selected").text();
   $(".aaa").css("background-color",str);
});

Also, there's no reason to rebuild a selector for the element you already have inside the click() handler. You can pass this to a jQuery object.

So this:

$("#"+idd).text("div #"+idd);

should be this:

$(this).text("div #"+idd);
Community
  • 1
  • 1
user113716
  • 318,772
  • 63
  • 451
  • 440
0

I've setup a fiddle @ http://jsfiddle.net/BUghk/

Your code made my eyes bleed. So I've took the liberty to rewrite it :)

PeeHaa
  • 71,436
  • 58
  • 190
  • 262
  • 1
    Best answer and nice easy to read code. Thank you. In this code we are assuming the clicked item is a div. Now what if the clicked element is not a div and we need to figure it out before we can apply color selections. – Hussein Jan 04 '11 at 01:29