Summary card CSS

Community feedback

• 

  Éric Férole•420

  @Eric-FerolePosted almost 3 years ago

  Hi JsRavel, Nice work. Well done for the JS. Here's some comments :

  • Fix the positioning of your hover in desktop
  • Try to center your card in the page horizontally and vertically
  • Use more meaningful name for your classes. Instead of superCont, drw, shaR. It help other to - better understand what it is.

  Hope it helps,

  Keep up.

  1
• 

  Vanza Setia•27,775

  @vanzasetiaPosted almost 3 years ago

  👋 Hi Jordy!

  🎉 Congratulations on finishing this challenge! I have some feedback on this solution:

  • Accessibility
    • article is not a landmark. There are three landmarks, header (banner), main, and footer (content info). In this case, you can wrap all the page content, except your attribution with main tag.
    • For the attribution, you should wrap it with footer tag.
    • For any decorative images, each img tag should have empty alt="" and aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images. In this case, all images are decorative only.
    • Create a custom :focus-visible styling to any interactive elements (button, links, input, textarea). This will make the users can navigate this website using keyboard (Tab) easily.
    • The below markup should not be an unordered list. It should be a paragraph.

  <ul class="card-text">
          <li class="card-text__first">You can now listen to millions of </li>
          <li class="card-text__second"> songs, audiobooks, and podcasts on</li>
          <li class="card-text__third"> any device anywhere you like!</li>
  </ul>
  

  • Heading order is extremely important. People can navigate the website using the heading tags, so it is important that they should be used correctly. You should use h1 for the Order Summary and h2 for the Annual Plan.
  • The payment button and the cancel order button should be wrapped with interactive elements (button or a, depending on what the button is going to do).
  • Only use div and span for styling purposes.
  • Styling
    • Why are you commenting the display: flex; code? Isn't it needed in order to make the card in the middle of the page?

  body{
      padding: 0;
      margin: 0;
      /* display:         flex; */
  

  • You can remove the margin: 0 auto; from the .card and let the body make the card in the middle of the page.

  /**
   * 1. Make the card vertically center and
   *     and allow the body element to grow
   *     if needed
   */
  body {
    display: flex;
    align-items: center;
    justify-content: center;
    min-height: 100vh; /* 1 */
  }
  

  • Let the content inside the card element dictate the height of it. It allows the card to grow, let's say if you add more content to it.
  • Best Practice (Recommended)
    • It's recommended to not aim pixel-perfect. You can read this blog post from Josh Comeau about Chasing the Pixel-Perfect Dream
    • Always set a backup font family.

  body {
    font-family: Outfit, sans-serif;
  }
  

  Tha's it! Hopefully, this is helpful!

  0