1
\$\begingroup\$

I am writing code for a very basic jQuery slider with the following features:

  1. Slide the content left or right on click of next/previous links till there is no more content on the clicked side.
  2. 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.

jsFiddle

 $('#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
\$\endgroup\$
3
  • 1
    \$\begingroup\$ On code review one is supposed to include at least part of one's code in the question. \$\endgroup\$ Commented 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\$ Commented Jul 16, 2012 at 10:05
  • \$\begingroup\$ @Inkbug: Included the code. Thanks for improvement suggestions :) \$\endgroup\$ Commented Jul 16, 2012 at 10:10

1 Answer 1

2
\$\begingroup\$

JsFiddle

Not writing this as a plugin here are some suggestions:

  1. instead of removeClass('a').addClass('b') use toggleClass('a b')
  2. instead of searching the entire DOM for .current you should constrain it to the children of #container
  3. instead of css('height') use height()
  4. 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
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.