How to make JS to show different image-CodePudding

I used that script to show a modal with images, but I want to do that the modal that will pop-up will show the specific image that pressed.

The problem is that the JS is only respond the the first img, but when I add the second img to the script its just wont working... There is any easy way to make the JS work for each img or I need to write and whole script for each img?

var modal = document.getElementById("myModal");

var img = document.getElementById("myImg");
var modalImg = document.getElementById("img1"); // second img (img1)
var modalImg = document.getElementById("img2"); // second img (img2)

img.onclick = function() {
  modal.style.display = "block";
  modalImg.src = this.src;
}

var span = document.getElementsByClassName("close")[0];

span.onclick = function() {
  modal.style.display = "none";
}

#myImg {
  border-radius: 5px;
  cursor: pointer;
  transition: 0.3s;
}

#myImg:hover {
  opacity: 0.7;
}

.modal {
  display: none;
  position: fixed;
  z-index: 1;
  padding-top: 100px;
  left: 0;
  top: 0;
  width: 100%;
  height: 100%;
  overflow: auto;
  background-color: rgb(0, 0, 0);
  background-color: rgba(0, 0, 0, 0.9);
}

.modal-content {
  margin: auto;
  display: block;
  width: 80%;
  max-width: 700px;
}

.modal-content,
{
  animation-name: zoom;
  animation-duration: 0.6s;
}

@keyframes zoom {
  from {
    transform: scale(0)
  }
  to {
    transform: scale(1)
  }
}

.close {
  position: absolute;
  top: 15px;
  right: 35px;
  color: #f1f1f1;
  font-size: 40px;
  font-weight: bold;
  transition: 0.3s;
}

.close:hover,
.close:focus {
  color: #bbb;
  text-decoration: none;
  cursor: pointer;
}

@media only screen and (max-width: 700px) {
  .modal-content {
    width: 100%;
  }
}

<div>
  <img src="https://picsum.photos/id/237/150/150" id="myImg">
  <div id="myModal" >
    <span >
         &times;
      </span>
    <img  id="img1">
  </div>
</div>
<div>
  <img src="https://picsum.photos/id/137/150/150" id="myImg">
  <div id="myModal" >
    <span >
         &times;
      </span>
    <img  id="img2">
  </div>

Thanks a lot!

CodePudding user response：

first, you cannot give the same id to multiple elements in your document. second, when you redeclare the variable named modalImg the second time, you are overwriting the value that you put in the variable the first time. third, getElementById() will only get reference to one element, but even if it were a function to get multiple elements, you would have to iterate through those elements to set an event listener on each.

your code and your question are at a level where I think you may want to consider taking a javascript class, to learn how variables, listeners, loops etc work, before jumping straight in on modifying tutorials or whatever this code is originally from.

CodePudding user response：

Cleaning up the duplicate id's, moving the "close" behavior inline, and (for brevity's sake) taking advantage of auto-wired-up DOM variables (any element with a unique id is available as a variable immediately) I got this:

myImg.onclick = function() {
  myModal.style.display = "block";
  modalImg1.src = this.src;
}
myImg2.onclick = function() {
  myModal2.style.display = "block";
  modalImg2.src = this.src;
}

#myImg {
  border-radius: 5px;
  cursor: pointer;
  transition: 0.3s;
}

#myImg:hover {
  opacity: 0.7;
}

.modal {
  display: none;
  position: fixed;
  z-index: 1;
  padding-top: 100px;
  left: 0;
  top: 0;
  width: 100%;
  height: 100%;
  overflow: auto;
  background-color: rgb(0, 0, 0);
  background-color: rgba(0, 0, 0, 0.9);
}

.modal-content {
  margin: auto;
  display: block;
  width: 80%;
  max-width: 700px;
}

.modal-content,
{
  animation-name: zoom;
  animation-duration: 0.6s;
}

@keyframes zoom {
  from {
    transform: scale(0)
  }
  to {
    transform: scale(1)
  }
}

