3 Column Preview

Community feedback

• 

  P

  Fluffy Kas•7,715

  @FluffyKasPosted almost 3 years ago

  Hiya,

  Your solution looks pretty good ^^ There's a few things I noticed where you may be able to improve this a tiny bit.

  1. Your border-radius is different in mobile and desktop view. Not a big issue, it's just a matter of taste, I suppose.

  2. Some top and bottom margin would look nice in mobile view so the component doesn't touch the side of the screen.

  3. You could wrap the attribution in a footer.

  4. Lastly (in this is the only really important thing), do not set your font-size in px. If a user would like to change the default font-size in their browser, they won't be able to do so. Use relative units that scale with the changes. :)

  Marked as helpful

  1

  ---

  

  Jacques•110

  @JJOosthuizenPosted almost 3 years ago

  Hi @FluffyKas ,

  Thanks for the feedback. I didn't even realize I have 2 different border-radius sizes. I'll definitely play around more with the margins and add the footer.

  0
• 

  Dev Rathore•2,600

  @GitHub-dev12345Posted almost 3 years ago

  In this code:- You change the div tag to Footer tag

  <div class="attribution"> Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank">Frontend Mentor</a>. Coded by <a href="">JJ Oosthuizen</a>. </div>

  to

  <footer class="attribution"> Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank">Frontend Mentor</a>. Coded by <a href="">JJ Oosthuizen</a>. </footer>

  1
• 

  Naveen Gumaste•10,480

  @NaveenGumastePosted almost 3 years ago

  Hay ! Jacques Good Job on challenge

  These below mentioned tricks will help you remove any Accessibility Issues

  -> Add Main tag after body <main class="container"></main>

  -> Learn more on accessibility issues

  If this comment helps you then pls mark it as helpful!

  Have a good day and keep coding 👍!

  0