QR code using CSS Flexbox

Community feedback

• 

  Daniel 🛸•44,310

  @danielmrz-devPosted 12 months ago

  Hello, @Singraft!

  Your project looks nice!

  I have a few improvement tips for you:

  • You can place your card in the middle of the page by doing this:

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

  Doing that, you don't need to use margins on your card in order to center it.

  • Also, the color of your text is very light. Take a look at the style-guide file in order to use the correct color.

  Other than those little things, great job!

  Marked as helpful

  1

  ---

  

  Dongmo Shu•280

  @dongmo-shuPosted 12 months ago

  Hello @danielmrz-dev

  you hit the right point. I had so much trouble getting the body to be at the center of the page. I will certainly add your recommendation to the code the next time I have a go at it.

  I will send an update once I do so. Thank you.

  1

  

  Daniel 🛸•44,310

  @danielmrz-devPosted 12 months ago

  I'm glad my feedback was useful! @Singraft

  I'll be waiting for the update 😊

  1

  

  Dongmo Shu•280

  @dongmo-shuPosted 12 months ago

  Hello @danielmrz-dev

  With your feedback, I made the following modifications;

  1. Changed the text colour to that of style-guide
  2. Increased the overall size of the website
  3. Modified the body
  4. Improve code readability and reduced length.

  Have a look, and let me know if more modifications are needed.

  0

  

  T

  Grace•31,270

  @grace-snowPosted 10 months ago

  @danielmrz-dev always min-height, never height. And use a flex column (the default is row)

  It's really important to never limit the height of any elements that contain text, including the body. Height 100vh will cause definite bugs, moving some content offscreen and inaccessible (eg in small mobile landscape or when users have a larger font size)

  0
• 

  T

  Grace•31,270

  @grace-snowPosted 10 months ago

  There are some foundational issues in this that need correcting then taking into future challenges

  1. Only import the specific font weights you need. Otherwise you will seriously damage performance
  2. Always use html landmarks. A main on every page is the bare minimum, but in this case you also need a footer for the attribution
  3. The image is the most important content on this component. It is not a decorative image so must not have empty alt. The description needs to say what it is (QR code) and where it goes (to FrontendMentor.io)
  4. Keep the html nodes as simple as you can. There is no need for the img to be wrapped in an extra div
  5. Height on the body needs to be min height as I explained in my other comment
  6. Always use a modern css reset at the start of the styles in every project. Andy Bell has a good one you can look up and use.
  7. Include fallbacks in any font family declaration
  8. The card does not need flexbox. Block elements stack by default. There is no benefit at all to using flex there, just some extra bloat in the css
  9. The card must not have a width or height, especially not in pixels! This is important to get your head around ASAP. Never ever limit height like that. The browser is smart, let it do its job and choose the height based off the cards contents and any padding and margins within. Instead of width on the card use max-width and in rem. This tells the browser the card can shrink narrower if it needs to (eg on a small device screen) but don't let it go above X size. By using rem instead of px you ensure all users have the same experience, even those of us with different font size settings.
  10. The image does not need a div around it and definitely not one with a fixed height.
  11. The img only needs border radius and two properties that are a standard part of most css resets (as mentioned above: max-width: 100%; display: block ;
  12. The text does not need to be display flex and it must not have a width. If you want to limit it's width in some way use max-width again (usually in ch units on a paragraph, failing that it needs to be rem) along with margin-inline auto for the centering. Alternatively you can give the paragraph no max width but add a little inline margin to make it narrower but that may look strange on small screens.
  13. The card has padding already so elements within it should only have vertical margins (unless limiting their width, in which case you'd use margin-inline auto as I just mentioned)
  14. ⚠️ this one is extremely important: Never ever write font size in px. That immediately makes a solution inaccessible. Use rem. Here is a post about font size units. I think you'll find some other posts on my site useful after reading that one.

  0