The click events runs only after clicking twice or three times-CodePudding

    <html>
<head>
    <title>Random manga</title>
                    <script src="./js/my.js"></script>
    <link type="text/css" rel="stylesheet" href="./css/style.css">
</head>
<body>

<div id="al">

<div id="pic">
<img src="" id="img" style="visibility: hidden;">
</div>  
    <button id="btng" onclick="my()">Suggest me manga</button>
</div>

</body>
</html>

Here is the html for button div and other things Below is the css

    body
{
    background : white;
    color : black;
}

#pic
{
    border-radius : 25px;
    background-color : #f2f2f2;
    -webkit-border-radius : 35px;
    -moz-border-radius : 75px;
    box-shadow: rgba(0, 0, 0, 0.25) 0px 54px 55px, rgba(0, 0, 0, 0.12) 0px -12px 30px, rgba(0, 0, 0, 0.12) 0px 4px 6px, rgba(0, 0, 0, 0.17) 0px 12px 13px, rgba(0, 0, 0, 0.09) 0px -3px 5px;
    background-size : cover;
    
    text-align : center;
    height: 400px;
    width: 400px;
    background-size : cover;
    background-repeat: no-repeat;
    background-position: center; 
}


#btng

 {
  
  backface-visibility: hidden;
  background-color: #405cf5;
  border-radius: 6px;
  border-width: 0;
  box-shadow: rgba(50, 50, 93, .1) 0 0 0 1px inset,rgba(50, 50, 93, .1) 0 2px 5px 0,rgba(0, 0, 0, .07) 0 1px 1px 0;
  
  color: #fff;
  
  
  font-size: 100%;
  height: 44px;
  line-height: 1.15;
  margin: 12px 0 0;
  outline: none;
  overflow: hidden;
  padding: 0 25px;
  position: relative;
  text-align: center;
  text-transform: none;
  transform: translateZ(0);
  transition: all .2s,box-shadow .08s ease-in;
  

  width: 50%;
}



#btng:focus {
  box-shadow: rgba(50, 50, 93, .1) 0 0 0 1px inset, rgba(50, 50, 93, .2) 0 6px 15px 0, rgba(0, 0, 0, .1) 0 2px 2px 0, rgba(50, 151, 211, .3) 0 0 0 4px;
}
  


#al
{
    
    text-align : center;
    position : absolute;
    top : 50%;
    left : 50%;
    transform : translate(-50%, -50%);
}
#img{
    height: 100%;
    width: 100%;
}

And below the function for onclick on button

function my(){
    var bt=document.getElementById("btng");
    bt.textContent = "Suggest me another";
var my = new Array("im/R.jpg","im/S.jpg","im/E.jpg");
    var randomNum = Math.floor(Math.random() * my.length);
   var backgroundImage = my[randomNum];
     document.getElementById("img").src = my[randomNum];
document.getElementById("pic").style.backgroundImage=`url(${backgroundImage})`;


}

The onclick event only runs after the button is clicked twice or thrice sometimes I tried doing something to slove it but I was unsuccessful, is there any way to do it? Here is the live preview:- https://mangasuggestions.000webhostapp.com/index.html

Please click on buttons 4/5 times to get the problem I am facing.

CodePudding user response：

The onclick event and the function is working properly. The image didn't update is because the randomNum is the same as the previous one.

Our goal is to prevent using the same number that is generated previously.

One way to do this is to keep track of the last generated random number, and regenerate until it is different from the previous one.

Main logic

    let randomNum;
    do {
        randomNum = Math.floor(Math.random() * images.length);
    } while (randomNum === prevRandomNum);
    prevRandomNum = randomNum

Full code (With some modification of your original code)

// Keep track of the last random number
let prevRandomNum = -1

function my(){
    const btn = document.getElementById("btng");
    btn.innerText = "Suggest me another";
    const images = ["im/R.jpg","im/S.jpg","im/E.jpg"];

    let randomNum;
    do {
        randomNum = Math.floor(Math.random() * images.length);
    } while (randomNum === prevRandomNum);
    prevRandomNum = randomNum

    const backgroundImage = images[randomNum];    
    document.getElementById("pic").style.backgroundImage=`url(${backgroundImage})`;
}

