\$\begingroup\$
\$\endgroup\$
3
I am writing code for a very basic jQuery slider with the following features:
- Slide the content left or right on click of next/previous links till there is no more content on the clicked side.
- If any content is added dynamically, the entire content should slide.
While there are still many additions and modifications necessary, I would like to know if my approach is correct and the code that I am writing is going the correct way.
I am planning to write a plugin for this later.
$('#next').click(function(){
//slide a div only if there is a next element
if($('.currentRight').length >0)
{
$('.current').animate({left:'-100px'}).removeClass('current').addClass('currentLeft').next().animate({left:'0px'}).removeClass('currentRight').addClass('current');
}
});
$('#previous').click(function(){
//slide a div only if there is a previous element
if($('.currentLeft').length >0)
{
$('.current').animate({left:'100px'}).removeClass('current').addClass('currentRight').prev().animate({left:'0px'}).removeClass('currentLeft').addClass('current');
}
});
$('#add').click(function(){
//it is used for simplicity's sake. I intend to use append()/prepend() to add more content
var cont = $('.current').html()+'More<br/>';
$('.current').html(cont);
var ht = $('.current').css('height');
$('#container').css('height',ht);
});
/*the main div that contains all the sliding div*/
#container{
width:100px;
border:1px solid #000;
height:20px;
position: relative;
overflow:hidden;
}
/*there are 3 different classes
* current - div that is presently visible
* currentLeft - div that should slide from left on pressing previous
* currentRight - div that should slide from right on pressing next
*/
.current{
width:99px;
height:auto;
position: absolute;
float: left;
left:0px;
}
.currentLeft{
width:99px;
height:auto;
position: absolute;
left:-100px;
float:left;
}
.currentRight{
width:99px;
height:auto;
position: absolute;
left:100px;
float:left;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div id="container">
<div class="currentLeft">
First Div
</div>
<div class="current">
Second Div
</div>
<div class="currentRight">
Third Div
</div>
</div>
<a href="#" id="previous">Previous</a>
<a href="#" id="next">Next</a>
<a href="#" id="add">Add content</a>
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 16, 2012 at 8:46
-
1\$\begingroup\$ On code review one is supposed to include at least part of one's code in the question. \$\endgroup\$Inkbug– Inkbug2012年07月16日 09:50:26 +00:00Commented Jul 16, 2012 at 9:50
-
\$\begingroup\$ The link for the fiddle has the entire code. I thought it would be easier to review the code at the fiddle since the jQuery, CSS and HTML are separated and also are working. \$\endgroup\$gentrobot– gentrobot2012年07月16日 10:05:07 +00:00Commented Jul 16, 2012 at 10:05
-
\$\begingroup\$ @Inkbug: Included the code. Thanks for improvement suggestions :) \$\endgroup\$gentrobot– gentrobot2012年07月16日 10:10:09 +00:00Commented Jul 16, 2012 at 10:10
1 Answer 1
\$\begingroup\$
\$\endgroup\$
0
Not writing this as a plugin here are some suggestions:
- instead of
removeClass('a').addClass('b')
usetoggleClass('a b')
- instead of searching the entire DOM for
.current
you should constrain it to the children of#container
- instead of
css('height')
useheight()
- you should set the container height to the maximum height of all child elements, not just the height of the current visible one
answered Jul 16, 2012 at 14:17
default