.close {
  position: absolute;
  top: 15px;
  right: 35px;
  color: #f1f1f1;
  font-size: 40px;
  font-weight: bold;
  transition: 0.3s;
}

.close:hover,
.close:focus {
  color: #bbb;
  text-decoration: none;
  cursor: pointer;
}

@media only screen and (max-width: 700px) {
  .modal-content {
    width: 100%;
  }
}

<div>
  <img src="https://picsum.photos/id/237/150/150" id="myImg">
  <div id="myModal" >
    <span  onclick="myModal.style.display='none'">
         &times;
      </span>
    <img  id="modalImg1">
  </div>
</div>
<div>
  <img src="https://picsum.photos/id/137/150/150" id="myImg2">
  <div id="myModal2" >
    <span  onclick="myModal2.style.display='none'">
         &times;
      </span>
    <img  id="modalImg2">
  </div>

CodePudding user response：

it better to not have duplicate id, I will modify the block html like below

<div > <!-- add class to the block -->
  <img src="https://picsum.photos/id/237/150/150" >
  <div >
    <span >
         &times;
      </span>
    <img >
  </div>
</div>

so the complete code will be like

document.querySelectorAll('.item').forEach(function(item) {
  var modal = item.querySelector('.modal');
  item.querySelector('.thumb').onclick = function() {
    modal.style.display = "block";
  }
  item.querySelector('.close').onclick = function() {
    modal.style.display = "none";
  }
})

