CODEWITHSUNDEEP
javascript
Loco
Loco

Reputation: 19

Seven circles but only one is changing colour, why?

I am new to javascript and I have an assignment to create seven circles which should change colour on a mouse click. The first circle is changing colour but the other six just remain red, despite the fact I am calling the same method on a click?? I cannot use JQuery in my answer, only javascritpt. Any help is much appreciated!

My instinct was to try and create a for loop but this is not working for me..

<!DOCTYPE html>
    <html>
    <head>
    	<title>Circles of rainbow colours</title>
    	
    
    	
    </head>
    
    <body>
    <h2> Rainbow Colours</h2>
    	<svg height="1000" width="500"> 
    	<circle  id="circle1" onclick="function()"  cx="50" cy="50" r="40" 
    style="fill:red;"/>
    	<circle  id ="circle1" onclick="function()"  cx="50" cy="150" r="40" 
    style="fill:red;"/>
    	<circle  id ="circle1" onclick="function()"  cx="50" cy="250" r="40"  
    style="fill:red;"/>
    	<circle  id ="circle1" onclick="function()"  cx="50" cy="350" r="40" 
    style="fill:red;"/>
    	<circle  id ="circle1" onclick="function()"  cx="50" cy="450" r="40" 
    style="fill:red;"/>
    	<circle  id ="circle1" onclick="function()"  cx="50" cy="550" r="40" 
    style="fill:red;"/>
    	<circle  id ="circle1" onclick="function()"  cx="50" cy="650" r="40" 
    style="fill:red;"/>
    	</circle>
    	
    	</svg>
    
    
    <script>
    	
       var circle = document.getElementById("circle1");
    
       
    	colors = ['orange', 'yellow', 'green', 'blue', 'indigo','violet'];
    	
    	circle.onclick = function () 
    	
    	
    	{
    		color = colors.shift();
    		colors.push(color);
    	
    		circle.style.fill = color;
    	}
    	
    	
     </script>
    
    
    </body>
    
    </html>

Upvotes: 0

Views: 179

Answers (8)

Monday A Victor
Monday A Victor

Reputation: 471

Try it like this ` let circles = Array.from(document.querySelectorAll('circle'));

circles.forEach((circle) =>{ circle.onclick = function () { color = colors.shift(); colors.push(color); circle.style.fill = color; } } `

cheers

Upvotes: 0

jo_va
jo_va

Reputation: 13964

Use querySelectorAll('circle') to query all circles by their element name.

Also do not use the same id on all circles, element ids must be unique.

You can then use a forEach to iterate on your circles and attach your click handler with circle.addEventListener('click', handler).

Using this technique, you don't need ids or classes to refer to your circles, and you can also remove your onclick handler on each circle in your HTML:

<circle cx="50" cy="50" r="40" style="fill:red;"/>

Here is an example:

const circles = document.querySelectorAll('circle');

const colors = ['orange', 'yellow', 'green', 'blue', 'indigo','violet'];

circles.forEach(circle => {
  circle.addEventListener('click', () => {
    color = colors.shift();
    colors.push(color);
    circle.style.fill = color;
  });
});
<svg height="1000" width="500"> 
  <circle cx="50" cy="50"  r="40" style="fill:red;"/>
  <circle cx="50" cy="150" r="40" style="fill:red;"/>
  <circle cx="50" cy="250" r="40" style="fill:red;"/>
  <circle cx="50" cy="350" r="40" style="fill:red;"/>
  <circle cx="50" cy="450" r="40" style="fill:red;"/>
  <circle cx="50" cy="550" r="40" style="fill:red;"/>
  <circle cx="50" cy="650" r="40" style="fill:red;"/>
</svg>

Upvotes: 0

Nick Parsons
Nick Parsons

Reputation: 50664

Elements in HTML must have a unique id attribute, otherwise, your HTML is considered invalid.

Instead, you should use a class, this way you can iterate through your selected elements (here I have used .forEach), and apply the click handler to each.

Note: Now that you're using a class, you need to use getElementsByClassName() which will give you a HTMLCollection of matching elements.

See working example below:

const circles = [...document.getElementsByClassName("circle1")], // use spread syntax to convert collection to array
colors = ['orange', 'yellow', 'green', 'blue', 'indigo', 'violet'];

circles.forEach(circle => {
  circle.onclick = function() {
    color = colors.shift();
    colors.push(color);
    circle.style.fill = color;
  }
});
<h2> Rainbow Colours</h2>
<svg height="1000" width="500"> 
    <circle  class="circle1" onclick="function()"  cx="50" cy="50" r="40" 
style="fill:red;"/>
    <circle  class ="circle1" onclick="function()"  cx="50" cy="150" r="40" 
style="fill:red;"/>
    <circle  class ="circle1" onclick="function()"  cx="50" cy="250" r="40"  
style="fill:red;"/>
    <circle  class="circle1" onclick="function()"  cx="50" cy="350" r="40" 
style="fill:red;"/>
    <circle  class="circle1" onclick="function()"  cx="50" cy="450" r="40" 
style="fill:red;"/>
    <circle  class ="circle1" onclick="function()"  cx="50" cy="550" r="40" 
style="fill:red;"/>
    <circle  class="circle1" onclick="function()"  cx="50" cy="650" r="40" 
style="fill:red;"/>
</svg>

Upvotes: 1

Rohit Mittal
Rohit Mittal

Reputation: 2104

