QR code component

Community feedback

• 

  Sivalingam R•30

  @sivalingamr2001Posted 18 days ago

  Its most valuable information. Thank you for sharing those things. Once again thank for your feedback.

  0
• 

  T

  Grace•31,090

  @grace-snowPosted about 2 months ago

  Hi I hope these changes are helpful

  • all content should be contained within landmarks. Although this isn't a full Web page get into the habit of wrapping single component demos like this in a main landmark. (The card should go.inside it, the footer stays separate as it's own landmark outside of main). Every page should have a main.
  • you don't need to wrap every element in a div. The card only needs an img, heading and paragraph inside.
  • the alt description on the image needs more information. It needs to say what the image is (QR code) and where it goes (to FrontendMentor.io).
  • it's better for performance to link fonts in the html head instead of css imports.
  • get into the habit of including a full modern css reset at the start of the styles in every project. Andy Bell or Josh Comeau both have good ones you can look up and use.
  • keep everything as simple as possible. Just as you don't need all the extra divs you don't need any media queries in this. Remove them.
  • avoid setting overflow-x hidden on the body. That can lead to big accessibility problems on larger sites. On the rare occasions content does overflow people need to be able to read it.
  • Font size shouldn't ever be set in pixels. Use rem instead.
  • the card must not have a width or height. Never limit the height of elements that contain text. Let the browser do it's job and decide what height is needed based on the content inside. All this card needs in terms of size is a single max width in rem. Doing that allows the card to be flexible and shrink narrower when it needs to like on a small screen.
  • the card should have padding on all sides. As I said you don't need those inner divs no extra paddings are needed inside. To create vertical space between the card child elements use vertical margins. I've written a post about the difference between padding and margin which you may find helpful.
  • try to avoid using complex css selectors unless you have no choice. You don't need to use card-header > img. Just place a class directly on the img. Same on heading and paragraph.
  • all the image needs is border radius and optionally width 100% and aspect-ratio 1. The css reset I mentioned would already set img elements to display block and max-width 100%.
  • remember block elements stack vertically by default. You don't need flexbox inside the card or footer.
  • you can set the body or card text-align to center in this and it will be inherited by the child elements. That would save you having to repeat the property lots of times.

  0
• 

  Sivalingam R•30

  @sivalingamr2001Posted about 2 months ago

  Nothing all are good.

  0

  ---

  

  T

  Grace•31,090

  @grace-snowPosted about 2 months ago

  @sivalingamr2001 take a look at the feedback left on this. Its good to learn from others feedback.

  0