Skip to content
Submitted almost 2 years ago

order summary challenge attempt

Babatunde•20
@n-uzumaki
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Order summary component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

I used bootstrap cards to complete this project, I would appreciate some helpful feedback. Thank you

Community feedback

  • Melvin Aguilar 🧑🏻‍💻•61,140
    @MelvinAguilarPosted almost 2 years ago

    Hello there 👋. Good job on completing the challenge !

    I have some feedback for you if you want to improve your code.

    • Not all images should have alt text. The illustration-hero and music-icon are for decoration purposes only. In this case, you can add aria-hidden="true" to the element and leave the alt attribute blank.

      Adding aria-hidden="true" to an image will prevent it from being read by screen readers. This is useful when the image is purely decorative and does not convey any information that needs to be communicated to the user.

      <img src="./images/illustration-hero.svg" alt aria-hidden="true" class="card-img-top">

      You can read more about this here 📘

    • There is a debate about what is the best type of elements for this challenge, but in my opinion, the "change" element should be a button and not an anchor tag, because anchor tags are typically used to link to other pages or sections of a page, while buttons are used to trigger an action. In this case, the action is to change the plan of the order, so a button would be more appropriate.

    I hope you find it useful! 😄

    Happy coding!

    Marked as helpful
    0
    Babatunde•20
    @n-uzumakiPosted almost 2 years ago

    @MelvinAguilar Thank you, this was insightful. I think your argument regarding the change element makes sense, I'll incorporate your suggestions in my code. Thanks again for the feedback!

    0
  • Atif Iqbal•3,200
    @atif-devPosted almost 2 years ago

    Hi n-uzumaki, Congrats🎉 for completing the challenge. Take care of the following points:

    • Always check frontendmentor report generated issues after submitting the project for improvements.
    • To avoid accessibility issue "All page content should be contained by landmarks" use code as :
    <body>
    <main>
        ---your code here----
    </main>
  <footer>
  </footer>
</body>
    • For centering the container(whole card) vertically and horizontally, you can use code as:
    body {
    min-height: 100vh;
    display: grid;
    place-content: center;
}

    Or you can learn centering from here.

    Hope you will find this Feedback Helpful.

    Marked as helpful
    0
    Babatunde•20
    @n-uzumakiPosted almost 2 years ago

    @atif-dev Thank you for your feedback, I'll update my code accordingly. This was very helpful! I'll check out the link as well. Thanks again!

    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