royvanderzon/web-app-from-scratch

Minor project

web-app-from-scratch's Issues

Use addEventListener instead of binding event as a property

Web-App-from-Scratch/week 2/opdracht 1/main.js

Line 80 in 15a1897

request.onload = function() {

Use meaningful variable names

Consider using 'section' (for example) instead of 'val'

Web-App-from-Scratch/opdracht 5 - spa/assets/js/main.js

Line 25 in 332c196

sections.forEach(function(val, index, arr){

addEventListener not required, parameter not used

addEventListener method not required

There is a possibility to write less code reaching the same results using the "hashchange" method. The "addEventListener" method is not required.

Parameter in addEventListener method not being used

The second feedback I have is that you are using an "event" parameter in the "addEventListener" method, but are not using it.

Current

window.addEventListener("hashchange",function(event){
      sections
      sections.toggle(window.location.hash);
});

Solution

      window.onhashchange = function() {
        sections.toggle();
      };

Lint your code

Be consistent in the use of spaces in your statements

Strict mode

Are you sure you want to run strict mode on the globals scope?

missing semicolon

app.js:8

Your app structure could be clearer

Please discuss!

Use em instead of pixels

Only use px for borders and box-shadows, not for padding, width, etcetera..

Bind events instead of adding event as a property

AddEventListener is prefered over onhashchange..

Refatored geo script is not structured in object literals

Don't nest single <article> elements in <section> elements

section elements (

or ) are ment to group multiple elements

Can't find research on libraries, frameworks & spa

Inline style, use class for hiding