Skip to main content
deleted 64 characters in body
Source Link

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.

Cache things

You get the length of the array and the target container over and over again. You should get both once outside the function, cache them in variables and reuse them:

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.

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.
Source Link

Cache things

You get the length of the array and the target container over and over again. You should get both once outside the function, cache them in variables and reuse them:

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.