@gmagnenat
Posted
Hi, congrats for completing the challenge ! I just finished it as well and yours was picked for a review :)
I like the stuff you did for the image optimisation it's great ! I'll pick some ideas to add in my personal notes.
Does the solution include semantic HTML? I don't have much to say on the html structure but maybe some part could be simplified.
- Your shape element structure looks quite complicated to me. It could be just a div with a before pseudo element of 1px width.
- Still about the shape element, as it represent the number linked to a section, I would add the second one inside the footer section markup for more logic.
Is it accessible, and what improvements could be made? Currently the layout is breaking if the user changes the default browser font size. If for example I increase the font-size to 30px the layout should present the mobile version zommed in. This may be caused by your media queries set in pixels instead of relative units. How to "make a site responsive" and use Media Queries well
Is the code well-structured, readable, and reusable? There is quite some work on the file structure and folder structure for sass. I like it. I might take some idea here for my next projects :) however, I find it a bit difficult to navigate and debug. Maybe some more information on the tooling can help. In your README you mention npm init, gulp, autoprefixer etc.. it all sounds great but there is no package.json in your folder and no information on which package to install. This can be improved for better collaboration with other developers.
Does the layout look good on a range of screen sizes? The hero images are cropped if you test on larger screen. Image is cropped too on mid size screen and small screen. I spent some time trying to find the best solution for this. You can check my submission for this challenge.
I used a wrapper larger then the container to allow the overflow of the images. the container itself has overflow hidden but will get wider on larger screen and the images will be visible without any crop. I don't know if it's the best way for this, take it with a pinch.
Does the solution differ considerably from the design? The solution looks close to the original design and respect the design intention, good job !
I hope you find my comments helpful for improving your solution. Don't hesitate to reach out if you have any questions.
Happy coding !
Marked as helpful