Div doesn't react to display=none

Question

My goal was to create function which will exchange 4 divs in the same place after delay which means...

div "commission1" is visible so after 1 second it dissapears and div "commission2" takes its place> dissapears after 1 second> comission3>dissapears>comission4 and it should be looped, but before i managed to do that it is already not working.

i have unexpected end of input error I didn't find any lack of sign or letter so i don't know what is going on with this error

Keep in mind that it's first time after 4 years of break that im using java and back then i was already beginner :D.

Thank you in advance :)

function visibility() 
{
 if (document.getElementById("commission1") != null) {
 document.getElementById('commission1').style.display = 'display';
  setTimeout("document.getElementById('commission1').style.display = 'none'", 1000);
  setTimeout("document.getElementById('commission2').style.display = 'display'", 1000);
  setTimeout("document.getElementById('commission2').style.display = 'none'", 2000);
  setTimeout("document.getElementById('commission3').style.display = 'display'", 2000);
  setTimeout("document.getElementById('commission3').style.display = 'none'", 3000);
  setTimeout("document.getElementById('commission4').style.display = 'display'", 3000);
  setTimeout("document.getElementById('commission4').style.display = 'none'", 4000);
  setTimeout("document.getElementById('commission1').style.display = 'display'", 4000);
  }

Not really the prettiest code I have ever seen, but to help you track down your error, you could count opening and closing {, } and see that the numbers don't match. — Matus Dubrava
– Matus Dubrava, Commented Apr 10, 2018 at 22:32

Sir Rubberduck · Accepted Answer · 2018-04-10 22:37:53Z

1

The problem is because you are providing a string to the setTimeout function as a parameter, instead of another function to be executed. It should look like this.

setTimeout(function() {document.getElementById('foo').style.display = 'none'}, 1000);

Also, DRY (Don't Repeat Yourself) i.e. use functions instead of copying the exact same code over and over again. I provided my take on the code.

DEMO: https://jsfiddle.net/9txkdtj3/19/

HTML

<div id="commission1" class="invisible">1</div>
<div id="commission2" class="invisible">2</div>
<div id="commission3" class="invisible">3</div>
<div id="commission4" class="invisible">4</div>

CSS

.visible {display: block;}
.invisible {display: none;}

Javascript

function hide(id){
    document.getElementById(id).className = "invisible";
}

function show(id){
    document.getElementById(id).className = "visible";
}

function visibility(){
    hide("commission4"); show("commission1");
    setTimeout(() => { hide("commission1"); show("commission2") }, 1000);
    setTimeout(() => { hide("commission2"); show("commission3") }, 2000);
    setTimeout(() => { hide("commission3"); show("commission4") }, 3000);
    setTimeout(() => { visibility() }, 4000);
}

visibility();

edited Apr 10, 2018 at 22:37

answered Apr 10, 2018 at 22:32

Sir Rubberduck

2,3225 gold badges41 silver badges77 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Krzysztof Orczyk Over a year ago

Thank you! You helped a lot and reminded me some things. It will be my base to learn more.

Sir Rubberduck Over a year ago

@KrzysztofOrczyk Have a look at my programming notes (github.com/8483/notes). I'm sure you'll find something. :)

Michio · Accepted Answer · 2018-04-11 01:10:08Z

Firstly, to show an element using style.display, you should do element.style.display = 'block' (or other valid display value such as inline-block). I don't think display is a valid css display value.

As for the overall goal you are trying to achieve, this is how I would go about it:

const MAX = 4;
const TIMEOUT = 1000;

function fn(n) {
	if (n > MAX || n < 0) {
		n = 1;
	}

	for (let i = 1; i <= MAX; i++) {
		let id = "commission-" + i;
		let div = document.getElementById(id);
		if (n === i) {
			div.style.display = 'block';
		} else {
			div.style.display = 'none';
		}
	}

	setTimeout(function () {
		fn(++n);
	}, TIMEOUT);
}

fn(1);

<!DOCTYPE html>
<html>

<head>
	<meta charset="utf-8" />
	<meta http-equiv="X-UA-Compatible" content="IE=edge">
	<title>Page Title</title>
	<meta name="viewport" content="width=device-width, initial-scale=1">
</head>

<body style="padding:24px">
	<div id="commission-1">Commission 1</div>
	<div id="commission-2">Commission 2</div>
	<div id="commission-3">Commission 3</div>
	<div id="commission-4">Commission 4</div>
	
	<script src="main.js"></script>
</body>

</html>

Collectives™ on Stack Overflow

Div doesn't react to display=none

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related