nmoskaleva / hyf-homework Goto Github PK

Very impressive!
You are certainly not making my job of finding stuff to improve very easy.
HTML validates with no errors. Css validates with no errors.
Good use of semantic tags. HTML following the style guide perfectly.
I also like how close you have come to the original design.
So, I have to go look at really small details.
Like - the colors - they are not exactly the same as used in the design (in my experience that often means a lot to designers). Like, the grey used in the design is #FAFAFA but are using #F0F0F0 (so slightly darker). The purple footer should have been #6635CC but you have #4D16A7. If you want, you can use a tool like 'Color Cop' or another color picker to get the colors from an image. However, in-real-life a designer would often give you a style guide with colors and fonts to use (or the photoshop file for the design), so you wouldn't have to figure it out yourself.

The big challenge here was of course to make sure it worked responsive and again I think you have come very close to the design as outlined in the assignment!
Well done!

Privet Natalia. Here is some more in-depth review of your homework for week 1. Sorry for the delay. As I wrote on slack, overall it looks really good (очень хорошо!).
There are a few minor details though:

1. The line referenced in this issue above (line 7) a link to a few google fonts. That's all fine, but technically the line breaks the official W3C validator for valid html syntax because the pipe '|' character is not allowed in a url. The solution is easy though - just replace it with the URL-encoded value for the same character: '%7C' (I bet you have seen the % notation often in urls in your browser - that's url-encoding).
2. As I also wrote on Slack. It's great that you are using Header, Footer (although that is empty?) and Aside - but it would be great to see more semantic tags like Main, Section and so on. W3 Schools has a good list of the most commonly used here: https://www.w3schools.com/html/html5_semantic_elements.asp
3. Your picture is fairly large 3.4 MB and takes a while to load on the site. It could easily be resized and optimized without losing quality to at least 1/10th of the current file size. Also you have resized it with in-line styling. It would make more sense if you gathered all your styling in the style sheet.
4. It is not built adaptive or responsive which means that it's not ideal to read on for instance mobile devices (where it all just becomes small). As far as I know that wasn't part of the scope for the assignment (as the foundation for this was first mentioned in class last sunday), so this is just a hint that trying to make a flexible layout can improve the mobile experience a lot. I can see that you tried to start on this in the CSS with the media query, but it doesn't seem to be working as expected in my browser.
5. Some of your indentation in the html is a bit mixed up, which makes it harder to read - it's only a few places though - like around the <ul> tags.

I think that's all I have. Keep up the good work!