First project

Community feedback

• 

  Zsolt Varjú•490

  @zsoltvarjuPosted over 1 year ago

  Hello Apag!

  Great solution a near perfect replication of the design, love it!

  here are my suggestions:

  CSS:

  • Consider adding comments to your code, adding comments to your CSS code can help make it more understandable and maintainable, especially if you are working on a larger project. Consider adding comments to explain the purpose of each section of code or any notable features. I know thet it is not that big project but it is good to practice ;)
  • Try to use more descriptive class names. While the class names used in the code are functional, they could be more descriptive. Consider using class names that are more specific to the content they are styling, like card-container instead of container. Again it is good to practice and will repay you greatly later on.

  HTML:

  • Add** alt text** to images. The <img> tag includes the alt attribute, it should be filled with a descriptive text that describes the image content. This is useful for accessibility. If i remember correctly 20-30% percent of the internet users are disabled so it is very important to watch out for accesibility.
  • Close <h6> tags: The <h6> tag for "Challenge provided by" is not closed properly, which can cause issues with the layout of the page. Make sure all HTML tags are closed properly.
  • Try to maintain the hierarchy of the Heading tags, the web should always contain a H1 and after the H1 comes the H2 and so on.

  I would like to mention that you centered your card correctly, there are alternatives for it though.

  • Me myself i prefer to use display:grid and then place-items: center. it automatically centers the items both horizontally and vertically.
  • If you use flex you can use align-items:center combined with justify-content:center. One is centering it horizontally the other vertically

  I know i mentioned a lot of things here but none of them are a serious problem just a suggestion! :)

  Once more really great work and happy coding! :)

  Marked as helpful

  1

  ---

  

  Apag•20

  @dizososPosted over 1 year ago

  @zsoltvarju Thank you for you feedback :) i will take notes and try to improve!

  1
• 

  Hassia Issah•50,650

  @HassiaiPosted over 1 year ago

  Replace <div class="container"> with the main tag and <h2> with <h1> to make the content/page accessible. click here for more on web-accessibility and semantic html

  Give the alt attribute in the img a value. The value of the alt attribute is the description of the image. For decorative images like icons, there is no need to give it an alt value, for more on alt attribute Click here.

  Every html must have <h1> to make it accessible. Always begin the heading of the html with <h1> tag wrap the sub-heading of <h1> in <h2> tag, wrap the sub-heading of <h2> in <h3> this continues until <h6>, never skip a level of a heading.

  <h6>Challenge provided by <a href="https://www.frontendmentor.io/home">Frontend Mentor</a> <h6>Coded by<a href="https://www.frontendmentor.io/profile/dizosos">Apag</a></h6> should be out of <div class="container"> and wrap in a footer tag.

  To center .container on the page using flexbox only instead of flexbox and margin, add justify-content: center to the body and remove the margin value in .container.

  body{
  min-height: 100vh;
  display: flex;
  align-items: center;
  justify-content: center;
  }
  

  Use relative units like rem or em as unit for the padding, margin, width values and preferably rem for the font-size values, instead of using px which is an absolute unit. For more on CSS units Click here and here

  Hope am helpful.

  Well done for completing this challenge. HAPPY CODING

  Marked as helpful

  0