Div circular scroll

Question

Have one div:

enter image description here

Here's what scrolling of this div should look like:

enter image description here

The only way I see is to duplicate div to left and right sides of original one, and then scroll the whole bunch. Here is a corner-cutting implementation sketch: Fiddle

$(document).ready(function() {
 $original = $("#original");
 // Duplicate div to the left and right of original
 $original.clone().attr("id", "left").prependTo("#scroll");
 $original.clone().attr("id", "right").appendTo("#scroll");
 // Set according width to parent we are going to scroll
 w = $original.width();
 $scroll = $("#scroll");
 $scroll.width(w * 3);
 // Radio button
 $left_rb = $("#left_rb");
 function roll_it() {
 dest_pos = 0;
 // Check radion button
 if ($left_rb.prop("checked")) dest_pos = -2 * w;
 // Set start position
 $scroll.css('left', -w + 'px');
 // Roll it
 $scroll.animate({
 left: dest_pos + "px"
 }, 1000, "linear", roll_it);
 }
 roll_it();
});

.scrollable {
 width: 150px;
 height: 50px;
 float: left;
}
#container {
 width: 150px;
 height: 50px;
 overflow: hidden;
}
#scroll {
 position: relative;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div id="container">
 <div id="scroll">
 <div class="scrollable" id="original">
 <img src="https://i.sstatic.net/vvcpe.gif" />
 </div>
 </div>
</div>
<p>Direction:</p>
<form>
 <input type="radio" name="direction" value="left" id="left_rb" checked>&lt;&lt;&lt;&lt;&lt;
 <input type="radio" name="direction" value="rigth" id="rigt_rb">&gt;&gt;&gt;&gt;&gt;</form>

Any better way? Existing plugins (it should be scrollable to any choosen position, not just slideshow)?

Answer 1

Be sure to localize all of your variables using var.

JavaScript naming using interCaps is more idiomatic than under_scores.

For a better user experience, use <label> tags on your radio buttons. That makes the ">>>>>" text part of the control, for a larger click target.

If you are willing to change the semantically significant <img> tag into a purely decorative background-image, then the code can be greatly simplified through the use of background-repeat: repeat-x.

$(function() {
 var $scroll = $(".scrollable");
 var w = $scroll.width();
 var pos = 0;
 // Radio button
 var $left_rb = $("#left_rb");
 function rollIt() {
 var movement = $left_rb.prop("checked") ? -w : w;
 pos += movement;
 $scroll.animate({
 backgroundPosition: pos + "px"
 }, 1000, "linear", rollIt);
 }
 rollIt();
});

.scrollable {
 width: 150px;
 height: 50px;
 background-repeat: repeat-x;
}
#scroll {
 background-image: url(//i.sstatic.net/vvcpe.gif);
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="scrollable" id="scroll"></div>
<p>Direction:</p>
<form>
 <label><input type="radio" name="direction" value="left" id="left_rb" checked>&lt;&lt;&lt;&lt;&lt;</label>
 <label><input type="radio" name="direction" value="right">&gt;&gt;&gt;&gt;&gt;</label>
</form>

Answer 2

Your code was overall hard to read. But it is somewhat structured properly.

Let's analyse Javascript!

1. Are you afraid of vars?

   Yeah, you don't use a single var. EVERYTHING is a global variable.

   Why is it bad? You are polluting an already polluted object, you allow outside code to mess with your code and it is slower to access global variables than local variables (it's around 10x slower).

   Please, use vars.

2. Your variable names are not that good.

   Some are short, cryptic, meaningless.

   Names like w and dest_pos don't help much. What's w? Can you figure it just by the name?

3. Misuse of the library.

   Why do you have this:

   $original = $("#original");
   $original.clone().attr("id", "left").prependTo("#scroll");
   $original.clone().attr("id", "right").appendTo("#scroll");
   

   When you can write:

   var $original = $("#original");
   var $scroll = $("#scroll");
   $original.clone().prependTo($scroll);
   $original.clone().appendTo($scroll);
   

   The id isn't being used and you can store #scroll in a variable (as you do a few lines after).

4. More library misuse.

   jQuery has something really great going on for it: chaining methods.

   You did it everywhere, except there:

   $scroll.css('left', -w + 'px');
   // Roll it
   $scroll.animate({
    left: dest_pos + "px"
   }, 1000, "linear", roll_it);
   

   You should write it on this way:

   $scroll
    .css('left', -w + 'px')
    .animate({
    left: dest_pos + "px"
    }, 1000, "linear", roll_it);
   

5. Hidden dangers!

   Lets analyse this line:

   if ($left_rb.prop("checked")) dest_pos = -2 * w;
   

   Please, don't eat away your braces.

   Remember the Apple SSL Bug? It was all caused because someone repeated a line after an if without braces. Like yours.

   Please, use braces.

6. Mixed quotes.

   Even though it only happened once, you have to check your quotes.

   Pick between single and double quotes and use the same style, from the beggining to the end of the document.

---

With all those changes, this is how I would write your code:

$(document).ready(function() {
 var $original = $("#original");
 var $scroll = $("#scroll");
 $original.clone().prependTo($scroll);
 $original.clone().appendTo($scroll);
 var original_width = $original.width();
 $scroll.width(original_width * 3);
 var $left_rb = $("#left_rb");
 function roll_it () {
 var scroll_by = 0;
 if ($left_rb.prop("checked")) {
 scroll_by = -2 * original_width;
 }
 $scroll
 .css('left', -original_width + "px")
 .animate({
 left: scroll_by + "px"
 }, 1000, "linear", roll_it);
 }
 roll_it();
});

• 200_success: w and dest_pos seemed obvious enough to me. I also don't see any danger with omitting braces if it's a short statement on the same line as the if.
• Ismael Miguel: @200_success Apple didn't too...
• 200_success: The key difference is on the same line. Apple's goto statements were on their own lines.
• Ismael Miguel: @200_success Correct. Because the lines were too big.
• 200_success: var roll_it = function() { ... } and function roll_it() { ... } have exactly the same scope. The only difference is hoisting.