Html & CSS

Community feedback

• 

  Numan Iftikhar•160

  @numan-iftikharPosted about 2 years ago

  Hey dear, I've made a pull request to your code on github. I've fixed a lot of things. Do check it out and remember to read the pull request message :)

  0
• 

  Lucas 👾•104,340

  @correlucasPosted about 2 years ago

  👾Hi Mysooma Maqbool, congratulations for your first solution!👋 Welcome to the Frontend Mentor Coding Community!

  Great solution and great start! By what I saw you’re on the right track. I’ve few suggestions to you that you can consider to add to your code:

  1.Fix the alignment of the whole content using flex and min-height to manage the vertical alignment and make everything centered.First of all put min-height: 100vh to the body to make the body display 100% of the viewport height (this makes the container align to the height size that's now 100% of the screen height) size and display: flex e flex-direction: column to align the child element (the container) vertically using the body as reference.

  body {
      min-height: 100vh;
      background-color: rgb(167, 189, 196);
      display: flex;
      align-items: center;
      justify-content: center;
      flex-direction: column;
  }
  

  2.Add the alt text to allow screen readers to recognize that img. Adding alternative text to photos is first and foremost a principle of web accessibility. Visually impaired users using screen readers will be read an alt attribute to better understand an on-page image.

  3.Replace the <div> you’ve used to wrap the card container with <main>, that in this case is the best tag and shows that this is the main block of content of this page.

  4.Instead of using <b> for the heading, it's better you use <h1> since this is the main heading for this page. Remember that any page needs a h1 heading to display which is the more important text. Note that you need to increase the headings by one level, like h1, h2, h3 to show the titles hierarchy and you cannot have more than one h1 heading.

  5.Add the correct color for this background, the value is background-color: #D6E1F0

  ✌️ I hope this helps you and happy coding!

  0
• 

  Travolgi 🍕•31,400

  @denieldenPosted about 2 years ago

  Hi Mysoona, congratulations on completing the challenge, great job! 😁

  Some little tips for optimizing your code:

  • add main tag and wrap the card for improve the Accessibility
  • you can use article or div tag instead of p to the container card for improve the Accessibility
  • p is only for contain text
  • img element must have an alt attribute, it's very important!
  • remove all unnecessary code, the less you write the better as well as being clearer: for example the a container of image in this case
  • remove all margin from card container (in this case the p)
  • use flexbox to the body to center the card. Read here -> best flex guide
  • after, add min-height: 100vh to body because Flexbox aligns child items to the size of the parent container
  • instead of using px use relative units of measurement like rem -> read here

  Hope this help! Happy coding 😉

  0