Responsive Landing Page using Flexbox

Community feedback

• 

  P

  Q-bert•210

  @QBERT18Posted about 2 years ago

  Hello! Nice try! But there stuff to improve.

  HTML Improvement:

  • I like your creativity. You solved the image responsivenes with 2 different image tags, by switching them on and off. But that isn't the optimal way to do it. Read this https://developer.mozilla.org/en-US/docs/Web/HTML/Element/picture?retiredLocale=de or type "picture" html tag in google :).
  • It is also creative that you used whitespace between the letters of your headline. Also you typed capital letters manually. Also not optimal way to solve that problem. Read this https://developer.mozilla.org/en-US/docs/Web/CSS/letter-spacing?retiredLocale=de and this https://developer.mozilla.org/en-US/docs/Web/CSS/text-transform?retiredLocale=de for better solution.
  • section and aside tag are overkill for just 2 short price strings haha. Just use span or p or div. Read about the usage of section and aside to understand their usage.

  CSS Improvement:

  • You are giving individual width's to both of the left and right containers. Why not one width to the parent element? Just wrap them into section and give it 500px with instead of 250px to each container.
  • Float is old use flex or grid.
  • You are using to much px. Learn rem for more responsivenes.
  • padding-top: 14px; padding-bottom: 12px; padding-left: 70px; padding-right: 70px; can be written shorter like this padding: 14px 12px 70px 70px; read more about padding here: https://developer.mozilla.org/en-US/docs/Web/CSS/padding

  That were the short list of stuf i could find. I hope i could help you a little bit. Let me know if i did.

  Best Regards Q-bert :).

  Marked as helpful

  0
• 

  Lindelwe Khumalo•170

  @iSoyecodesPosted about 2 years ago

  Hello, congrats on finishing the challenge! Instead of using margin-top to center your card, try using minimum height: 100 vh, which will center your card in line with the design. Put width: 100% rather than a max-width on your image to enable responsiveness and prevent it from being locked to 100%. Additionally, instead of max-width in your media query, try using min-width. Do you know what CSS Grid is? If so, try integrating it with Flexbox. If not, try learning it so that you can comprehend how some people use grid in conjunction with flexbox when reading other people's solutions.

  Please Mark this useful, Please.

  a sample of how to center your card using grid

  .container { width: 100%; min-height: 100vh; display: grid; grid: 1fr min-content / 1fr; row-gap: 1rem; place-items: center; padding: 1.69rem 1rem; }

  using flex-box

  .container { background-color:hsl(212, 45%, 89%); min-height: 100vh; display: flex; flex-direction: column; justify-content: center; }

  Marked as helpful

  0