You need to work on class instead of a fix id. Use below code:

<!DOCTYPE html>
<html>
<head>
    <title>Circles of rainbow colours</title>
</head>

<body>
<h2> Rainbow Colours</h2>
    <svg height="1000" width="500"> 
    <circle  onclick="changecolor(this);"  cx="50" cy="50" r="40" 
style="fill:red;"/>
    <circle  onclick="changecolor(this);"  cx="50" cy="150" r="40" 
style="fill:red;"/>
    <circle  onclick="changecolor(this);"  cx="50" cy="250" r="40"  
style="fill:red;"/>
    <circle  onclick="changecolor(this);"  cx="50" cy="350" r="40" 
style="fill:red;"/>
    <circle  onclick="changecolor(this);" cx="50" cy="450" r="40" 
style="fill:red;"/>
    <circle  onclick="changecolor(this);" cx="50" cy="550" r="40" 
style="fill:red;"/>
    <circle onclick="changecolor(this);" cx="50" cy="650" r="40" 
style="fill:red;"/>
    </circle>

    </svg>


<script>

colors = ['orange', 'yellow', 'green', 'blue', 'indigo','violet'];
function changecolor(e) {
    color = colors.shift();
    colors.push(color);

    e.style.fill = color;
}
 </script>
</body>
</html>

hope it helps you.

Upvotes: 0

Ande Caleb
Ande Caleb

Reputation: 1204

id attributes are for referencing single Items, while Class attributes are for referencing group of items..

so changing ID to Class would look like this.. - 

<svg height="1000" width="500"> 
    <circle  class="circle1" onclick="function()"  cx="50" cy="50" r="40" 
style="fill:red;"/>
    <circle  class ="circle1" onclick="function()"  cx="50" cy="150" r="40" 
style="fill:red;"/>
    <circle  class ="circle1" onclick="function()"  cx="50" cy="250" r="40"  
style="fill:red;"/>
    <circle  class ="circle1" onclick="function()"  cx="50" cy="350" r="40" 
style="fill:red;"/>
    <circle  class ="circle1" onclick="function()"  cx="50" cy="450" r="40" 
style="fill:red;"/>
    <circle  class ="circle1" onclick="function()"  cx="50" cy="550" r="40" 
style="fill:red;"/>
    <circle  class ="circle1" onclick="function()"  cx="50" cy="650" r="40" 
style="fill:red;"/>
    </circle>

    </svg>

and then you get the group of items by their class name using this.. 

var circle = document.getElementsByClassName("circle1");

Upvotes: 0

brk
brk

Reputation: 50291

You have multiple element with same id. Instead of id use class and add event listener to it

var circle = document.getElementsByClassName("circle");
let colors = ['orange', 'yellow', 'green', 'blue', 'indigo', 'violet'];

[...circle].forEach(function(item, index) {
  item.addEventListener('click', function(e) {
    item.style.fill = colors[index]
  })
})
<h2> Rainbow Colours</h2>
<svg height="1000" width="500"> 
    <circle  id="circle1" class ="circle" cx="50" cy="50" r="40" 
style="fill:red;"/>
    <circle  id ="circle1" class ="circle"cx="50" cy="150" r="40" 
style="fill:red;"/>
    <circle  id ="circle1" class ="circle"  cx="50" cy="250" r="40"  
style="fill:red;"/>
    <circle  id ="circle1" class ="circle"  cx="50" cy="350" r="40" 
style="fill:red;"/>
    <circle  id ="circle1" class ="circle"  cx="50" cy="450" r="40" 
style="fill:red;"/>
    <circle  id ="circle1" class ="circle"  cx="50" cy="550" r="40" 
style="fill:red;"/>
    <circle  id ="circle1" class ="circle"  cx="50" cy="650" r="40" 
style="fill:red;"/>
</svg>

Upvotes: 0

ellipsis
ellipsis

Reputation: 12152

In the onclick function pass the element itself using this keyword. It works in this case but remember it is wrong to give same id to more than 1 element. It should be unique.

var circle = document.getElementById("circle1");


colors = ['orange', 'yellow', 'green', 'blue', 'indigo', 'violet'];
function a(e)

{
  color = colors.shift();
  colors.push(color);

  e.style.fill = color;
}
<html>
<body>
  <h2> Rainbow Colours</h2>
  <svg height="1000" width="500"> 
    <circle  id="circle1" onclick="a(this)"  cx="50" cy="50" r="40" 
style="fill:red;"/>
    <circle  id ="circle1" onclick="a(this)"  cx="50" cy="150" r="40" 
style="fill:red;"/>
    <circle  id ="circle1" onclick="a(this)"  cx="50" cy="250" r="40"  
style="fill:red;"/>
    <circle  id ="circle1" onclick="a(this)"  cx="50" cy="350" r="40" 
style="fill:red;"/>
    <circle  id ="circle1" onclick="a(this)"  cx="50" cy="450" r="40" 
style="fill:red;"/>
    <circle  id ="circle1" onclick="a(this)"  cx="50" cy="550" r="40" 
style="fill:red;"/>
    <circle  id ="circle1" onclick="a(this)"  cx="50" cy="650" r="40" 
style="fill:red;"/>
    </svg>
</body>

</html>

Upvotes: 0

baao
baao

Reputation: 73221

An id (identifier) needs to be unique, document.getElementById() always catches the first one only.

Use a class and loop

Upvotes: 8

Related Questions