QR Code Component CSS Flexbox solution

Community feedback

• 

  Hassia Issah•50,650

  @HassiaiPosted over 1 year ago

  There is no need for <div class="qr-background"> in the html and its styling in the css.

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

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

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

  Give h1 and p the same font-size of 15px which is 0.9375rem, text-align: center, the same margin-left and margin-right values . Give p a margin bottom value.

  For a responsive content,

  • Replace the width in .container with max-width.
  • Give the img a max-width of 100% and a border-radius value, the rest are not needed.

  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

  1

  ---

  

  Chris Sutcliffe•10

  @testpilotukPosted over 1 year ago

  @Hassiai thank you for your comments, they were helpful. I do know a little about accessibility but did not apply it here. I will study more and follow this principle in future projects.

  0
• 

  Cesar D.•400

  @ThatDevDiazPosted over 1 year ago

  Hey, you pretty much nailed it.

  You should use the font weight within the stylesheet so the text below the QR code is just a little bit closer to the original than what you have. I believe the fonts-weight was 400 and 700 on the provided sheet.

  Your HTML has a lot of "links" in them when in reality you only needed to link your stylesheet. Everything else could've been done within the stylesheet like the font-family for example.

  Setting your font size to a % instead of 16px as shown within the stylesheet was something I'm not 100% sure about why it was done, so I don't want to comment on that, but it is something I noticed.

  I would avoid using "ID" and instead use classes more often. It's a good habit to have.

  As for the QR code, you could've maybe specified margin-left and margin-right to get a more even spacing between the QR code and the container. There's other ways to do this too depending on the situation. Specifying left/right padding might have worked too. I would've played around with both and worked with whichever gave me the result I was looking for. Ideally, using flexbox and separating the design into 2 major components within 1 "container" might have made this easier.

  You could have also added a "Border-shadow" to the container since it seems like you didn't have one when the original design did. This is something I just recently learned myself.

  Nothing too major to fix that I see from my newbie eyes, just small things that might maybe help in the long run from what I've been learning.

  Good job

  Marked as helpful

  0

  ---

  

  Chris Sutcliffe•10

  @testpilotukPosted over 1 year ago

  @ThatDevDiaz Thanks for your feedback. The percentage for the font is just there to make calculations in em and rem easier (it makes the default 10px instead of 16) but does not affect the users ability to change font size.

  Thanks for your tip about id and I will curb that bad habit.

  I linked the google font as that was where the font Outfit came from otherwise it did not display the correct font on my desktop (Linux).

  0
• 

  Pablo Alvarado•60

  @paalvaradorPosted over 1 year ago

  @testpilotuk

  Hi Chris. The design it's ok

  I recommend you use semantic HTML. For example, you can put all the main code into <main> tag and center the component on de screen. This is the first challenge and It's ok, but in the next challenges is important the use of layouts like flex or grid

  When the project see on screen sizes with 375px you have to change the css to adjust it on mobile screens.

  I think that your code its accessible

  Enjoy your nexts challenges and happy code!

  Marked as helpful

  0

  ---

  

  Chris Sutcliffe•10

  @testpilotukPosted over 1 year ago

  @paalvarador Thank you for your helpful comments. The semantic issue seems to be a common thread throughout my feedback, I will strive to correct this for future challenges.

  0