Opening and closing pictures with single buttons (JavaScript)

Question

I am new to JavaScript. I created this code in order to try and make buttons that will hide and show certain pictures on the page. I have 3 buttons, the first of which is supposed to run my JavaScript code in <script></script> tags, the other two just have Javascript code inside them and they work fine. But they don't hide the picture once they are clicked a second time, which is why I am trying to do that for the first one if possible.

For some reason, I cannot get the first button with "open()" to work the way I want with my Javascript code. Can anyone with more experience please explain to me what I am doing wrong? Thank you in advance...

var btn1 = document.getElementById('1');
var btn2 = document.getElementById('2');
var btn3 = document.getElementById('3');
var display1 = btn1.getAttribute('display')
var display2 = btn2.getAttribute('display')
var display3 = btn3.getAttribute('display')

function open() {
  if (display1 === ('none')) {
      btn1.setAttribute('display', 'block');
  }  else { 
    btn1.setAttribute('display', 'none');
  }
}

<img id="1" src="forge.PNG" style="height:320px; display:none; padding:10px">
<img id="2" src="lizard.jpg" style="height:320px; display:none; padding:10px">
<img id="3" src="walkway.jpg" style="height:320px; display:none; padding:10px">

<button onclick="open()">1</button>
<button onclick="document.getElementById('2').style.display='block'">2</button>
<button onclick="document.getElementById('3').style.display='block'">3</button>

score 1 · Accepted Answer · answered Jan 25 '20 at 21:47

I'd use event delegation to watch for clicks on the container. When the nth button is clicked, select the nth image, and toggle a class that hides/shows the image:

const images = document.querySelectorAll('img');
const buttons = [...document.querySelectorAll('button')];

document.addEventListener('click', (e) => {
  if (e.target.matches('button')) {
    const i = buttons.indexOf(e.target);
    images[i].classList.toggle('hidden');
  }
});

.hidden {
  display: none;
}

<img id="1" src="https://www.gravatar.com/avatar/34932d3e923ffad9a4a1423e30b1d9fc?s=48&d=identicon&r=PG&f=1" style="height:320px; padding:10px" class="hidden">
<img id="2" src="https://www.gravatar.com/avatar/978ec0c47934c4b04401a8f4b4fec8bd?s=32&d=identicon&r=PG&f=1" style="height:320px; padding:10px" class="hidden">
<img id="3" src="https://lh3.googleusercontent.com/-uIr21N5ccCk/AAAAAAAAAAI/AAAAAAAAHeg/ohNEkpJKXQA/photo.jpg?sz=32" style="height:320px; padding:10px" class="hidden">

<button>1</button>
<button>2</button>
<button>3</button>

Problems with your original code include:

You're trying to select the elements before they exist in the DOM
Elements do not have a display property - in order to check the style of an element, you have to access its .style property first (eg, someImage.style.display)
Similarly, to set the style of an element, you have to set a property of its style property (eg someImage.style.display = <newDisplay>). Setting the display attribute of the element won't do anything.

Try to avoid inline handlers if at all possible - they have many problems and are pretty much universally considered to be quite poor practice. Always attach listeners properly using Javascript instead, whenever that's an option.

Dude, you're a lifesaver. Thank you! – SenorCardgage Jan 25 '20 at 22:00 — SenorCardgage, Jan 25 '20 at 22:00

score 0 · Answer 2 · answered Jan 25 '20 at 22:23

The event listener is the better solution, but if you want to see a working code in your way:

<!DOCTYPE html>
<html>
<head>
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>switchpics</title>
</head>
<script type="text/javascript">

var open = function(param) {
  img = document.getElementById(param.innerHTML);

  if (img.style.display == 'none'){
    img.style.display = "block";
  } else {
    img.style.display = "none";
  };
};
</script>
<body>
  <img id="1" src="1.jpg" style="height:20px; display:block; padding:10px">
  <img id="2" src="1.jpg" style="height:20px; display:none; padding:10px">
  <img id="3" src="1.jpg" style="height:20px; display:none; padding:10px">

  <button onclick="open(this)">1</button>
  <button onclick="open(this)">2</button>
  <button onclick="open(this)">3</button>
</body>
</html>

Opening and closing pictures with single buttons (JavaScript)

2 Answers2