i used flexbox

Community feedback

• 

  P

  Jeff Guleserian•480

  @jguleserianPosted 4 months ago

  Greetings, Itchaboidavid!

  Thank you for letting me take a look at your solution for the QR Code Component Challenge. You code is easy to read and shows you know how to center the component (which can sometimes be tricky). Everything seems to be proportioned well and the presentation is effective and functional.

  In terms of offering constructive criticism, I noticed a couple of things that might be helpful. In comparing your solution to the original, yours is smaller. As I look through the code, I believe that this was the result of you setting the width of the div.container at 225px rather than the called for 320px. This would also explain why the text starting with "Soon the qr code..." does not mirror the model and why your fonts sizes are smaller than the model.

  I also noticed that the padding between the qr image and the top of your <div> container is not equal to the padding on the right and left. Personally, I solved this by putting a padding in my main container (or in your case, div.container) to make it even on the left, top, and right. Then I put a bigger padding on the bottom, something like: padding: 16px 16px 40px 16px.

  Don't forget that the Figma file (included with the download) would supply all of these measurements. It also would have indicated that the image needed a drop shadow. I tell you, these Figma files are so helpful!

  The last thing I will mention, and this is only to keep it at the forefront of your mind, is accessibility. You probably noticed on the accessibility report run on your submission, that it was lacking an<h1> or major heading. I get it that this is just a component and that, depending on how it was used, you might, or might not, need an <h1> tag. In a case like this when it is not obvious that an <h1> tag is needed, I usually put in an<h1> and then hide it offscreen. While there are many ways to do this, I used:

  h1 { display: hidden; position: absolute; left: 200rem; }

  As a postscript, I would say that, while for this simple of a submission, inline styles work well, as the projects get more complex, I would encourage you to use a stylesheet.

  In conclusion, your solution is very good, especially if you did not use the Figma file. I applaud your effort. I hope you find these comments helpful (take them for what they're worth). If you have any questions that I may be able to help with, please let me know.

  Again, good job!

  Happy coding!

  Jeff

  0
• 

  shriyamamidela•50

  @shriyamamidelaPosted 4 months ago

  Great job

  0