Great start for far but you need to work on your overall design.
Here are some tips.
##1) Separate the css, html and javascript into their own files.
If you were place all your css into a file called "style.css" and javascript in a file called "main.js", then you could import the files from your html like so.
... more code
<head>
<link rel="stylesheet" type="text/css" href="style.css" />
</head>
... more code
<script src="main.js"></script>
##2) For your css, try using a creating a class for the box to share common properties.
Old Code:
#a {
width:400px;
height:140px;
background: #FF0000;
float: left;
}
#b {
width:400px;
height:140px;
background: #FFFF00;
float: left;
}
#c {
width:400px;
height:130px;
background: #00FFFF;
float: left;
}
New Code:
#a {
background: #FF0000;
}
#b {
background: #FFFF00;
}
#c {
background: #00FFFF;
}
.box{
width:400px;
height:140px;
float: left;
}
Remember to add the classes.
<div id="abc-container">
<div id="a" class="box"></div>
<div id="b" class="box"></div>
<div id="c" class="box"></div>
</div>
##3) Don't repeat yourself.
The if conditions for firstval and secondval can be combined by using or, ||.
Old Code:
if(firstval == 400){
StopRun();
StartTimer()
return;
}
if(firstval == 800){
StopRun();
StartTimer()
return;
}
runCarousel = setTimeout(Carousel, 10);
New Code:
if(firstval == 400 || firstval == 800){
StopRun();
StartTimer()
}else{
runCarousel = setTimeout(Carousel, 10);
}
##4) Declare all variables are the top of a function.
Old Code:
var runCarousel, runTimer;
firstval = 0;
secondval = 0;
New Code:
var runCarousel, runTimer, firstval = 0, secondval = 0;
##5) Pass the function name instead of string to setTimeout()
Also it's better to use multiplication when setting the timeout delay.
Old Code:
setTimeout("Carousel()", 10000);
New Code:
setTimeout( Carousel, 10 * 1000);
##6) Attach all the global variables to a object literal.
##7) Name variables and functions based on their operations.
- Rename
Carousel() to moveToNextSlide()
##8) Cache elements for faster lookup.
Code:
var infoEl = document.getElementById('asas'),
container = document.getElementById('abc-container');
##9) There needs to be a limit for firstval inside Carousel()
Add this:
var abc_container = 1200;
firstval += 10;
if( abc_container < firstval ){
firstval = 0;
}
##10) Use variables in place of numeric constants.
It's hard to understand the meaning of 400.
It seems that 400 is the width of each box.
##11) secondval isn't needed. Just place everything inside setTimeout() for 10 seconds.
Better yet seconds as a parameter.
Old Code:
function StartTimer() {
secondval += 1;
el.innerHTML = "-" + secondval;
if (secondval == 10 || secondval == 20 ) {
runTimer = window.setTimeout(StartTimer, 1000);
}else{
...
}
New Code:
function StartTimer( seconds ) {
setTimeout(function(){
window.clearTimeout(runTimer);
moveToNextSlide();
}, seconds * 1000);
}
##12) Read the source code from other image slideshow to find out better techniques.
Here's a place to start. http://www.tripwiremagazine.com/2012/08/jquery-image-slider.html
##Final Code:
HTML:
<!DOCTYPE html>
<html>
<head>
<link rel="stylesheet" type="text/css" href="style.css" />
</head>
<body>
<div id="wrapper">
<div id="abc-container">
<div id="a" class="box"></div>
<div id="b" class="box"></div>
<div id="c" class="box"></div>
</div>
</div>
<div id="asas"></div>
<!-- !!! Use script tag to import "main.js" -->
</body>
</html>
Javascript:
var carouselTimer, runTimer, firstval = 0,
boxWidth = 400, abc_container = 1200,
infoEl = document.getElementById('asas'),
container = document.getElementById('abc-container');
function moveToNextSlide() {
firstval += 10;
if( abc_container < firstval ){
firstval = 0;
}
container.style.left = "-" + firstval + "px";
infoEl.innerHTML = "container.style.left.px = " + firstval;
if (firstval % boxWidth) {
carouselTimer = setTimeout(moveToNextSlide, 10);
}else{
window.clearTimeout(carouselTimer);
StartTimer( 2 );
}
}
function StartTimer( seconds ) {
setTimeout(function(){
window.clearTimeout(runTimer);
moveToNextSlide();
}, seconds * 1000);
}
StartTimer( 2 );
CSS:
#wrapper {
width:400px;
height:140px;
position:absolute;
left:50%;
top:50%;
margin: -70px 0 0 -200px;
background: #383838;
overflow: hidden;
}
#abc-container {
position: absolute;
width:1200px;
height:140px;
}
#a {
background: #FF0000;
}
#b {
background: #FFFF00;
}
#c {
background: #00FFFF;
}
.box{
width:400px;
height:140px;
float: left;
}
Demo: http://jsfiddle.net/vSWmx/
