flexbox css

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, awesome work on this one. The overall layout I think looks fine, just needed for the wavy-background to be visible because right now it is not present. For mobile state, the layout have a fixed width that causes the screen to hide the content if you go to like 340px upwards.

  Aakash Verma already gave a feedback on this one, just going to add some suggestions as well:

  • On your body tag, replace the background-image value to url("./images/pattern-background-mobile.svg"). You mistakenly use ../ instead of ./. Also, use the desktop background for the desktop layout. Also, use background-size: 100% instead of contain so that it will look like the design properly.
  • Use footer tag on the .attribution so that it will be its own landmark element.
  • Using figure on the illustration is not really needed on this one. Typically, you want to use figure when the image have a dedicated text that serves as the figcaption. On this one, a regular img with alt="' and aria-hidden="true" attribute will be really nice, since the image is only acting as decorative image and decorative image should always be hidden for screen-reader users.
  • Also, when using img tag, you don't need to add words that relates to "graphic" such as "illustration" and others, since img is already an image so no need to describe it as one.
  • Same goes for the music-icon, use only an img tag with the same attributes since it is only a decorative image.
  • The annual plan text could use a heading tag since it gives information on what a section would contain, hence the plan's price.
  • When using a tag, always include the href attribute on it so that the a tag will work properly as a link or interactive element.
  • On your .card selector, use max-width: 450px instead of width: 450px so that the selector won't have a fixed size. Then use a width: 100% so that it will fill the size.
  • Lastly, add a width: 100% on the illustration of the card so that it will only consume the parent's width and not overlap.

  Aside from those, great job again on this one.

  Marked as helpful

  1

  ---

  

  ibra188•20

  @ibra188Posted almost 3 years ago

  @pikapikamart thank you so much I will re-define and re-build everything again..! I really appreciate your feedback thank you.

  1
• 

  Aakash Verma•6,710

  @skyv26Posted almost 3 years ago

  Hi! Ibra, I appreciate your efforts and you did it really good.

  Your Desktop design is ok, but as we move to mobile view then, you card going outside from the viewport. Fix it using margin, width in media query CSS.

  Fix above and your design will then be OK.

  I hope you understand.

  Good Luck

  Marked as helpful

  0

  ---

  

  ibra188•20

  @ibra188Posted almost 3 years ago

  @skyv26@media screen and (min-width: 320px) { .card{ width: 450px; margin: 0; } I try but am stock...?

  1

  

  ibra188•20

  @ibra188Posted almost 3 years ago

  @skyv26 thanks for the help.

  1