Skip to content
Submitted over 2 years ago

stats-preview-card-component-main

Nida Ismail•70
@nidaismail
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Stats preview card component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

Happy to complete another challenge, looking forward to feedback.

Community feedback

  • Vanza Setia•27,775
    @vanzasetiaPosted over 2 years ago

    Hello, Nida! 👋

    Congratulations on finishing this challenge! 🎉

    I have some feedback on this solution:

    • Accessibility
      • All the page content should live inside landmark elements (header, main, and footer). By using them correctly, users of assistive technology navigate the website easily. In this case, wrap all of it with main tag,except the attribution. The attribution should be lived inside the footer.
    <body>
  <main>
    page content goes here...
  </main>
  <footer class="attribution">
      attribution links goes here...
  </footer>
</body>
    • Like @david-oncu has said earlier, don't skip heading level.
    • Also, the stats number should not be a heading. The content below it is too small. You might find it helpful if you think of a heading like a title on a document.
    • Use CSS to uppercase the text. The uppercased word in the HTML will be spelled by the screen reader (spelled letter by letter).
    • Good job on leaving the alt empty for the decorative image! 👏
    • Use rem or sometimes em unit instead of px. Using px will not allow the users to control the size of the page based on their needs.
    • Styling
      • Use flexbox to make the card centered both vertically and horizontally.
      • I notice that you have imported all the font-weight for each font family and that makes your site load so slow. I would recommend only importing the necessary font weight for each font family.
      • color: hsla(0, 0%, 100%, 0.75);; There are double semi-colons on line 66 in the style.css.
      • Adjust the breakpoint of the media query. Two columns layout at 600px width is too early.
    • Best Practice (Recommended)
      • I would recommend writing the styling using the mobile-first approach. It often leads to shorter and better performance code. As a result, mobile users will no longer be required to process all of the desktop styles.

    That's it! Hope you find this useful! 😁

    Marked as helpful
    0
    Nida Ismail•70
    @nidaismailPosted over 2 years ago

    @vanzasetia Thank you so much for such brief and comprehensive feedback I really appreciate it and of course, it's very helpful and productive.

    0
    Vanza Setia•27,775
    @vanzasetiaPosted over 2 years ago

    @nidaismail You're welcome! 😄

    0
  • David Oncu•350
    @david-oncuPosted over 2 years ago

    Hello Nida, congratulations on another solution!

    Accessibility:

    1. Set a role for your "main-container" between div and .main-container add role="main" - more about this here: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles

    2. When using h1, h2, h3, h4 scale them from highest h1 to h2, h3, h4, h5, etc. in chronological order. Change your h3 to h2.

    3. For your <div class="attribution"> give a landmark. Add between div and class="... role="footer" > <div role="footer" class="attribution">

    Overall great solution and clean code! Good job!

    Marked as helpful
    0
    Vanza Setia•27,775
    @vanzasetiaPosted over 2 years ago

    @david-oncu Why don't just use the main element?

    0
    Nida Ismail•70
    @nidaismailPosted over 2 years ago

    @david-oncu thank for your feedback

    0

Please log in to post a comment

Log in with GitHub

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord