I have a slider with 3 images and left/right arrows. If you click on them, the image changes. You also have 3 bullets below the images and the image changes depending on what bullet you click.
I don't want to use buttons "arrows"; I want to use the tag "a." Any ideas on making it better? You'll need to add 3 images "because sliders use 3 images" and the other 2 images are to use left/right arrows (to change the images). That is what I don't want to use, as I said.
HTML:
<!DOCTYPE html>
<html>
<head>
<title>Slider</title>
<meta charset="utf-8" />
<link rel="stylesheet" href="style.css">
<script src="js/jquery-2.1.4.min.js"></script>
<script src="js/slider.js"></script>
</head>
<body>
<div class="sliders">
<ul >
<li class="activa"><img src="fotos-de-Hamburguesa-americana.jpg"></li>
<li><img src="images (3).jpg"></li>
<li><img src="images21312.jpg"></li>
</ul>
<!-- <ul class="controles">
<li class="activa"> </li>
<li> </li>
<li> </li>
</ul> -->
</div>
<div class="sliders">
<ul>
<li class="activa"><img src="fotos-de-Hamburguesa-americana.jpg"></li>
<li><img src="images (3).jpg"></li>
<!-- <li><img src="images21312.jpg"></li> -->
</ul>
</div>
</body>
</html>
JS:
$.fn.slider = function(config){
var nodos = this;
var delay = (typeof config.delay == "number")?parseInt(config.delay):4000;
for (var i = 0; i < nodos.length; i++) {
Slider(nodos[i]);
}
function Slider(nodo){
var galeria = $(nodo).find('ul');
var btn1 = "<button class='before'></button>";
if(!$(nodo).hasClass('slider'))
$(nodo).addClass('slider');
if(!galeria.hasClass('galeria'))
galeria.addClass("galeria");
//Encontrar cuantas imagenes hay en la galeria
var imagenes = $(galeria).find('li');
//Controles
var html_bullets="<ul class='controles'>";
for (var it = 0; it < imagenes.length; it++) {
if(it==0)
html_bullets+="<li data-index='"+it+"' class='activa'> </li>";
else
html_bullets+="<li data-index='"+it+"'> </li>";
}
html_bullets+="</ul><button class='next'></button>";
$(nodo).append(html_bullets);
$(nodo).prepend(btn1);
var bullets = $(nodo).find("ul.controles li");
bullets.click(function(){
var index= $(this).data("index");
bullets.removeClass('activa');
imagenes.removeClass('activa');
$(imagenes[index]).addClass("activa");
$(bullets[index]).addClass("activa");
});
}
$(".slider").on("click","button.before",function(){
var div = this;
div = $(div).parent();
console.log(div);
flechas({div:div});
});
$(".slider").on("click","button.next",function(){
var div = this;
div = $(div).parent();
flechas({div:div,direccion:1});
});
function flechas(tipo){
var div = tipo.div;
var imagen = $(div).find("ul.galeria li.activa");
var imagenes = $(div).find("ul.galeria li");
var bullet = $(div).find("ul.controles li.activa");
var bullets = $(div).find("ul.controles li");
var index = bullet.data("index");
var max = bullets.length-1;
bullets.removeClass('activa');
imagenes.removeClass('activa');
if(tipo.direccion){
if(index == max){
$(imagenes[0]).addClass("activa");
$(bullets[0]).addClass("activa");
}else {
$(imagenes[index+1]).addClass("activa");
$(bullets[index+1]).addClass("activa");
}
}
else{
if(index == 0){
$(imagenes[max]).addClass("activa");
$(bullets[max]).addClass("activa");
}else {
$(imagenes[index-1]).addClass("activa");
$(bullets[index-1]).addClass("activa");
}
}
}
};
$(document).ready(function() {
$(".sliders").slider({delay:5000});
});
CSS:
.slider{
width: 420px;
overflow: hidden;
}
.slider ul{
list-style: none;
padding: 0;
}
.slider ul.galeria{
height: 200px;
position: relative;
margin-left: 30px;
}
.slider ul.galeria li{
position: absolute;
top: 0;
left: 0;
opacity: 0;
transition: all 2s;
}
.slider ul.galeria li.activa{
opacity: 1;
}
.slider ul.galeria img{
max-height: 200px;
margin-left: 5px;
}
/*Controles*/
.slider ul.controles {
text-align: center;
}
.slider ul.controles li{
background-color: black ;
display: inline-block;
width: 20px;
height: 20px;
border-radius: 10px;
}
.slider ul.controles li.activa{
background-color: blue ;
}
/*Botones-flechas*/
.slider button.before{
background-image: url(Flecha_002.png);
position: relative;
left: 0;
top: 128px;
background-repeat: no-repeat;
background-size: 28px;
height: 48px;
width: 33px;
background-color: white;
border: none;
}
.slider button.next{
background-image: url(Flecha_001.png);
position: relative;
left: 86%;
bottom: 190px;
background-repeat: no-repeat;
background-size: 28px;
height: 48px;
width: 33px;
background-color: white;
border: none;
}
.slider button.before:focus{
outline: none;
}
.slider button.next:focus{
outline: none;
}
.slider button.before:hover,.slider button.next:hover,.slider ul.controles li:hover{
cursor: pointer;
}
1 Answer 1
Your code is good, but there's some points you can improve upon:
Assumptions:
$.fn.slider = function(config){ var nodos = this; var delay = (typeof config.delay == "number")?parseInt(config.delay):4000;
You're assuming config
won't be empty or not added. You should have a default config, and simply extend the parameter config to include the items the parameter config left off.
abusing jQuery:
You may not need to use jQuery for everything: See youmightnotneedjquery.com for a full list.
For example:
$(nodo).find('ul')
can be written asnodo.getElementsByTagName('ul')
$(this).data("index")
can be written asthis.getAttribute('index')
$(div).parent()
can be written asdiv.parentElement
.addClass
can be written as.classList.add
Better call the redundancy department department:
Your code has quite a bit of redundancy, for example:
var div = this; div = $(div).parent(); console.log(div); flechas({div:div});
Could be expressed as:
flechas({div: this.parentElement});
And the following is also redundant:
var btn1 = "<button class='before'></button>";
var html_bullets="<ul class='controles'>";
html_bullets+="...";
$(nodo).append(html_bullets);
$(nodo).prepend(btn1);
Simply remove btn1
and attach its contents to html_bullets
.
Indentation and spacing:
Your indentation and spacing is wrong:
- You should have spaces around your operators and after commas.
- You should have each layer of indentation indented by either two, four or eight spaces. Additionally, keep consistent.
Miscellanous
typeof
vs.instanceof
: While both are similar, I would prefer to useinstanceof
as it lets you declare the type as the type, meaning the compiler will pick up if you mess up the format on the name:
if (config.delay instanceof Number)
parseInt(config.delay)
: there's many ways to do this as described by this example, however, I would preferNumber(config.delay)
as it looks more clear as to what are trying to create.- In addition to this above point, leaving off the second parameter in
parseInt
can cause strange issues. Unless needed, you can use10
as the second parameter to parse to decimal.
- In addition to this above point, leaving off the second parameter in
-
\$\begingroup\$ That's really nice information, thanks for your time and help I'll have on mind that tips \$\endgroup\$JorgeAlberto– JorgeAlberto2016年01月18日 01:42:39 +00:00Commented Jan 18, 2016 at 1:42