@Amer-10Submitted about 1 month ago
Luiz Felippe
@luizfbnAll comments
- @luizfbnPosted about 1 month ago
Really nice solution! Few things you could improve:
- Change the font family of Ingridients, Instructions and Nutrition;
- For a better semantic HTML, you could use <section> tag instead of <div> on each section. https://www.w3schools.com/tags/tag_section.asp
- You could use a HTML <table> on Nutrition's table. https://www.w3schools.com/html/html_tables.asp
- Remove the image border on small screens;
- Preparation time title has a different font color (check style-guide.md file);
- Nutrition table's info has a different font color;
Good job and keep studying! :)
Marked as helpful1 - @ghamsinSubmitted about 1 month agoWhat are you most proud of, and what would you do differently next time?
hi i use resposive and variable font + give smoth animation to resposive title and link + hover animation on avatar img
What challenges did you encounter, and how did you overcome them?to took alot of time learn variable font and use animation this but this satisfyed ilike result
What specific areas of your project would you like help with?analysis the project spets and Structuring + clean coding and some tip to faster coding becus im slow as hell tnxxx ^_^ have a nice day
@luizfbnPosted about 1 month agoNice solution! I found some things you can improve:
About HTML:
- You used a lot of divs, this is not wrong but you could use semantic HTML. For example, for list links you can use <ul> and <li> tags;
- There's no need to wrap a div inside a div like you did on class "container" and "main-menu". There's no difference between them.
About CSS:
- Links should be in a higher font weight;
- On link card hover, it should be background green and font color grey. You can check it on active state image, inside design folder;
- Try to keep it responsive (on small screens the card breaks);
- On style.css there are some empty classes;
- There are repeated style about attribution class in CSS file and HTML;
Good job and keep studying! :)
0 - @MarcosAvgSubmitted about 1 month agoWhat are you most proud of, and what would you do differently next time?
I liked the use of flexbox for the layout from the container to the child elements, sectioning with BEM.
What challenges did you encounter, and how did you overcome them?In the author element, I found that I could not reduce the size of the image with flex-shrink. I solved it by putting the image in a and it could be reduced without problems.
What specific areas of your project would you like help with?I would like help with CSS structure. I use Flex and utilities, but I am not clear if I have omitted some good practice or if it can be optimized in some way.
@luizfbnPosted about 1 month agoGood solution. I found some stuffs that you can improve.
About HTML:
- You can wrap your img with figure tag: https://www.w3schools.com/tags/tag_figure.asp
- Inside content section, you can wrap the learning stuff and the publish date inside a header;
About style:
- The card shadow needs to be bigger;
- On card hover, the shadow needs to expand (you can see this on Figma's prototype presentation) and cursor needs to be pointer;
- The typography needs to be reviewed. For exemple, the card content paragraph have wrong font weight.
Tips:
- Always check if your solution is responsive, it's very important;
- Avoid using width and height based on percentage, it is very situational and if you don't use wisely, this can break your style on different screen sizes;
- Flex it's an awesome tool, but you don't need to use on everything. For this design, for example, you could use only blocks and inline blocks elements and you could be fine.
Good job and keep studying! :)
Marked as helpful1 - @khaleelibraheemSubmitted about 1 month ago@luizfbnPosted about 1 month ago
Nice solution. It is accessible, responsive and has semantic HTML. The only thing I think can be improved is the card border needs to be bigger. Good job! :)
0
