Cache DOM selectors
You select the target container over and over again. You should get it once outside the function, cache it in a variable and reuse it:
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.