Cache thingsDOM selectors
You get the length of the array andselect the target container over and over again. You should get bothit once outside the function, cache themit in variablesa variable and reuse themit:
const quotesLength = quotes.length;
const target = document.getElementById('quotes');
Naming
Try to use descriptive names instead of myarray
, myVar
and quote
:
const quotes = [];
function changeQuoteRandomly() {}
Style
It's easier to read, if you use single quotes on the strings, as you don't have to escape the double quotes:
quotes.push('"Important words" by Someone');
You sometimes use a semicolon at the end of the line and sometimes you don't – use only one way and be consistent:
var myarray = [] var myVar = setInterval(quote, 15000);
I would prefer using push
to add elements to the array instead of setting the index manually. Or you could even add all elements during initialization:
const quotes = [
'"Important words" by Someone',
'"Other important words" by Someone'
];
Both ways make it easier to re-arrange elements later as you don't have to keep track of the index yourself.
Semantics
Currently your markup looks like this:
<div id="quotes"> "The single best piece of advice: Constantly think about how you could be doing things better and questioning yourself."<br> <strong> Elon Musk</strong> </div>
You could improve this, by using blockquote
, cite
and even figure
elements. Here you can find more infos and examples on W3C: 4.4.5. The blockquote element.
User experience
While this is a nice addition on your website, there are a few downsides:
- There's no introduction to this section, the visitor has no clue why you're showing them. Try to relate them to your business.
- The first quote is shown after 15s, until then only an empty blue container ist visible.
- 15s between quotes is a really long time. There's no indicator that more is coming, so there's a great chance that a user will not see a second one.