.thumb{border-radius:5px;cursor:pointer;transition:.3s}
.thumb:hover{opacity:.7}
.modal{display:none;position:fixed;z-index:1;padding-top:100px;left:0;top:0;width:100%;height:100%;overflow:auto;background-color:#000;background-color:rgba(0,0,0,0.9)}
.modal-content{margin:auto;display:block;width:80%;max-width:700px}
.modal-content,{animation-name:zoom;animation-duration:.6s}
@keyframes zoom {from{transform:scale(0)}to{transform:scale(1)}}
.close{position:absolute;top:15px;right:35px;color:#f1f1f1;font-size:40px;font-weight:700;transition:.3s}
.close:hover,.close:focus{color:#bbb;text-decoration:none;cursor:pointer}
@media only screen and (max-width: 700px) {.modal-content{width:100%}}

<div >
  <img src="https://picsum.photos/id/237/150/150" >
  <div >
    <span >
         &times;
      </span>
    <img >
  </div>
</div>

<div >
  <img src="https://picsum.photos/id/137/150/150" >
  <div >
    <span >
         &times;
      </span>
    <img >
  </div>
</div>

CodePudding user response：

Remember that an id attribute uniquely identifies HTML elements on the page. Have a look at the WCAG spec for review. Therefore, you should never have two elements with the same unique identifier. Using duplicate id values will cause accessibility issues for assistive technologies like screen readers. Also, you should always provide a src and alt for images, otherwise there isn't any point to add it on the page.

Many HTML issues exist in your code, have a look at the W3 HTML Validator to resolve those. The problem with your modal logic is that it only selects a single element via .getElementById() so you can't really expect the second modal element to have any interactivity. If you want to utilize id's for each .modal element then you should use unique identifiers and select them from the DOM as you need e.g. id="myModalTwo", just don't repeat IDs.

A better idea would be to select all the parent container modal elements from the DOM in one go using something like .querySelectorAll() and then you can attach event listeners and handle the specific modal logic for each modal component on the page.

function setupModals() {
  // select all .modal-component containers
  const modals = document.querySelectorAll(".modal-component");
  // iterate through the .modal-component containers
  modals.forEach((m) => {
   const modal = m.querySelector(".modal");
   const image = m.querySelector(".modal-img");
   const modalImg = m.querySelector(".modal-content");
   const closeBtn = modal.querySelector(".close");

   // create a click event listener for each display image
   image.addEventListener("click", () => {
    modal.style.display = "block";
    // set both the src and alt attributes
    // or create an <img> here programmtically
    modalImg.src = image.src;
    modalImg.alt = image.alt;
   });

   // handle click event for close button
   closeBtn.addEventListener("click", () => {
    modal.style.display = "none";
   });
  });
}

setupModals();

.modal-img {
  border-radius: 5px;
  cursor: pointer;
  transition: 0.3s;
}

.modal-img:hover {
  opacity: 0.7;
}

.modal {
  display: none;
  position: fixed;
  z-index: 1;
  padding-top: 100px;
  left: 0;
  top: 0;
  width: 100%;
  height: 100%;
  overflow: auto;
  background-color: rgb(0, 0, 0);
  background-color: rgba(0, 0, 0, 0.9);
}

.modal-content {
  margin: auto;
  display: block;
  width: 80%;
  max-width: 700px;
}

.modal-content,
{
  animation-name: zoom;
  animation-duration: 0.6s;
}

@keyframes zoom {
  from {
    transform: scale(0)
  }
  to {
    transform: scale(1)
  }
}

.close {
  position: absolute;
  top: 15px;
  right: 35px;
  color: #f1f1f1;
  font-size: 40px;
  font-weight: bold;
  transition: 0.3s;
}

.close:hover,
.close:focus {
  color: #bbb;
  text-decoration: none;
  cursor: pointer;
}

@media only screen and (max-width: 700px) {
  .modal-content {
    width: 100%;
  }
}

<div >
  <img  src="https://picsum.photos/id/237/150/150" alt="A black lab">
  <div id="myModal" >
    <span >
         &times;
      </span>
    <img >
  </div>
</div>
<div >
  <img  src="https://picsum.photos/id/137/150/150" alt="A light shining at the end of a narrow opening">
  <div id="myModalTwo" >
    <span >
         &times;
      </span>
    <img >
  </div>
</div>

CodePudding user response：

Here you go I rewrote everything to look a bit cleaner. Might wanna use class as id's are specific to a element and class you can invoke multiple times.

var img = $(".myImg");

function showModal(src) {
    var modal = $("#myModal");

    var modalImg = $(".modal-content");

    modalImg.attr('src', src);
    modal.show();
}
$("span").on("click", function() {
  $(this).parent().hide();
});   
img.on("click", function(){
  showModal($(this).attr("src"));
});

    .myImg {
      border-radius: 5px;
      cursor: pointer;
      transition: 0.3s;
    }

    .myImg:hover {
      opacity: 0.7;
    }

    .modal {
      display: none;
      position: fixed;
      z-index: 1;
      padding-top: 100px;
      left: 0;
      top: 0;
      width: 100%;
      height: 100%;
      overflow: auto;
      background-color: rgb(0, 0, 0);
      background-color: rgba(0, 0, 0, 0.9);
    }

    .modal-content {
      margin: auto;
      display: block;
      width: 80%;
      max-width: 700px;
    }

    .modal-content,
    {
      animation-name: zoom;
      animation-duration: 0.6s;
    }

    @keyframes zoom {
      from {
        transform: scale(0)
      }
      to {
        transform: scale(1)
      }
    }

    .close {
      position: absolute;
      top: 15px;
      right: 35px;
      color: #f1f1f1;
      font-size: 40px;
      font-weight: bold;
      transition: 0.3s;
    }

    .close:hover,
    .close:focus {
      color: #bbb;
      text-decoration: none;
      cursor: pointer;
    }

    @media only screen and (max-width: 700px) {
      .modal-content {
        width: 100%;
      }
    }

<html>
<head>
    <title>Test</title>
    <script type="text/javascript" src="https://code.jquery.com/jquery-3.6.0.min.js" integrity="sha256-/xUj 3OJU5yExlq6GSYGSHk7tPXikynS7ogEvDej/m4=" crossorigin="anonymous"></script>
</head>
<body>
  <div id="myModal" >
    <span >
         &times;
      </span>
    <img >
  </div>
  <div>
    <img src="https://picsum.photos/id/237/150/150" >
    
  </div>
  <div>
    <img src="https://picsum.photos/id/137/150/150" >
      </body>
  </div>
</html>