CSS, HTML

Community feedback

• 

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  Hi Sophia,

  Welcome here, good job :D

  I have checked your project and I would like to help you to achieve important points => it will help you to build better projects in the future, a few tips from me below:

  HTML:

  • instead of div class="bigbox" you can use the header tag;
  • text at the top: it is not a good practice to use divs for the text => instead of it you can use the heading - h1, h2 and paragraphs. Just to let you know, you should only use one h1 per page. Using more than one will not result in an error, but using only one is seen as a best practice. => h1 is the most important heading, and tells you what the purpose of the overall page is;
  • instead of div class="container2 you can use the main tag and inside of it four divs as a wrapper for each card => Divs are perfect for generic groupings of content but from the other hand divs are also semantically inert elements — elements that don’t do or say anything, of course, they are important too but it easy to overuse them. Please, change a bit the HTML structure by using semantic tags;
  • names of the classes are not readable and descriptive => I'd recommend learning BEM naming convention;
  • alt text => don't need to use words like picture or image, photo, icons in the alt text as it's already announced as being an image. In this project, icons have an only a decorative role - that's a reason why alt text should be provided as an empty (alt="") so these icons can be ignored by assistive technologies, such as screen readers => in this project images have only a decorative role so alt should be empty;

  CSS:

  • you didn't reset CSS styles by adding the selector * { margin: 0; padding: 0; box-seizing: border-box; }; (please, read about it why we have to use them);
  • don't have to use position absolute and position: absolute & transform:translate(-50%, -50%) to center the content => use the flex;
  • RWD: yes, doesn't work on mobiles, tablets;
  • important => I'd recommend reading more about flex and height / width in the flex context. You gave explicit height and width => that's not a good practice especially that you want to use the flexbox or grid. It is essential to understand well the height and width vs min-height/max-height & min-width/max-width. You shouldn’t need to give items height unless they have a background or absolutely-positioned or floated elements within them that would not normally be accounted for in the height of an element. Experienced developers use min-height and min/max-width more than anything else. It allows elements to grow and shrink.

  In the end, I'd recommend reading about:

  • semantic tags, headings and alt (why they matter);
  • it was quite difficult to read your HTML structure because you didn't put proper spacing (gaps between elements) => readable code helps a lot, it will be good for you and the others (code refactoring, code review etc.);
  • do lots of simple exercises by using flexbox.

  Happy coding and keep going. Ps. Don't forget to upvote any comments on here that you find helpful.

  Greetings :D

  2

  ---

  

  Sophia•50

  @Sophia0501Posted almost 4 years ago

  @SzymonRojek

  Oh, wow! Thanks a lot for your help! Now it's much easier to understand what I should tackle first :)

  1

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  @Sophia0501

  I am glad you are satisfied. Flexbox takes a while but grid is more difficult. Start to learn flexbox first and when you will be confident with it then grid. I would like to recommend two courses providing by Wes Bos for free:

  • flexBox;
  • grid.

  Check them out, are amazing 😃

  1