CODEWITHSUNDEEP
javascripthtml
whinytween96
whinytween96

Reputation: 1965

HTML img onclick javascript function not working

Here's the html code. 

<script type="text/javascript">
function func() {

    if (document.getElementById("heart").src == "heart_empty.png") 
    {
        document.getElementById("heart").src = "heart_filled.png";
    }
    else if(document.getElementById("heart").src == "heart_filled.png")
    {
        document.getElementById("heart").src = "heart_empty.png";
    }
}
</script>
<img id="heart" src='heart_empty.png' onclick="func()" />

Javascript function func() is not working.

Upvotes: 0

Views: 857

Answers (4)

Chloe Seifert
Chloe Seifert

Reputation: 1

The second of your else if statements has one = rather than two. It needs to be a comparison, so use ==.

function func() {
    if (document.getElementById("heart").src == "heart_empty.png")  {
    document.getElementById("heart").src = "heart_filled.png";
    } else if(document.getElementById("heart").src == "heart_filled.png") {
    document.getElementById("heart").src = "heart_empty.png";
    }
}

Also, because you only have two statements, you can simply use else instead of else if.

function func() {
    if (document.getElementById("heart").src == "heart_empty.png")  {
    document.getElementById("heart").src = "heart_filled.png";
    } else (document.getElementById("heart").src == "heart_filled.png") {
    document.getElementById("heart").src = "heart_empty.png";
    }
}

Upvotes: 0

user8106201
user8106201

Reputation:

Hello

Try to use the operator "?" for this instance

Your current code looks like:

function func() {
  if (document.getElementById("heart").src == "heart_empty.png") 
   {
  document.getElementById("heart").src = "heart_filled.png";
  }
  else if(document.getElementById("heart").src == "heart_filled.png")
  {
      document.getElementById("heart").src = "heart_empty.png";
  }
}

Instead, use the "?" operator to compact your function into one line:

Also, instead of getting the element each time, add a script at the bottom of the page that looks like this:

var src = document.getElementById("heart").src;

That will not only decrease lag but also will increase readability of your code

The new script will use the fact that the "?" operator can combine an "if" statement into a one line declaration. To recap, the operator does this:

variable = condition ? /*true*/ : /*false*/

Put your (true) code in the "true" bit and same with the false bit If the condition is true, then it will set the "variable" to the code written in the "true" bit, otherwise, it will set it to the code written in the "false" bit.

So now with this newfound knowledge, we can update our code!!!

NEW CODE:

function func() {
  document.getElementById("heart").src =
  document.getElementById("heart").src === "heart_empty.png" ?
  "heart_filled.png" : "heart_empty.png";
}

This can be further compacted by using the notation stated above where "src" variable = document.getElementById("heart").src, now it looks like:

function func() {
  src = src === "heat_empty.png" ? "heart_filled.png" : "heart_empty.png";
}

Much Better! Now your code is neat, compact and flawless!

Upvotes: 0

Toby Mellor
Toby Mellor

Reputation: 8215

Your second else if has a single =. It should be a comparison, so it should use == like the first.

Always try to use the strict comparison operator === to ensure that you're dealing with the same types. If the types are different, you should convert it before comparison.

function func() {
    if (document.getElementById("heart").src == "heart_empty.png")  {
        document.getElementById("heart").src = "heart_filled.png";
    } else if(document.getElementById("heart").src == "heart_filled.png") {
        document.getElementById("heart").src = "heart_empty.png";
    }
}

In addition, since it looks like there is only ever two states, you don't have to have a second else if. Just skip to the else.

function func() {
    var heart = document.getElementById("heart");

    if (heart.src == "heart_empty.png") {
        heart.src = "heart_filled.png";
    } else {
        heart.src = "heart_empty.png";
    }
}

Upvotes: 3

DB1500
DB1500

Reputation: 155

Can you use background image instead? So have a div and depending on the class you have different image as the background.

HTML

<div class="heartImg filled"></div>

CSS

.filled {
    background-image: url(filled.jpg);
}
.empty {
    background-image: url(empty.jpg);
}

Jquery

$('.heartImg').click(function(){
    $(this).toggleClass('filled empty');
}

Upvotes: 0

Related Questions