error at end of loop on slide show

Question

I have the following JS

window.onload = function() {
  var indexAtiva = 0;
  const lis = document.getElementsByClassName('slider').item(0).getElementsByTagName('li');

  function slider() {

    for (i = 0; i < lis.length; i++) {

      if (i != indexAtiva) {
        lis[i].classList.remove('ativa');
      } else {
        lis[i].className = 'ativa'
      }
    }

    if ((indexAtiva + 1) == lis.length) {
      indexAtiva = 0;
    } else {
      indexAtiva++;
    }

    setTimeout(slider, 3000);

  }

  slider();
  
  var slider = document.getElementsByClassName('slider')[0]
  var nav = slider.getElementsByTagName('nav')[0]
  var anterior = nav.getElementsByClassName('anterior')[0]
  var proximo = nav.getElementsByClassName('proximo')[0]
  
  anterior.onclick = function(){
	  prev = indexAtiva - 1	   
	  prev = prev.length ? prev : lis[ lis.length - 1 ];  
	  mostraBloco(prev);
  }
  
  proximo.onclick = function(){
	  next = indexAtiva + 1	   
      next = next.length ? next : lis[0];    
	  mostraBloco(next);	  
  }
// Função para exibir as imagens 
  function mostraBloco(next) {
	  ativa = document.getElementsByClassName('ativa')
	  ativa[0].classList.remove('ativa')  
	  next.className = 'ativa'
  }

}

* {
	margin:0;
	padding:0;
	border:none;
	outline:0;
}

body {
	width:100vw;
}


ul {
	list-style: none;
}
  .fade {
	  -webkit-animation-name: fade;
	  -webkit-animation-duration: 1.5s;
	  animation-name: fade;
	  animation-duration: 1.5s;
  }
  @-webkit-keyframes fade {
	 from {
	opacity: .4
	}
	 to {
	opacity: 1
	}
  }
  @keyframes fade {
	 from {
	opacity: .4
	}
	 to {
	opacity: 1
	}
  }  
@keyframes slider {
    0% {
        transform: scale(1);        
    } 
    100% {
        transform: scale(1.1);     
    }
}  

div.slider {
	position:relative;
	width:100%;
	overflow:hidden;	
}
div.slider ul.slide {
}
div.slider ul.slide li{
	display:none;
}

.ativa{
	display:block !important;	
}

div.slider ul.slide li img {
    position:relative;
	width: 100%;
	animation: slider 1s linear;
	animation-fill-mode: forwards;
}
div.slider ul.slide li span {
    position:absolute;
	width: 100px;
	left: calc(50% - 50px);
	line-height:40px;
    bottom:0;
	text-align:center;
	color:rgb(255,255,255);
	z-index:2;	
}

  div.slider nav {
      position:absolute;
      width:100%;
      height:40px;
      bottom:0;
      background-color:rgba(0,0,0,.5);
      z-index:1;
  }
  div.slider nav button {
      position:absolute;
      width:150px;
      height:100%;
	  cursor:pointer;
  }
  div.slider nav  button.anterior {
      left: 10%;
  }
  div.slider nav  button.proximo {
      right: 10%;
  }

<div class="slider">
  <ul class="slide">
     <li class="ativa">
        <img class="fade" src="_img/_banner/_site/bg_1.jpg" />
        <span>Este é 1</span>
     </li>
     <li>
        <img class="fade" src="_img/_banner/_site/bg_2.jpg" />
        <span>Este é 2</span>
     </li>
  </ul>
  <nav>
    <button class="anterior">Anterior</button>
    <button class="proximo">Próximo</button>
  </nav>
</div>

My goal is to catch a UL with their Lis and show one to one. But not all at once.

The error is as follows: the slide runs normally. But when it arrives at the end, it hangs and the console shows no error.

Where is the error?

Note: if I withdraw the part that is below

slider();

all, then the slider runs normal. That is, the problem should be on the navigation buttons

Follow the online link

http://funerariasaopedro.net.br/novo/index5.php

Answer 1

One of the problems is that you created an object with the same function name:

