QR Code Frontend Mentor

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  Hi, Amanda! 👋

  Congratulations on completing your first Frontend Mentor challenge! 🎉

  First, I recommend writing your code with a consistent format. This makes it easier for developers to read your code. I prefer using code-formatter to automate this for me. You can use Prettier to make sure the code base has a consistent format.

  I suggest wrapping the card with main element. Then, wrap the attribution with footer tag. By wrapping each content with the right landmark element, the users of screen reader can use those landmarks to navigate through the web page.

  For the styling:

  • I recommend making the body element as the grid container for the card. Also, set the background color on the body element. This way, you can remove the background-color div element.
  • There's no need for max-width on the body element. It should always fill the entire page.
  • Set min-height: 100vh on the body element. It makes sure the body element always fill the entire page and makes the card vertically center.
  • I don't think it needs margin: auto to make the card in the middle of the page. I think grid is already enough.
  • Set the font family on the body element. Most of the text element (e.g. p, headings, etc) will inherit the font settings from the body element.
  • Set max-width on the card instead of setting grid-template-columns: 375px;
  • There's no need to set the height of the card (or .background). Let the content inside the card controls its height.
  • Use either px or rem unit for the margin and padding. Using the percentage unit can lead to unexpected results since it is a relative unit. It means the value can be really small or really high. Most importantly, you can't give a max or min value.
  • There's no need for media queries to make the site responsive. If you set a max-width to the card it will allow the card to shrink and grow until the specified value.

  I hope this helps! Happy coding!

  Marked as helpful

  0

  ---

  

  Amanda Buttineau•200

  @ab1820Posted about 2 years ago

  @vanzasetia Hey, thanks for the input! :) I completely forgot to add the main element to my HTML, lol. I am going to try some of these awesome tips and see which ones work!

  1
• 

  Adriano•38,390

  @AdrianoEscarabotePosted about 2 years ago

  Hi Amanda Buttineau, how are you?

  Welcome to the front-end mentor community!

  I really liked the result of your project, but I have some tips that I think you will enjoy:

  A good practice to center content is using grid or flex-box, avoid using margin or padding to make placements, use only in the latter case! we can do it like this:

  Flex-box:

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

  GRID

  body {
      display: grid;
      min height: 100vh;
      place-content: center;
  }
  

  The rest is great!

  I hope it helps... 👍

  Marked as helpful

  0
• 

  David Bedford•180

  @djbedfordPosted about 2 years ago

  Hey, if you want to use grid you can set the display property to grid and then apply the grid-template-columns property specifying the width you want the column to be. So for example if you want a one column grid that is 500px you would do the following

  .container { display: grid; template-grid-columns: 500px; }

  You can get a better explanation here: https://developer.mozilla.org/en-US/docs/Web/CSS/grid-template-columns

  For this exercise though, I would personally stick with using flexbox as you are only working in 1 dimension, grid is more suitable for working in 2 dimensions.

  0

  ---

  

  Amanda Buttineau•200

  @ab1820Posted about 2 years ago

  @djbedford I definitely should have used flexbox! I'll probably give that a try as another solution. Thanks! :)

  0