CODEWITHSUNDEEP
javascripthtmlcss
enlguy
enlguy

Reputation: 87

Is there a cleaner and more repeatable way to code this onclick?

I'm coding many, many lines of these circles that are fillable based on a click function. Imagine a form where you can fill a certain number of these to denote levels of an attribute (these are for and RPG character sheet). So there are hundreds of lines of these between all the attributes and characters. Edit: The client has given me two images to use for this, an empty circle and a filled circle.
Here's how I've coded two lines of circles:

<div style="position:absolute;left:130.47px;top:155.30px" class="cls_005"><span class="cls_005">Intelligence</span></div>
<div style="position:absolute;left:213.52px;top:144.41px" class="cls_006"><span class="cls_006"><p>
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" id="imgClickAndChange" onclick="changeImage()"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" id="imgClickAndChange2" onclick="changeImageTwo()"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" id="imgClickAndChange3" onclick="changeImageThree()"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" id="imgClickAndChange4" onclick="changeImageFour()"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" id="imgClickAndChange5" onclick="changeImageFive()"  />
</p>

<script language="javascript">
    function changeImage() {

        if (document.getElementById("imgClickAndChange").src == "https://i.imgur.com/QJVkq9A.png") 
        {
            document.getElementById("imgClickAndChange").src = "https://i.imgur.com/8apyMiH.png";
        }
        else 
        {
            document.getElementById("imgClickAndChange").src = "https://i.imgur.com/QJVkq9A.png";
        }
    }
        function changeImageTwo() {

        if (document.getElementById("imgClickAndChange2").src == "https://i.imgur.com/QJVkq9A.png") 
        {
            document.getElementById("imgClickAndChange2").src = "https://i.imgur.com/8apyMiH.png";
        }
        else 
        {
            document.getElementById("imgClickAndChange2").src = "https://i.imgur.com/QJVkq9A.png";
        }
    }
        function changeImageThree() {

        if (document.getElementById("imgClickAndChange3").src == "https://i.imgur.com/QJVkq9A.png") 
        {
            document.getElementById("imgClickAndChange3").src = "https://i.imgur.com/8apyMiH.png";
        }
        else 
        {
            document.getElementById("imgClickAndChange3").src = "https://i.imgur.com/QJVkq9A.png";
        }
    }
            function changeImageFour() {

        if (document.getElementById("imgClickAndChange4").src == "https://i.imgur.com/QJVkq9A.png") 
        {
            document.getElementById("imgClickAndChange4").src = "https://i.imgur.com/8apyMiH.png";
        }
        else 
        {
            document.getElementById("imgClickAndChange4").src = "https://i.imgur.com/QJVkq9A.png";
        }
    }
            function changeImageFive() {

        if (document.getElementById("imgClickAndChange5").src == "https://i.imgur.com/QJVkq9A.png") 
        {
            document.getElementById("imgClickAndChange5").src = "https://i.imgur.com/8apyMiH.png";
        }
        else 
        {
            document.getElementById("imgClickAndChange5").src = "https://i.imgur.com/QJVkq9A.png";
        }
    }
</script></span></div>
<div style="position:absolute;left:600.48px;top:155.30px" class="cls_005"><span class="cls_005">Strength</span></div>
<div style="position:absolute;left:650.53px;top:157.41px" class="cls_006"><span class="cls_006"><p>
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" id="imgClickAndChange6" onclick="changeImageSix()"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" id="imgClickAndChange7" onclick="changeImageSeven()"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" id="imgClickAndChange8" onclick="changeImageEight()"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" id="imgClickAndChange9" onclick="changeImageNine()"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" id="imgClickAndChange10" onclick="changeImageTen()"  />
</p>

