1

I have a problem when I am making the website for one gallery. I made the code for the button that can show and hide multiple images. I intend to make the button can place several images in randomly. I write the code that can function for only one image.

Please tell me the code that functions as a button to place multiple images in a random location.

  • Users can hide images by pressing the button.
  • And when users press the button again, it places the images in another random location.

const btn = document.querySelector("button");
const height = document.documentElement.clientHeight;
const width = document.documentElement.clientWidth;
const box = document.getElementById("color");

btn.addEventListener("click", () => {
  let randY = Math.floor((Math.random() * height) + 1);
  let randX = Math.floor((Math.random() * width) + 1);
  box.style.top = randY + "px";
  box.style.right = randX + "px";
});


function showhide() {
  var x = document.querySelectorAll("#color");
  var i;
  for (i = 0; i < x.length; i++) {
    if (x[i].style.display === "block") {
      x[i].style.display = "none";
    } else {
      x[i].style.display =
        "block";
    }
  }
}
body {
  height: 500px;
}

.random {
  position: absolute;
}
<button onclick="showhide()" value="Zeige Features" id="button">click me</button>

<img id="color" style="display: none;" class="random" src="http://lorempixel.com/200/200/">

<img id="color" style="display: none;" class="random" src="http://lorempixel.com/200/200/">
Rickard Elimää
  • 7,107
  • 3
  • 14
  • 30
jieun kim
  • 27
  • 3
  • 1
    Is has to be unique, so you sill always only get one element if you select `document.getElementById()` If you need to have multiple elements, use classes for this. – cloned Jul 02 '21 at 09:41
  • You had included JQuery, but not using any JQuery in the code. So I removed it. – Rickard Elimää Jul 02 '21 at 09:44
  • thanks for your comment, first of all. can you comment more in detail? I am not a coding expert man so, I need a comment with coding example. – jieun kim Jul 02 '21 at 09:47

2 Answers2

0
  1. You're doing the correct thing in showHide() when using querySelectorAll. You are then able to get all images.

  2. You should never have elements with the same ids. They should be unique. So querySelectorAll("#color") works, but it's now how you should do. Do a querySelector on "img.random" instead.

  3. getElementById only returns a single element, not like querySelectorAll. So you need to use querySelectorAll('img.random').


This might be beyond your knowledge, I don't think you should add the images in HTML, but in javascript code.

a) Add all image paths in an array: ['https://image.com/image.png', ...]

b) Add a single img element. <img id="template" class="random">

c) In javascript code, clone that element for each image path in the array. You can use cloneNode for this.

d) Randomize each position for each element, just like you have done now.

e) Add each element to the DOM through appendChild. Have a unique div that you append to. Be sure to clear it every time second time you hit the button.

f) Solve all bugs along the way. :P

Rickard Elimää
  • 7,107
  • 3
  • 14
  • 30
0

The problem

The main issue here is that you're using getElementById to query #color

const box = document.getElementById("color");

Since getElementById only returns one element (but you have two in your DOM) and the style only applies to one element. That's why you're seeing only one element is randomly moving and the other just stay in the same place.

A side note here, id should be unique in a DOM.

You're in fact using the correct API for the job in the showhide function

var x = document.querySelectorAll("#color");

The fix:

To fix this, you need to query all images by their classname (as suggested in the side note, don't use id for the job)

const boxes = document.querySelectorAll(".random");

Now we have a node list, as you do in the showhide function, we need to loop thru it, I'm not using a for loop here, instead, a forEach loop, it's just more terser and a modern addition to the JS

// Since boxes are not array, we need to covert it to array so we can use that handy `.forEach` here:
Array.from(boxes).forEach(box => {
  box.style.top = Math.floor((Math.random() * height) + 1) + "px";
  box.style.right = Math.floor((Math.random() * width) + 1) + "px";
})

Now, this should fix your issue. See the complete code below.

const btn = document.querySelector("button");
const height = document.documentElement.clientHeight;
const width = document.documentElement.clientWidth;
const boxes = document.querySelectorAll(".random");

btn.addEventListener("click", () => {
  Array.from(boxes).forEach(box => {
    box.style.top = Math.floor((Math.random() * height) + 1) + "px";
    box.style.right = Math.floor((Math.random() * width) + 1) + "px";
  })
});

function showhide() {
  var x = document.querySelectorAll(".random");
  var i;
  for (i = 0; i < x.length; i++) {
    if (x[i].style.display === "block") {
      x[i].style.display = "none";
    } else {
      x[i].style.display =
        "block";
    }
  }
}
body {
  height: 500px;
}

.random {
  position: absolute;
}
<button onclick="showhide()" value="Zeige Features" id="button">click me</button>

<img id="color" style="display: none;" class="random" src="http://lorempixel.com/200/200/">
<img id="color" style="display: none;" class="random" src="http://lorempixel.com/100/100/">
Joshua
  • 3,055
  • 3
  • 22
  • 37