I have a title text, which I want to fade into a box. Afterwards, a subtitle should fade in beneath it. Once both are visible, they should fade out, and the next set should fade in, in the same fashion.
This I have created, however, I have my doubts on how well it runs, that is, if it takes to much CPU for the browser. One of my concerns are also the recursion depth of Javascript.
I have the following code (which I have also included in a jsfiddle, along with CSS and HTML http://jsfiddle.net/ukewY/1/)
var no = 3;
function fadeText(id) {
// Fade in the text
$("#text" + id).animate({
opacity: 1,
left: '+=50'
}, 5000, function() {
// Upion completion, fade in subtext
$("#subtext" + id).animate({
opacity: 1,
}, 5000, function() {
// Upon completion, fade the text out and move it back
$("#subtext" + id).animate({
opacity: 0,
}, 1000, function() {
$("#text" + id).animate({
opacity: 0,
开发者_开发百科left: '+=200'
}, 3000, function() {
// Yet again upon completion, move the text back
$("#text" + id).css("left", "19%");
$("#subtext" + id).css("left", "10%")
fadeText((id % no) + 1);
});
});
});
});
}
$(document).ready(function() {
fadeText(1);
});
My question is if this is the right way to do it; or if there is a better, maybe none-recursive, way of doing this?
PS. As this is for a banner for a website, I do not worry about id
being to big, as the people have probably moved on since.
The recursion seems fine to me, but there are a few other things I spotted:
- You probably want to dynamically read the number of titles, rather than having to specify them at the top of the script.
- You are using the same selectors $("#text" + id) and $("#subtext" + id) twice in each title. You should only do it once, and assign it to a variable. This means you only have one function call, not two.
- You may want to use the eq() selector to eliminate the need for $("text" + id) and make it tidier
- There is a couple of data maps that you are passing to animate() that end with commas even though there is only 1 value (specifically "{opacity: 0,}"). This will cause problems on some browsers.
- I'm not 100% sure, but i think calling a function from within itself is bad, you should use setTimeout (and use an anonymous function if you need to pass the function some parameters to avoid it using eval())
- I know you said it didn't matter, but id getting too big can happen if a user just leaves your page open (e.g. answers the phone then has to rush out). They shouldn't come back to an error.
- You can use $(DO STUFF) rather than $(document).ready(DO STUFF)
I took car of these and took the liberty of tweaking your code to the following:
function fadeText() {
thistext = $text.eq(titleid);
thissubtext = $subtext.eq(titleid);
thistext.animate({
opacity: 1,
left: '+=50'
}, 5000, function () {
thissubtext.animate({
opacity: 1
}, 5000, function () {
thissubtext.animate({
opacity: 0
}, 1000, function () {
thistext.animate({
opacity: 0,
left: '+=200'
}, 3000, function () {
thistext.css("left", "19%");
thissubtext.css("left", "20%");
if (titleid != $text.length - 1) {
titleid++;
} else {
titleid = 0;
}
setTimeout(fadeText, 0);
});
});
});
});
}
var titleid=0;
$(function () {
$text = $("span.floating-text");
$subtext = $("span.floating-subtext");
fadeText();
});
精彩评论