<script language="javascript">
    function changeImageSix() {

        if (document.getElementById("imgClickAndChange6").src == "https://i.imgur.com/QJVkq9A.png") 
        {
            document.getElementById("imgClickAndChange6").src = "https://i.imgur.com/8apyMiH.png";
        }
        else 
        {
            document.getElementById("imgClickAndChange6").src = "https://i.imgur.com/QJVkq9A.png";
        }
    }
        function changeImageSeven() {

        if (document.getElementById("imgClickAndChange7").src == "https://i.imgur.com/QJVkq9A.png") 
        {
            document.getElementById("imgClickAndChange7").src = "https://i.imgur.com/8apyMiH.png";
        }
        else 
        {
            document.getElementById("imgClickAndChange7").src = "https://i.imgur.com/QJVkq9A.png";
        }
    }
        function changeImageEight() {

        if (document.getElementById("imgClickAndChange8").src == "https://i.imgur.com/QJVkq9A.png") 
        {
            document.getElementById("imgClickAndChange8").src = "https://i.imgur.com/8apyMiH.png";
        }
        else 
        {
            document.getElementById("imgClickAndChange8").src = "https://i.imgur.com/QJVkq9A.png";
        }
    }
            function changeImageNine() {

        if (document.getElementById("imgClickAndChange9").src == "https://i.imgur.com/QJVkq9A.png") 
        {
            document.getElementById("imgClickAndChange9").src = "https://i.imgur.com/8apyMiH.png";
        }
        else 
        {
            document.getElementById("imgClickAndChange9").src = "https://i.imgur.com/QJVkq9A.png";
        }
    }
            function changeImageTen() {

        if (document.getElementById("imgClickAndChange10").src == "https://i.imgur.com/QJVkq9A.png") 
        {
            document.getElementById("imgClickAndChange10").src = "https://i.imgur.com/8apyMiH.png";
        }
        else 
        {
            document.getElementById("imgClickAndChange10").src = "https://i.imgur.com/QJVkq9A.png";
        }
    }
</script></span></div>

So, as you can see, lots of lines of code. Would love a way to simplify this. Thank your for any thoughts.

Upvotes: 0

Views: 56

Answers (3)

KooiInc
KooiInc

Reputation: 122986

It's generally not a good idea to use inline event handlers. You can use one handler for the document and use a generic handler (event delegation). In this case something like the given snippet. In the snippet a data-attribute is used to recognize an image

document.addEventListener("click", changeImage);

function changeImage(evt) {
  const origin = evt.target;
  if (origin.dataset.togglesrc) {
    const swap = {"8apyMiH.png": "QJVkq9A.png", "QJVkq9A.png": "8apyMiH.png"};
    const imgRoot = origin.src.substr(0, origin.src.lastIndexOf("/"));
    const currentImg = origin.src.split("/").slice(-1);
    origin.src = `${imgRoot}/${swap[currentImg]}`;
  }
}
body {margin: 2rem;}
<img alt="" src="https://i.imgur.com/QJVkq9A.png" data-togglesrc="1"
      style="height: 20px; width: 20px" id="imgClickAndChange6" />
  <img alt="" src="https://i.imgur.com/QJVkq9A.png" data-togglesrc="1"
      style="height: 20px; width: 20px" id="imgClickAndChange7" />
  <img alt="" src="https://i.imgur.com/QJVkq9A.png" data-togglesrc="1"
      style="height: 20px; width: 20px" id="imgClickAndChange8" />
  <img alt="" src="https://i.imgur.com/QJVkq9A.png" data-togglesrc="1"
      style="height: 20px; width: 20px" id="imgClickAndChange9"  />
  <img alt="" src="https://i.imgur.com/QJVkq9A.png" data-togglesrc="1"
      style="height: 20px; width: 20px" id="imgClickAndChange10" />

Alternatively you can create a more generic factory function that returns a handler and receives the image file names as parameter:

document.addEventListener("click", swapper("JouzaLc.png", "rgqRxiB.png"));

