Chibs+Ed - code review #5
Description
README
[Does the README describe the project using the subheadings: Why?, What?, How? i.e. Why have you created this repo, what does it do, how does it do it?]
There's been a start on the Readme, subheadings are in place, but looks like they'll come back to update that before the presentation.
User stories
[Does the project meet the user stories for that week?]
yes, but contact form doesn't have required fields yet. I can tab the site which is good for accessibility, Wasn't able to test screen reader. Really great responsiveness for different devices! It looks really slick too! I like the animated title, the logo in front of the image on the landing page, and the layout of the socials. A really unique colour scheme too! well done!
Learning outcomes
[Does it demonstrate the learning outcomes for that week?]
Yes, great use of semantic html, and performed well with lighthouse.
UI bugs
[Can you see any obvious bugs or areas to improve?]
The lighthouse performance assessment highlighted loading time, and I did experience a gap when the page was loading. Totally understand the good practice of the javascript DOMContentLoaded, but as it does take a moment to load, there was a gap where you can see some basic html without styling before all the content kicks in. It's a small thing as the site looks really great when it does load, but if there was time left, that might be something to explore in tandem with the Domcontentloaded event listener, to have something in its place like a loader or something to blank out the nuts and bolts before it fully loads.
Instructions
[Does everything work as expected or were there missing instructions?]
File structure
[When you open the project in your editor, does the file structure make sense?]
Yes, really well organised folder structure, good practice as will allow scaling up in this setup, if more pages of css or JS were to be added.
Flow of control
[ Can you you follow the different paths the code might take?]
Yes, I think so. There are lots of comments which is great. There's a lot of inserting html from javascript, so I can see that might get a bit complicated if the project were to scale up. My question to that would be if using native html might work just as well? But I don't know, there might be specific reasons due to the use of JSON, which is beyond my skillset, just something to be slightly wary of with accessibility. But looks very modular, and maybe nice to produce the html this way without having to repeat yourself.
Naming
[Do variables and functions have clear and descriptive names?]
Great naming conventions, really clear
Readability
[Do you understand the code?]
Mostly the slideshow JS, but the JSON is a little beyond me, but it does look very neatly formatted and well laid out, I'm sure if I knew a bit about JSON it would be very clear.