Code review 13 juni

[Anna-Kyra]

Ziet er netjes uit, hier zijn wat tips!!

HTML commentaar

Footer.ejs

proof-of-concept/views/preface.ejs

Line 55 in 1af4f24

<%- include('./partials/footer') %>

Je normale .ejs bestanden zijn niet afgesloten met de </body> & </html> tags. Ik zie dat je onderaan bij de 'preface.ejs' een footer partial include. In deze partial zit alleen een lege <footer>, dit is zeker handig om te gebruiken wanneer je meerdere pagina's hebt die dezelfde footer gebruikt, maar zorg ervoor dat de </body> & </html> end tags hier zijn inbegrepen.

Classes benamingen

proof-of-concept/views/dashboard.ejs

Line 4 in 1af4f24

<main class="d">

proof-of-concept/views/index.ejs

Line 2 in 1af4f24

<main class="i">

proof-of-concept/views/preface.ejs

Line 3 in 1af4f24

<main class="p">

Ik snapte na een tijdje pas dat je de verschillende pagina's bedoelde met die letters. Misschien handig voor duidelijkheid om dit voluit te schrijven. Zo is de CSS ook duidelijker voor iemand anders die naar je code kijkt.

Classes benamingen nav

proof-of-concept/views/partials/navigation.ejs

Lines 11 to 44 in 1af4f24

	<li><a href="/"
	<% if (current == '/') { %> class="active active-1" <% } %>>
	<p>Dashboard</p>
	</a></li>
	<li><a href="#"
	<% if (current == '/testing') { %> class="active active-2" <% } %>>
	<p>Google analytics</p>
	</a></li>
	<li><a href="/lessons"
	<% if (current == '/lessons') { %> class="active active-3" <% } %>>
	<p>Linkedin analytics</p>
	</a></li>
	<li><a href="/statistics"
	<% if (current == '/statistics') { %> class="active active-4" <% } %>>
	<p>Hotjar analytics</p>
	</a></li>
	<li><a href="/profile"
	<% if (current == '/profile') { %> class="active active-5" <% } %>>
	<p>Customize view</p>
	</a></li>
	<li><a href="/profile"
	<% if (current == '/profile') { %> class="active active-6" <% } %>>
	<p>Account</p>
	</a></li></div>

Bij je navigatie heb je bij elk element een class met active en een class active-1, 2 ,3 etc. Ik weet niet of de active staat veranderd per link, maar als dit het zelfde is, is de tweede class niet nodig. Je kan ook met :nth-of-type(..), :last-child, :first-child, etc. werken om de knoppen te differentiëren. Ook handiger om te schrijven wat de class anders doen, ipv een nummer wat het coderen sneller kan maken.

HTML nesting indents

Let in sommige .ejs bestanden (bijvoorbeeld dashboard, navigatie, head) op je indents voor de html nesting.
Het is nu soms het een beetje verwarrend wat waar ingenest is. Je kan nu wel alles vinden, maar als het bestand veel groter word kan dit erg chaotisch zijn en vooral als iemand anders in je code moet werken.
Als je dit niet zelf wilt doen kan je naar extensions kijken zoals prettier/beautify.

In je CSS bestanden doe je dit wel erg goed!

bron: Over waarom html indents belangrijk zijn

CSS commentaar

TOP

Goed dat je de general styling boven aan de pagina plaatst en dit met de verschillende onderdelen met een comment verteld. Nu weet ik met gemak waar ik ben!

mediaqueries tip

proof-of-concept/public/styles.css

Line 297 in 1af4f24

@media (min-width:750px) {

Je kan bijvoorbeeld de mediaqueries korter schrijver met de min-width:

@media (width > 750px) {}`

Minder tekens✨

@media (30em < width < 50em) {
  /* … */
}

hiermee kan je ook een range mee aanduiden als dit nodig zou zijn in je project.

Bron: MDN mediaqueries

Server

Ziet er goed uit!!

Overig

Misschien handig in je partials een apart mapje te maken voor je svg bestanden. Zo word je indeling wat overzichtelijker.

Dit geldt ook in de public map. Nu staan al je fonts scripts.js en style.css door elkaar heen. Handig om een apart script, style en assets mapje voor het overzicht aan te maken. En in je assets mapje kan je ook nog een fonts, img, etc. mappen aanmaken.