qr-code-component-main

Community feedback

• 

  T

  Grace•30,990

  @grace-snowPosted over 1 year ago

  Hi

  This doesn’t look like the design yet. Try and get it much closer.

  Remember a few key points:

  • you must use landmarks. Minimally every html page should have a main.
  • do not style on IDs! That’s not what they are for
  • remove all the br elements. That is not how you create line breaks in modern responsive development. The line breaks should happen naturally determined by the available space.
  • the component needs a max-width (in rem)
  • the component needs a little margin on all sides (or you could add some padding to a wrapping element like the body)
  • always use a modern css reset at the start of your styles. Andy bell has a good article and example explaining what’s in his and why. This reset would include things like setting the image to display block and width 100% but does lots of other things too.
  • look up how to write good alt text on images. They shouldn’t ever include words like “image” because an img element already has a role of “image”. Instead this alt needs to say what the image is (QR Code) and where the QR code points to
  • do not use floats! You will almost never need to ever use floats except for images inset into blocks of text (such as in a blog article perhaps)
  • never ever write font size in px (really important!)
  • you are missing all the fonts in this. Link them in the html head then apply them in the css

  1

  ---

  

  KingSky•190

  @KingSkyrosPosted over 1 year ago

  thank you I will look into it

  0
• 

  KingSky•190

  @KingSkyrosPosted over 1 year ago

  This comment was deleted about 1 year ago

  0

  ---

  

  T

  Grace•30,990

  @grace-snowPosted over 1 year ago

  @KingSkyros this is still missing a modern CSS reset

  Also:

  • It's still got inaccessible font size
  • the alt attribute is incorrectly formatted with a space before the quote marks
  • the alt text has incorrect capitalisation and does not say where the QR Code goes to
  • the component does not yet appear to have any margin (or body padding). One of these is needed so it can't hit screen edges
  • the paragraph has strange tiny maegins and paddings. It should only have vertical margins, as should the heading or image to control the vertical space between each element.

  Have you re-deployed this?

  0

  

  KingSky•190

  @KingSkyrosPosted about 1 year ago

  @grace-snow I have added everything you have mentioned

  0
• 

  Kamlesh Rajesh Yadav•5,060

  @Kamlesh0007Posted over 1 year ago

  Congratulations on completing the challenge! 🎉That's a great achievement, and I'm sure you put a lot of effort into it. I really liked the way you approached the challenge and the code you wrote. You demonstrated a good understanding of the concepts and applied them effectively to solve the problem.I have a few suggestions to improve your code further. You need to add background color to body to make background as per design

  body
  {
  background:hsl(212, 45%, 89%);
  }
  

  0

  ---

  

  KingSky•190

  @KingSkyrosPosted over 1 year ago

  thanks i will also look into yours to

  0