Responsive price grid component built with html css flexbox

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  Hi, Andrew! 👋

  Congratulations on finishing your first Frontend Mentor challenge! 🎉 It's great that you made the site responsive using the mobile-first approach! Good job! 👍

  Regarding your question, when I did the challenge I wrapped the $29 on its own span. So, if you want to follow my approach then I recommend wrapping the $29 text with a span and the make the p element as the flex container to align the per month text center vertically.

  <p class="subscription__price">
    <span class="subscription__number">$29</span>
    <span class="subscription__month">per month</span>
  </p>
  

  You've used the correct HTML markup by using a paragraph tag as the main element to wrap those text. The text only makes sense if those are read together (or as one paragraph) by screen readers. So, you are on the right track. 👍

  One thing that is worth fixing is the background color for the Sign up link on the hover state. I believe it should be a lighter color than the original state. The black color is not match the color scheme of the page (at least in my opinion).

  That's it! I hope this information is useful! 😁

  Marked as helpful

  1

  ---

  

  Andrew Saganda•50

  @asagandaPosted over 2 years ago

  @vanzasetia I appreciate your feedback. I've actually updated the html structure of the price per month to align with your version because that makes more sense. I don't remember reading that the hover state of the CTA in desktop should be a specific color, however i do agree that the black is a bit harsh given the color theme of this challenge. I'm open to what background color the hover state of the CTA should be...

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @asaganda Great job on the update! I recommend using a lighter color than the original button color. You can increase the lightness of the color by adjusting the third value of the hsl(first, second, third).

  0
• 

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  Hi Andrew, great work on this challenge! 😉

  Here are a few tips for improve your code:

  • add main tag and wrap the card for improve the Accessibility
  • to smooth also the bottom angle add overflow: hidden to container class
  • remove all margin from container class and body
  • use flexbox to the body to center the card. Read here -> best flex guide
  • after, add min-height: 100vh to body because Flexbox aligns child items to the size of the parent container
  • add transition on the element with hover effect
  • instead of using px use relative units of measurement like rem -> read here

  Overall you did well 😁 Hope this help!

  0
• 

  T.Praveen•870

  @gwtpraveenPosted over 2 years ago

  hi Andrew Saganda, good job completing this challenge

  in terms of accessibility issues simply wrap all content between main tag and you should have one level one heading h1 tag

  keep up the good work...!

  0

  ---

  

  Andrew Saganda•50

  @asagandaPosted over 2 years ago

  @Tim5583 Can you provide a resource that explains the using the main tag for better accessibility?

  0

  

  T.Praveen•870

  @gwtpraveenPosted over 2 years ago

  @asaganda main tag ->> MDN

  Marked as helpful

  1
• 

  Shashree Samuel•9,260

  @shashreesamuelPosted over 2 years ago

  Hey good job completing this challenge

  Keep up the good work

  Your solution looks great however I think that the card needs a bit of margin from the top using margin-top.

  I hope this helps

  Cheers

  Happy coding 👍

  0

  ---

  

  Andrew Saganda•50

  @asagandaPosted over 2 years ago

  @TheCoderGuru I realized after i submitted that the desktop version had more margin on top than what i originally had. I fixed it as soon as i realized yesterday - check it out again please, https://fem-single-price-grid-component.netlify.app/

  0
• 

  Prabhash Ranjan•2,540

  @besttlookkPosted over 2 years ago

  hi, Any time you need to center, you can use flexbox. Like to make your content center w.r.t screen, you can do this:

  body{
    display:flex;
    justify-content:center;
    aligned-items:center;
    min-height: 100vh;
  }
  

  Good luck

  Happy Coding

  0