QR code component with custom css properties and css grid

Community feedback

• 

  Steven Portas•440

  @satropPosted almost 2 years ago

  Hey there!

  You’re final outcome looks great! And just looking at the CSS, because @denielden gave you great HTML pointers. The only tweak I would do is to you card-padding variables.

  Take This

  --card-padding-y: 1rem;
  --card-padding-x: 1rem;
  padding: var(--card-padding-y) var(--card-padding-x);
  

  And I Would Do This

  —padding: 1rem; 
  padding: var(—padding);
  

  So With the above code you’re setting the padding on both the x/y. A nice shorthand for doing the just top and bottom would be:

  —padding: 1rem; 
  padding-block: var(—padding);
  

  And to do padding just left and right:

  —padding: 1 rem; 
  padding-block: var(—padding);
  

  Just learnt these myself. This can also be used for margins margin-inline and margin-block`.

  Hope this helps!

  Marked as helpful

  2

  ---

  

  Edwin Boon•30

  @brwmasterPosted almost 2 years ago

  @satrop

  Thanks for the feedback. I red a out those new propertiew but I totally forgotten about them.

  Going to apply them in my code (:

  2

  

  Travolgi 🍕•31,400

  @denieldenPosted almost 2 years ago

  @satrop SteveThanks!

  Great reasoning for optimization. Furthermore, to make the variable more usable it is also possible to name it in a generic way (e.g. --sm: 1rem; --md: 2rem; --lg: 3rem) so as to use it a little more widely and not only for i padding.

  Keep going! :)

  2

  

  Steven Portas•440

  @satropPosted almost 2 years ago

  @denielden ah yes! Great call. I was looking at it from a far more focused view. This is a rule of thumb in my everyday builds.

  1
• 

  Travolgi 🍕•31,400

  @denieldenPosted almost 2 years ago

  Hello Edwin, You have done a good work! 😁

  Some little tips to improve your code:

  • you can use article tag instead of a simple div to the container card for improve the Accessibility
  • remove all unnecessary code, the less you write the better as well as being clearer: for example the picture container of image
  • remove all margin from card class because with grid they are superfluous
  • use min-height: 100vh to body instead of height, otherwise the content is cut off when the browser height is less than the content
  • instead of using px use relative units of measurement like rem -> read here

  Keep learning how to code with your amazing solutions to challenges.

  Hope this help 😉 and Happy coding!

  Marked as helpful

  2

  ---

  

  Edwin Boon•30

  @brwmasterPosted almost 2 years ago

  @denielden

  Thanks for your input! Really appreciate the effort and time you spent on the review. I updated the code based on your comments.

  It was really helpful!

  1

  

  Travolgi 🍕•31,400

  @denieldenPosted almost 2 years ago

  @brwmaster You are welcome and keep it up :)

  1