Modularization
I like that you pulled the functions that place the quote info into the HTML out into their own function. Nice job! It also improves readability.
Organization/Global Variables
You take two lines to both declare the var quotes global array and initialize it. You could do both in the same step.
var quotes = [ {...}, {...}, ... ];
Because of your image preloader, you have a global variable img[] floating around in the middle of your code. You should move it up to the section in which you declare your global variables so there isnât potential for overriding it with a local variable name.
Slimming down your code
In function indexOfObject(arr,obj), in the very heart of the nested for loop, in the innermost level, the 2nd condition of the 2nd if statement is redundant:
function indexOfObject(arr, obj) {
let objKeyArr = Object.keys(obj);
for (let i = 0; i < arr.length; i++) {
for (let k = 0; k < objKeyArr.length; k++) {
if (arr[i].hasOwnProperty(objKeyArr[k]) === true) {
if (obj[objKeyArr[k]] != arr[i][objKeyArr[k]]) {
break;
}
if ((k === (objKeyArr.length - 1)) && (obj[objKeyArr[k]] === arr[i][objKeyArr[k]])) {
return i;
}
}
}
}
return -1;
}
Youâve already checked for the second condition in the 1st innermost if statement. By virtue of the fact that you didnât break the for loop, you only need to check whether (k === objKeyArr.length - 1) Notice I left out some of the parentheses you included. If you look at MDNâs operator precedence table, youâll see that the general rule is:
- unary (++/
--)
- binary (
+-/*)
- tests (==, <=)
- boolean functions (||, &&)
- then finally, assignment(=,+=,*=)
If really readable code is your goal, then that tableâs a good thing to bookmark. I made a fiddle to show you an example of this stuff in play See if you can understand why a and b are what they are and why the test passed.
In the Main Function, you are testing your boolean flag, instead of just recognizing that it already has a value of 0 or 1 (truthy values so this code will work, but youâd be better off assigning false initially for legibility). Very common mistake. Now that youâve named your flag well, letâs see if this makes your application logic more understandable:
function main() {
if (!hasDisplayedFirstQuote) {
new_Quote();
fadeIn();
hasDisplayedFirstQuote = true;
} else {
fadeOut();
setTimeout(new_Quote, 1000);
setTimeout(fadeIn, 1000);
}
}
Variable Name Suggestions
function getRandomInt(max) {
return Math.floor(Math.random() * Math.floor(max));
}
Since you know you will use this function pretty much exclusively to pick a random index for an array of size max (a very common programming pattern) I would rename it thusly:
function getRandomIndex(arraySize) {
return Math.floor(Math.random() * Math.floor(arraySize));
}
The only reason Iâd do it that way is to separate it from the pattern of getting a random number between 1 and N, inclusive, where max is a perfectly explanatory variable name.
Cross-fade Woes
Now to the juicy stuff! Are you using Firefox? Because I was having the same issues with your sample pen. The top answer to this question on StackOverflow said you did it correctly, but it doesnât work on Firefox or IE. I tried it on chrome after forking it, and it worked perfectly, but the modified CSS looks smoother in my opinion (timing is your choice):
transition: background-image 1s ease-in-out;
I just had simple colored backgrounds, but I found an example of an idea that I thought would work for you: 2 HTML img elements, one on top of the other by z-index, and you animate the opacity. His example does it on hover, and doesnât manipulate the z-index, but that should give you plenty to get started (you could keep the z-indices for each element fixed, but alternating which image you place on which canvas by using a boolean flag oddNumberedImage or something.
FYI, I have never heard of a preloader, but itâs awesome with your high-res images. You taught me something Iâm totally going to use!
.
.
.