CSS flexbox, media-queries.

Community feedback

• 

  Piyush jain•220

  @PiyushJain01Posted over 3 years ago

  Nice work in mobile view. Small suggestion:- In .order-summary button you can add little transition, not mandatory but if you add then it looks perfect :)

  Marked as helpful

  0

  ---

  

  sanket kumar•300

  @sanketcharanpahadiPosted over 3 years ago

  @piyushjain2211 okay brother I will try it. Thank you for the feedback.

  0
• 

  T

  Grace•31,270

  @grace-snowPosted over 3 years ago

  I just realised you asked for feedback on media queries too…

  This doesn’t need any media query at all. A max-width on the card is all it needs and some padding on the outer wrapper to prevent content from hitting the sides of the screen on small screens.

  Those two things are fine at all screen sizes.

  For future projects

  1. Don’t include max-width: 100vw;. That can have unexpected results as viewport units don’t account for scroll bars or things like device toolbars or notches.
  2. Work from mobile first and use min-width media queries (target “this screen size and larger). Css will be shorter, simpler and with fewer overrides in media queries that way.
  3. Only add a media query at the point a layout needs to / has room to change. Don’t just set one as 375px for “larger/smaller than mobile”.
  4. Remember devices are sometimes held landscape, and some devices are large, so mobile may be larger than you think

  Marked as helpful

  0
• 

  T

  Grace•31,270

  @grace-snowPosted over 3 years ago

  Hi

  Viewing on mobile this looks good. The proceed button appears to move a little on hover/click because you’re removing the border. It would be better to change the border color to transparent so that the border height remains and nothing moves.

  You need to fix some important bits in the html on this

  • headings must go in order. So you can’t jump from h1 to h4. They are for semantic meaning, nothing to do with sizing. If you want annual plan to be a heading, it needs to be a h2
  • alt text should be written in a human readable way, not a phrase of hyphenated words
  • if an image is decorative (doesn’t give extra meaning to the page), alt should be left intentionally blank and the either role=“presentation” or aria-hidden=“true” added. That way, screenreaders and bots know the image is not important
  • the change button performs an action, so should definitely be a button element.
  • “proceed” implies navigation. And this is not structured as a form. Therefore, the proceed button should actually be an anchor tag not a button element
  • cancel could be either, but most likely an anchor tag as this is not a form. It 100% is not a paragraph tag. Always use interactive elements for things that would cause interactive behaviour.

  Because of the way you are applying classes - only to parent elements - it is forcing you to make your css more specific than it needs to be. Your css selectors on this are often “class element” or “class > element” (even more specific). This could start to cause big problems as you progress to larger projects. I recommend putting classes on anything you want to style directly so you can use single class selectors in css.

  Never style (or hook JavaScript) onto IDs. Use classes.

  I hope this has been helpful. Good luck with your coding 👍

  Marked as helpful

  0

  ---

  

  sanket kumar•300

  @sanketcharanpahadiPosted over 3 years ago

  @grace-snow Thanks for your feedback. I will try to improve this project and thanks for your suggestions I will always keep the suggestions on my mind in building future projects. Thank you once again.

  0