function slider() { and var slider = document.getElementsByClassName('slider')[0]

This will generate conflict because the object slider is no longer a function at this point:

var slider = document.getElementsByClassName('slider')

The solution is either to rename the function or variable, not the two with the same name. For example:

var slider1 = document.getElementsByClassName('slider')[0]
var nav = slider1.getElementsByTagName('nav')[0]

Another redundant thing is the for, that does not require that if within it.

Regarding the buttons, you can take advantage of the function slider() dispensing extra functions, as it did. Just you differentiate the click on the button Previous, placing a parameter true, so that the function knows that it was clicked. In the case of the button Next no need, because the default function already passes the slides forward.

See working:

window.onload = function() {
   var indexAtiva = 0;
   const lis = document.getElementsByClassName('slider').item(0).getElementsByTagName('li');

   function slider(s) {

      for (i = 0; i < lis.length; i++) {
         lis[i].classList.remove('ativa');
      }

      // aqui irá tratar o clique no "Anterior".
      // É preciso tratar de 3 formas diferentes
      // dependendo do valor de indexAtiva.
      // Veja que o valor de indexAtiva, em cada caso,
      // irá receber o valor do índice ativado
      if(s){
         // significa que o próximo a mostrar será o
         // slide de índice 1, então está sendo exibido o 0.
         // neste caso eu ativo o último da array
         if(indexAtiva == 1){
            lis[lis.length-1].className = 'ativa';
            indexAtiva = lis.length-1;
            
         // aqui diz que está exibindo o último,
         // logo próximo será o primeiro (índice 0).
         // Então eu ativo o antepenúltimo da array
         }else if(indexAtiva == 0){
            lis[lis.length-2].className = 'ativa';
            indexAtiva = lis.length-2;
            
          // aqui é quando não for nenhum dos casos anteriores.
          // Eu diminuo 2 índices porque mais a frente será
          // incrementado com +1
         }else{
            lis[indexAtiva-2].className = 'ativa';
            indexAtiva -= 2;
         }
      }else{
         // aqui é quando for clicado o botão "próximo"
         // ou quando nenhum botão for clicado
         lis[indexAtiva].className = 'ativa';
      }

      indexAtiva = indexAtiva + 1 == lis.length ? 0 : indexAtiva+=1;

      tempo = setTimeout(slider, 3000);
   }

   slider();
   
   var slider1 = document.getElementsByClassName('slider')[0]
   var nav = slider1.getElementsByTagName('nav')[0]
   var anterior = nav.getElementsByClassName('anterior')[0]
   var proximo = nav.getElementsByClassName('proximo')[0]
   
   anterior.onclick = function(){
      clearTimeout(tempo);
      slider(true);
   }
   
   proximo.onclick = function(){
      clearTimeout(tempo);
      slider();
   }

}

* {
	margin:0;
	padding:0;
	border:none;
	outline:0;
}

body {
	width:100vw;
}


ul {
	list-style: none;
}
  .fade {
	  -webkit-animation-name: fade;
	  -webkit-animation-duration: 1.5s;
	  animation-name: fade;
	  animation-duration: 1.5s;
  }
  @-webkit-keyframes fade {
	 from {
	opacity: .4
	}
	 to {
	opacity: 1
	}
  }
  @keyframes fade {
	 from {
	opacity: .4
	}
	 to {
	opacity: 1
	}
  }  
@keyframes slider {
    0% {
        transform: scale(1);        
    } 
    100% {
        transform: scale(1.1);     
    }
}  

div.slider {
	position:relative;
	width:100%;
	overflow:hidden;	
}
div.slider ul.slide {
}
div.slider ul.slide li{
	display:none;
}

.ativa{
	display:block !important;	
}

div.slider ul.slide li img {
    position:relative;
	width: 100%;
	animation: slider 1s linear;
	animation-fill-mode: forwards;
  height: 180px;
}
div.slider ul.slide li span {
    position:absolute;
	width: 100px;
	left: calc(50% - 50px);
	line-height:40px;
    bottom:0;
	text-align:center;
	color:rgb(255,255,255);
	z-index:2;	
}

  div.slider nav {
      position:absolute;
      width:100%;
      height:40px;
      bottom:0;
      background-color:rgba(0,0,0,.5);
      z-index:1;
  }
  div.slider nav button {
      position:absolute;
      width:150px;
      height:100%;
	  cursor:pointer;
  }
  div.slider nav  button.anterior {
      left: 10%;
  }
  div.slider nav  button.proximo {
      right: 10%;
  }

<div class="slider">
  <ul class="slide">
     <li class="ativa">
        <img class="fade" src="http://funerariasaopedro.net.br/novo/_img/_banner/_site/bg_1.jpg" />
        <span>Este é 1</span>
     </li>
     <li>
        <img class="fade" src="http://funerariasaopedro.net.br/novo/_img/_banner/_site/bg_2.jpg" />
        <span>Este é 2</span>
     </li>
     <li>
        <img class="fade" src="https://www.cleverfiles.com/howto/wp-content/uploads/2016/08/mini.jpg" />
        <span>Este é 3</span>
     </li>
     <li>
        <img class="fade" src="https://www.salford.ac.uk/__data/assets/image/0008/890072/varieties/lightbox.jpg" />
        <span>Este é 4</span>
     </li>
  </ul>
  <nav>
    <button class="anterior">Anterior</button>
    <button class="proximo">Próximo</button>
  </nav>
</div>

Answer 2

I think I found the error in the buttons:

in the block

prev = indexAtiva - 1      
prev = prev.length ? prev : lis[ lis.length - 1 ];  
mostraBloco(prev);

the logic is slightly wrong. That’s why, say: the maximum index is 4. It’s currently on the fifth slide (index 4).

prev = 4 - 1 // =3
prev = prev.length ? prev : lis[ lis.length - 1 ];// false, o valor de prev é 3 aqui, nunca vai executar.

the same error occurs on the other button. The solution is simple: make the condition without increasing/decreasing. If true, go back to zero/forward to 4. If false, add/remove 1. This way:

prev = indexAtiva; 
prev == 0 ? lis[indexAtiva] = lis[lis.length] : lis[ lis[indexAtiva] - 1 ];
mostraBloco(prev);

next = indexAtiva
next == lis.length ? lis[indexAtiva] = lis[0] : lis[ lis[indexAtiva] + 1 ];
mostraBloco(next);

(I made new response because I could not comment on the response of the dvd)