CodePudding user response：

Please make the button type="button". it seems working.

<button id="btng" type="button" onclick="my()">Suggest me another</button>

CodePudding user response：

As @AndrewLi already answered, there is possibilty that generated random number is the same as the last one.

Instead of generating new number multiple times (with while loop).

You can filter out currently displayed image from array.

function my(){
    var bt=document.getElementById("btng");
    bt.textContent = "Suggest me another";
    var img = document.getElementById("img");
    var my = new Array("/im/R.jpg","/im/S.jpg","/im/E.jpg").filter((src) => src !== new URL(img.src).pathname);

    var randomNum = Math.floor(Math.random() * my.length);
    var backgroundImage = my[randomNum];
    document.getElementById("img").src = my[randomNum];
    document.getElementById("pic").style.backgroundImage=`url(${backgroundImage})`;
}

But to make it work, you need provide absolute path to your images in array.

So /im/R.jpg instead of im/R.jpg

CodePudding user response：

Here is the probably most efficient way to do this without filtering the values causing a loop or re-generating values in case we get a duplicate using some index manipulation.

Explanation

For the first image we can select any image within the set of valid indices for the array i.e. 0 to array.length - 1 which we can do using Math.floor(Math.random() * array.length). Then we store the selected index in a variable currentImageIdx.

For any following image we generate a step value which determines the amount of steps we want to move on from the current index to the next value. We need to choose this in the range of 1 to array.length - 1 which we can do by using Math.floor(1 Math.random() * (arrayLength - 1)) as this will make sure we will never move on so far that we are back at the same index as we currently are. We then just need to add that step value to the current index and take the remainder using modulo to make sure we're not getting out of bounds.

Examples for moving on to next index

For the following examples an array with 3 values is assumed:

We're currently at index 0 so we should either move on 1 or 2 steps. If we were to move on 3 steps we would be at 0 again because (start index step) % array.length would translate to (0 3) % 3 = 0 and we'd be at the start index again.
If we're at index 1 we could again move on 1 or 2 steps but not 3 because (start index step) % array.length would translate to (1 3) % 3 = 1 and we'd be at the start index again.
The same applies for index 2

This will work for an array of any size. Except that for the case of just having one element in the array that one element will of course be selected every time.

let currentImageIdx = undefined;

function my() {
  var bt = document.getElementById("btng");
  bt.textContent = "Suggest me another";

  var my = new Array("https://dummyimage.com/20x20/000/fff", "https://dummyimage.com/20x20/00FF00/000", "https://dummyimage.com/20x20/FF0000/000");
  let backgroundImageIdx;
  // it is imporant to check for undefined here as currentImageIdx != 0 would also trigger this case if it just was if(!currentImageIdx) and may produce the same picture twice 
  if (currentImageIdx !== undefined) backgroundImageIdx = getNextImage(currentImageIdx, my.length)
  else backgroundImageIdx = Math.floor(Math.random() * my.length);
  document.getElementById("img").src = my[backgroundImageIdx];
  currentImageIdx = backgroundImageIdx;
  console.log(`selected image ${currentImageIdx}`);
  document.getElementById(
    "pic"
  ).style.backgroundImage = `url(${my[backgroundImageIdx]})`;
}

function getNextImage(currentIndex, arrayLength) {
  // generate random step value between 1 and array.length - 1
  // with three elements: either 1 and 2
  var step = Math.floor(1   Math.random() * (arrayLength - 1));
  // add the step value to current index and make sure we're not out of bounds
  console.log(`(${currentIndex}   ${step}) % ${arrayLength} = ${(currentIndex   step) % arrayLength}`)
  return (currentIndex   step) % arrayLength;
}

<div id="al">

  <div id="pic">
    <img src="" id="img" style="visibility: hidden;">
  </div>
  <button id="btng" onclick="my()">Suggest me manga</button>
</div>