function swapper(...images) {
  const swap = {
    [images[0]]: images[1],
    [images[1]]: images[0],
  };
  // return a listener function
  // the function uses the 'closed over' [swap] variable
  return evt => {
    const origin = evt.target;
    if (origin.dataset.imageId) {
      const imgRoot = origin.src.substr(0, origin.src.lastIndexOf("/"));
      const currentImg = origin.src.split("/").slice(-1).pop();
      origin.src = `${imgRoot}/${swap[currentImg]}`;
      console.clear();
      console.log(`You clicked image ${origin.dataset.imageId}, its src is now ${
        swap[currentImg]}`);
    }
  }
}
body {
  margin: 2rem;
}

img {
  width: 50px;
  height: auto;
}
<p>
  <img src="https://i.imgur.com/JouzaLc.png" data-image-id="11" />
  <img src="https://i.imgur.com/JouzaLc.png" data-image-id="2" />
  <img src="https://i.imgur.com/JouzaLc.png" data-image-id="23" />
  <img src="https://i.imgur.com/JouzaLc.png" data-image-id="46" />
  <img src="https://i.imgur.com/JouzaLc.png" data-image-id="15" />
</p>

Yet another idea may be to also use data-attibutes for the images to swap. That one can be found in a JsBin

Upvotes: 2

Jona
Jona

Reputation: 1061

Sounds like you need to look into using function parameters better. All your functions are doing the same thing, with different parameters - in your case, the parameters are an element's ID and two image URLs.

The point of a function is not just to encapsulate some code (i.e., separate a chunk of code into a block, as you're doing) - but to take some input (parameters), manipulate it, and return some output.

In your case, you might consider a function that looks like this:

const changeImageSrc = function(imageID, oldURL, newURL){
  const image = document.getElementById(imageID)
  if(image.src === oldURL){
    image.src = newURL
  }
}

Now you can call this function as many times as you like, each time passing it the relevant parameters:

changeImageSrc("imgClickAndChange10", "https://i.imgur.com/QJVkq9A.png", "https://i.imgur.com/8apyMiH.png")
changeImageSrc("imgClickAndChange9", ...)

There's other changes you can make too, like storing all your URLs/IDs in an array, or even getting them from the got go using document.getElementsByClassName if you're setting your HTML up a certain way, then looping to make your life easier - but that's a whole other story.

Upvotes: 1

Alfredo Rafael
Alfredo Rafael

Reputation: 27

If you are using the same functionality for all the images, you only need one function, you can pass as parameter the DOM HTML element with this, something like:

<img alt="" src="https://i.imgur.com/QJVkq9A.png" 
style="height: 10px; width: 10px" onclick="changeImage(this)"  />

And your adapted function will look something like this:

function changeImage(element) {
      const src = element.src
      src == "https://i.imgur.com/QJVkq9A.png" ? element.src = "https://i.imgur.com/8apyMiH.png" : element.src = "https://i.imgur.com/QJVkq9A.png";
  }

Your code will look something like:

<script language="javascript">
  function changeImage(element) {
      const src = element.src
      src == "https://i.imgur.com/QJVkq9A.png" ? element.src = "https://i.imgur.com/8apyMiH.png" : element.src = "https://i.imgur.com/QJVkq9A.png";
  }
</script>

<div style="position:absolute;left:130.47px;top:155.30px" class="cls_005"><span class="cls_005">Intelligence</span></div>
<div style="position:absolute;left:213.52px;top:144.41px" class="cls_006"><span class="cls_006"><p>
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" onclick="changeImage(this)"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" onclick="changeImage(this)"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" onclick="changeImage(this)"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" onclick="changeImage(this)"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" onclick="changeImage(this)"  />
</p>
</span></div>
<div style="position:absolute;left:600.48px;top:155.30px" class="cls_005"><span class="cls_005">Strength</span></div>
<div style="position:absolute;left:650.53px;top:157.41px" class="cls_006"><span class="cls_006"><p>
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" onclick="changeImage(this)"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" onclick="changeImage(this)"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" onclick="changeImage(this)"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" onclick="changeImage(this)"  />
        <img alt="" src="https://i.imgur.com/QJVkq9A.png" 
            style="height: 10px; width: 10px" onclick="changeImage(this)"  />
</p>
</span></div>

Upvotes: 0

Related Questions