Used flexbox

Community feedback

• 

  Lucas 👾•104,340

  @correlucasPosted about 2 years ago

  👾Bonjour Ornella T, congratulations for your new solution!

  Here's some tips to improve your solution:

  First of all use semantic tags for your content, so replace the div with <main> and since the bold title is the main title of this page you should use h1 instead of h2. To clean your keep everything inside a single div, no need for additional divs.Something you keep in mind to add for your next solution is to use relative units as rem or em instead of px to improve your performance resizing fonts between different screens and devices.

  👋 I hope this helps you and happy coding!

  Marked as helpful

  0

  ---

  

  Ornella T•425

  @ornel77Posted over 1 year ago

  @correlucas hello thank you for your feedback!

  0
• 

  Travolgi 🍕•31,400

  @denieldenPosted about 2 years ago

  Hi Ornella, congratulations on completing the challenge, great job! 😁

  Some little tips for optimizing your code:

  • add main tag and wrap the card for improve the Accessibility
  • also you can use article tag instead of a simple div to the container card for improve the Accessibility
  • use min-height: 100vh to body instead of height, otherwise the content is cut off when the browser height is less than the content
  • in this case display and object-fit properties in the img element are superfluous

  Hope this help! Happy coding 😉

  Marked as helpful

  0

  ---

  

  Ornella T•425

  @ornel77Posted over 1 year ago

  @denielden thank you for your feedback :)

  0
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted about 2 years ago

  Hi Ornella T,

  Congratulation on completing this frontend mentor challenge. Your solution looks great. I have some suggestions regarding your solution:

  • You should use <main> for the card. HTML5 landmark elements are used to improve navigation experience on your site for users of assistive technology.

  • In my opinion, the image is an important content. The alternate text is needed on this image. The alternate text should indicate where the Qr code navigate the user : like QR code to frontend mentor not describes the image. The alternate text should not be hyphenated, it should be human readable.

  • Page should contain <h1>. In this challenge , as it’s supposed to be a part of a whole page, you may use<h1>with class sr-only aiming at visually hiding the element while keeping it accessible to assistive technologies

  • There are a lot the arguments changing the root font size trick ,it state that you should never change the root font size because it harms accessibility.

  • Add min-height: 100vh instead of height: 100vhto the body that let the body grows taller if the content outgrows the visible page instead.

  • width: 23rem; an explicit width is not a good way to have responsive layout . Consider using max-width to the card instead .

  Hopefully this feedback helps.

  Marked as helpful

  0

  ---

  

  Ornella T•425

  @ornel77Posted over 1 year ago

  @PhoenixDev22 Thank you that was really helpfull!

  1

  

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 1 year ago

  @ornel77 You’re welcome. Happy coding!

  0
• 

  Elaine•11,400

  @elaineleungPosted about 2 years ago

  Hi Ornella, this looks great, and I'm glad to hear you're getting back to coding (I've been there too)!

  You did many things well here, including centering the component and making it responsive. Two small suggestions here:

  1. Instead of the height: 100vh, change that to min-height: 100vh instead so that when the height of the browser is smaller than the contents, the content would not be cut off when you use min-height.

  2. You can add 1rem of margin around .card so that the sides won't touch the browser.

  That's all, do keep going, you're doing great! 😊

  Marked as helpful

  0

  ---

  

  Ornella T•425

  @ornel77Posted over 1 year ago

  @elaineleung Hello I reply only now but thank you very much for your feedback !

  1

  

  Elaine•11,400

  @elaineleungPosted over 1 year ago

  @ornel77 No worries, I also only just saw this now! 😊

  0