I learned HTML, CSS, JS & some React before. 1st time practicing alone

Community feedback

• 

  Dominik Gartz•140

  @domieeePosted almost 2 years ago

  Hi Abdoulaye

  I took a look at your code and found a few things you could adjust.

  you have given the img element a height and width of 180px. This would be unresponsive if necessary and would not adjust to its parent element. You could fix this by giving the container a max-width of the desired width and the width: 100%;. The img should also have the property width 100%.

  for web accessibility you should add a <main role="main>" and nest your code in it. Also you should change your h3 element to an h1 element. this is also important for this case. You can take a look at google and see what is important for web accessibility.

  try to use rem instead of px for the font size. if necessary the font's wouldn't adjust to the brower size with px.

  Keep coding and have a great day

  Marked as helpful

  0

  ---

  

  T

  Grace•31,270

  @grace-snowPosted almost 2 years ago

  @domieee great feedback but no need to put role of main on a main element. It already has that role

  0

  

  Dominik Gartz•140

  @domieeePosted almost 2 years ago

  @grace-snow well, my bad. thanks for the correction.

  0
• 

  Hassia Issah•50,650

  @HassiaiPosted almost 2 years ago

  Replace<div class="container">with the main tag, <h3> with <h1> and <div class="attribution"> with the footer tag to fix the accessibility issues. click here for more on web-accessibility and semantic html

  wrap " Scan the QR code to visit Frontend Mentor and take your coding skills to the next level" in a <p> . e.g: <p> Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>

  Use the colors that were given in the styleguide.md found in the zip folder you downloaded

  Give .container a fixed width value, for a responsive content, give .container a fixed max-width value. eg: width:320px Give p and h1 the same padding left and padding right value , the same margin-top value and a text-align: center.

  Give the img a max-width of 00% instead of a width and height value in an inline css.

  To center .container on the page, add min-height:100vh; display: flex; align-items: center: justify-content: center; or min-height:100vh; display: grid place-items: center to the body.

  To center .container on the page using flexbox:
  body{
  min-height: 100vh;
  display: flex;
  align-items: center;
  justify-content: center;
  }
  

  To center .container on the page using grid:
  body{
  min-height: 100vh;
  display: grid;
  place-items: 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

  Hope am helpful.

  Well done for completing this challenge. HAPPY CODING

  Marked as helpful

  0

  ---

  

  T

  Grace•31,270

  @grace-snowPosted almost 2 years ago

  @Hassiai padding should not be used on headings and paragraphs.

  Instead, this whole card should have padding. The heading and paragraph only need vertical margins

  1